Re: [CHECKER] 87 potential array bounds error/buffer overruns in 2.5.53

From: Yichen Xie (yxie@cs.stanford.edu)
Date: Tue Jan 28 2003 - 04:21:11 EST


Hi Randy, Thanks for the quick feedback! I had some additional comments
below starting with '%'. -Yichen

---------------------------------------------------------
% I was thinking about the case where "de" is NULL and "old"
% gets garbage from the stack (since there's no initializer);
% _devfs_alloc_entry won't be called in this case...

# This one is OK. _devfs_append_entry() sets <old>.
[BUG] "old" might get garbage on the stack
/home/yxie/linux-2.5.53/fs/devfs/base.c:1179:_devfs_make_parent_for_leaf
: ERROR:BUFFER:1179:1179:Dereferencing uninitialized pointer (*old)
evaluated in the following state
            de = _devfs_alloc_entry (name, next_pos, MODE_DIR);
            devfs_get (de);
            if ( !de || _devfs_append_entry (dir, de, FALSE, &old) )
            {
                devfs_put (de);

Error --->
                if ( !old || !S_ISDIR (old->mode) )
                {
                    devfs_put (old);
                    devfs_put (dir);
---------------------------------------------------------
% In the definition of split_status_strings (line #683),
% Comma after the first string is missing, so the first and
% second string are concatenated together to form one element
% --the array only has 7 elements, instead of 8.

# I don't see the problem....
[BUG] missing comma in definition of split_status_strings
/home/yxie/linux-2.5.53/drivers/scsi/aic7xxx/aic79xx_pci.c:808:ahd_pci_s
plit_intr: ERROR:BUFFER:808:808:Array bounds error (off >= len) [RANGE]
(split_status_strings[bit], len = 7, off = sym_42005934, max(off-len) =
0)
                for (bit = 0; bit < 8; bit++) {

                        if ((split_status[i] & (0x1 << bit)) != 0) {
                                static const char *s;

Error --->
                                s = split_status_strings[bit];
                                printf(s, ahd_name(ahd),
                                       split_status_source[i]);
                        }
---------------------------------------------------------
% similar reason as above; missing comma after "REQ_SPECIAL"

# Don't see a problem here....
[BUG] missing comma in rq_flags
/home/yxie/linux-2.5.53/drivers/block/ll_rw_blk.c:685:blk_dump_rq_flags:
ERROR:BUFFER:685:685:Array bounds error (off >= len) (rq_flags[bit], len
= 17, off = 17, min(off-len) = 0)
        printk("%s: dev %s: flags = ", msg,
                rq->rq_disk ? rq->rq_disk->disk_name : "?");
        bit = 0;
        do {
                if (rq->flags & (1 << bit))

Error --->
                        printk("%s ", rq_flags[bit]);
                bit++;
        } while (bit < __REQ_NR_BITS);

---------------------------------------------------------
% as i suspected... ;-)

# Comment in struct says that its size there is a placeholder........
[BUG] suspicious, since sizeof cmd is 2 (not 0 or 1, which is commonly
used for buffers)
/home/yxie/linux-2.5.53/drivers/char/ip2/i2cmd.c:209:i2cmdUnixFlags:
ERROR:BUFFER:209:209:Array bounds error (off >= len) ((*pCM).cmd[6], len
= 2, off = 6, min(off-len) = 4)
        pCM->cmd[1] = (unsigned char) iflag;
        pCM->cmd[2] = (unsigned char) (iflag >> 8);
        pCM->cmd[3] = (unsigned char) cflag;
        pCM->cmd[4] = (unsigned char) (cflag >> 8);
        pCM->cmd[5] = (unsigned char) lflag;

Error --->
        pCM->cmd[6] = (unsigned char) (lflag >> 8);
        return pCM;
}

---------------------------------------------------------
% hmm.. looks like a bug in our checker :p

# But if unit is 0 and becomes -1, that shouldn't be executed??
I.e., I think this code is OK; it's just confusing.
[BUG]
/home/yxie/linux-2.5.53/drivers/ide/ide-probe.c:1055:alloc_disks:
ERROR:BUFFER:1055:1055:Array bounds error (off < 0) [RANGE]
(disks[unit], min(off) = -1)
        }
        return 0;
Enomem:
        printk(KERN_WARNING "(ide::init_gendisk) Out of memory\n");
        while (unit--)

Error --->
                put_disk(disks[unit]);
        return -ENOMEM;
}

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Jan 31 2003 - 22:00:18 EST