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

Yichen Xie
2003-01-28T09:12:31Z

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;
}