Hi,
On Sun, 26 Jan 2003, Yichen Xie wrote:
| Attached are 87 potential buffer overruns in 2.5.53. Most arise from
| improper bounds checks, and some might be security holes where the array
| index comes from an untrusted source (e.g. copy_from_user). In the bug
| report, "len" refers to the length of the array or buffer being
| accessed, and "off" refers to the offset/index that is being used to
| access it. (off >= len) corresponds to a buffer overrun, while (off < 0)
| signals an underrun.
|
| As always, confirmations and comments will be appreciated.
| ############################################################
This stuff is just too good to be ignored. Hopefully someone else
is also looking at it.
I've gone thru most of these (the ones that I could read :) and have
generated 37 patches that I will post after some sleep. Here are
some comments on the ones that I'm _not_ patching.
My comments being with '#'.
Thanks,
-- ~Randy--------------------------------------------------------- # Comment in struct says that its size there is a placeholder........ [BUG] cmd array has size 2 (might be false positive, if cmd is used as a buffer) /home/yxie/linux-2.5.53/drivers/char/ip2/i2cmd.c:205:i2cmdUnixFlags: ERROR:BUFFER:205:205:Array bounds error (off >= len) ((*pCM).cmd[2], len = 2, off = 2, min(off-len) = 0) i2cmdUnixFlags(unsigned short iflag,unsigned short cflag,unsigned short lflag) { cmdSyntaxPtr pCM = (cmdSyntaxPtr) ct47;
pCM->cmd[1] = (unsigned char) iflag;
Error ---> 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; --------------------------------------------------------- # Yes, looks wrong. [BUG] /home/yxie/linux-2.5.53/drivers/message/fusion/mptbase.c:4939:procmpt_cr eate: ERROR:BUFFER:4939:4939:Array bounds error (off >= len) (mpt_ioc_proc_list[ii], len = 2, off = 2, min(off-len) = 0) } ent->read_proc = mpt_ioc_proc_list[ii].f; ent->data = ioc; } } else {
Error ---> printk(MYIOC_s_WARN_FMT "Could not create /proc/mpt/%s subdir entry!\n", ioc->name, mpt_ioc_proc_list[ii].name); } ioc = mpt_adapter_find_next(ioc); --------------------------------------------------------- # Yes, looks wrong. [BUG] need one more element in ZeroAddr /home/yxie/linux-2.5.53/drivers/net/sk98lin/skxmac2.c:424:SkXmClrSrcChec k: ERROR:BUFFER:424:424:Array bounds error (off >= len) (*pByte + 6, len = 6, off = 6, min(off-len) = 0) SK_IOC IoC, /* IO context */ int Port) /* The XMAC to handle with belongs to this Port (MAC_1 + n) */ { SK_U16 ZeroAddr[3] = {0x0000, 0x0000, 0x0000};
Error ---> XM_OUTHASH(IoC, Port, XM_SRC_CHK, &ZeroAddr); } /* SkXmClrSrcCheck */ --------------------------------------------------------- # 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); --------------------------------------------------------- # Greg: Looks like all [count] in that loop should be [i].... [BUG] /home/yxie/linux-2.5.53/drivers/hotplug/ibmphp_pci.c:1630:ibmphp_unconfi gure_card: ERROR:BUFFER:1630:1630:Array bounds error (off >= len) ((*cur_func).mem[count], len = 6, off = 6, min(off-len) = 0) debug ("io[%d] exists \n", count); if (the_end > 0) ibmphp_remove_resource (cur_func->io[count]); cur_func->io[count] = NULL; }
Error ---> if (cur_func->mem[count]) { debug ("mem[%d] exists \n", count); if (the_end > 0) ibmphp_remove_resource (cur_func->mem[count]); --------------------------------------------------------- # Greg: Looks like all [count] in that loop should be [i].... [BUG] i or count? /home/yxie/linux-2.5.53/drivers/hotplug/ibmphp_pci.c:1624:ibmphp_unconfi gure_card: ERROR:BUFFER:1624:1624:Array bounds error (off >= len) ((*cur_func).io[count], len = 6, off = 6, min(off-len) = 0) } else { count = 6; }
for (i = 0; i < count; i++) {
Error ---> if (cur_func->io[count]) { debug ("io[%d] exists \n", count); if (the_end > 0) ibmphp_remove_resource (cur_func->io[count]); --------------------------------------------------------- # Vojtech: looks wrong. [BUG] /home/yxie/linux-2.5.53/drivers/input/keyboard/sunkbd.c:271:sunkbd_conne ct: ERROR:BUFFER:271:271:Array bounds error (off >= len) [RANGE] ((*sunkbd).keycode[i], len = 128, off = sym_44382539, max(off-len) = 126)
sprintf(sunkbd->name, "Sun Type %d keyboard", sunkbd->type);
memcpy(sunkbd->keycode, sunkbd_keycode, sizeof(sunkbd->keycode)); for (i = 0; i < 255; i++)
Error ---> set_bit(sunkbd->keycode[i], sunkbd->dev.keybit); clear_bit(0, sunkbd->dev.keybit);
sprintf(sunkbd->name, "%s/input", serio->phys); --------------------------------------------------------- # I'm guessing that this is what the SCSI command requests.... [BUG] length of vendorId is 8 /home/yxie/linux-2.5.53/drivers/scsi/psi240i.c:334:Irq_Handler: ERROR:BUFFER:334:334:Array bounds error (off >= len) ((*pinquiryData).VendorId[z], len = 8, off = 8, min(off-len) = 0) pinquiryData->AdditionalLength = 35 - 4;
// Fill in vendor identification fields. for ( z = 0; z < 20; z += 2 ) {
Error ---> pinquiryData->VendorId[z] = ((UCHAR *)identifyData.ModelNumber)[z + 1]; pinquiryData->VendorId[z + 1] = ((UCHAR *)identifyData.ModelNumber)[z]; }
--------------------------------------------------------- # Comment in struct says that its size there is a placeholder........ [BUG] /home/yxie/linux-2.5.53/drivers/char/ip2/i2cmd.c:207:i2cmdUnixFlags: ERROR:BUFFER:207:207:Array bounds error (off >= len) ((*pCM).cmd[4], len = 2, off = 4, min(off-len) = 2) cmdSyntaxPtr pCM = (cmdSyntaxPtr) ct47;
pCM->cmd[1] = (unsigned char) iflag; pCM->cmd[2] = (unsigned char) (iflag >> 8); pCM->cmd[3] = (unsigned char) cflag;
Error ---> pCM->cmd[4] = (unsigned char) (cflag >> 8); pCM->cmd[5] = (unsigned char) lflag; pCM->cmd[6] = (unsigned char) (lflag >> 8); return pCM; --------------------------------------------------------- # 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]); } --------------------------------------------------------- # Comment in struct says that its size there is a placeholder........ [BUG] suspicious /home/yxie/linux-2.5.53/drivers/char/ip2/i2cmd.c:261:i2cmdBaudDef: ERROR:BUFFER:261:261:Array bounds error (off >= len) ((*pCM).cmd[2], len = 2, off = 2, min(off-len) = 0) case 2: pCM = (cmdSyntaxPtr) ct55; break; } pCM->cmd[1] = (unsigned char) rate;
Error ---> pCM->cmd[2] = (unsigned char) (rate >> 8); return pCM; }
--------------------------------------------------------- # Needs work but not panic. [BUG] should panic after printk /home/yxie/linux-2.5.53/drivers/scsi/qlogicfc.c:926:isp2x00_make_portdb: ERROR:BUFFER:926:926:Array bounds error (off >= len) ((*hostdata).port_db[j], len = 256, off = 256, min(off-len) = 0) break; } } if (j == QLOGICFC_MAX_ID + 1) printk("qlogicfc%d : Too many scsi devices, no more room in port map.\n", hostdata->host_id);
Error ---> if (!hostdata->port_db[j].wwn) { hostdata->port_db[j].loop_id = temp[i].loop_id; hostdata->port_db[j].wwn = temp[i].wwn; } --------------------------------------------------------- # Already changed/modified/different in 2.5.59. [BUG] /home/yxie/linux-2.5.53/arch/i386/kernel/cpu/cpufreq/powernow-k6.c:202:p owernow_k6_setpolicy: ERROR:BUFFER:202:202:Array bounds error (off >= len) (clock_ratio[i], len = 8, off = 8, min(off-len) = 0) break; default: return -EINVAL; }
Error ---> if (clock_ratio[i] > max_multiplier) return -EINVAL;
powernow_k6_set_state(j); --------------------------------------------------------- # 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; }
--------------------------------------------------------- # I'm guessing that this is what the SCSI command requests.... [BUG] length of VendorId is only 8 /home/yxie/linux-2.5.53/drivers/scsi/psi240i.c:335:Irq_Handler: ERROR:BUFFER:335:335:Array bounds error (off >= len) ((*pinquiryData).VendorId[z + 1], len = 8, off = 9, min(off-len) = 1)
// Fill in vendor identification fields. for ( z = 0; z < 20; z += 2 ) { pinquiryData->VendorId[z] = ((UCHAR *)identifyData.ModelNumber)[z + 1];
Error ---> pinquiryData->VendorId[z + 1] = ((UCHAR *)identifyData.ModelNumber)[z]; }
// Initialize unused portion of product id. --------------------------------------------------------- # 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);
--------------------------------------------------------- # Comment in struct says that its size there is a placeholder........ [BUG] suspicious, but probably not bug /home/yxie/linux-2.5.53/drivers/char/ip2/i2cmd.c:206:i2cmdUnixFlags: ERROR:BUFFER:206:206:Array bounds error (off >= len) ((*pCM).cmd[3], len = 2, off = 3, min(off-len) = 1) { cmdSyntaxPtr pCM = (cmdSyntaxPtr) ct47;
pCM->cmd[1] = (unsigned char) iflag; pCM->cmd[2] = (unsigned char) (iflag >> 8);
Error ---> pCM->cmd[3] = (unsigned char) cflag; pCM->cmd[4] = (unsigned char) (cflag >> 8); pCM->cmd[5] = (unsigned char) lflag; pCM->cmd[6] = (unsigned char) (lflag >> 8); --------------------------------------------------------- # 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; }
--------------------------------------------------------- # Kai: OR s/bar/i/ on 2 lines ?? [BUG] bar == 4? (from the loop above) /home/yxie/linux-2.5.53/drivers/isdn/hardware/eicon/os_4bri.c:268:diva_4 bri_init_card: ERROR:BUFFER:268:268:Array bounds error (off >= len) ((*a).slave_adapters[bar], len = 3, off = (sym_17752159, 1, 1, 1), min(off-len) = 1) */ quadro_list = (PADAPTER_LIST_ENTRY) diva_os_malloc(0, sizeof(*quadro_list)); if (!(a->slave_list = quadro_list)) { for (i = 0; i < (tasks - 1); i++) {
Error ---> diva_os_free(0, a->slave_adapters[bar]); a->slave_adapters[bar] = 0; } diva_4bri_cleanup_adapter(a); --------------------------------------------------------- # Yes, problem. [BUG] base = &buf[4] /home/yxie/linux-2.5.53/drivers/cdrom/cdrom.c:1170:dvd_read_physical: ERROR:BUFFER:1170:1170:Array bounds error (off >= len) (*base + 16, len = 20, off = 20, min(off-len) = 0) layer->track_density = base[3] & 0xf; layer->linear_density = base[3] >> 4; layer->start_sector = base[5] << 16 | base[6] << 8 | base[7]; layer->end_sector = base[9] << 16 | base[10] << 8 | base[11]; layer->end_sector_l0 = base[13] << 16 | base[14] << 8 | base[15];
Error ---> layer->bca = base[16] >> 7;
return 0; } --------------------------------------------------------- # Yes, problem. [BUG] /home/yxie/linux-2.5.53/fs/cifs/cifssmb.c:233:CIFSSMBLogoff: ERROR:BUFFER:233:233:Dereferencing uninitialized pointer (*pSMB) evaluated in the following state if (atomic_read(&ses->inUse) > 0) { up(&ses->sesSem); return -EBUSY; } if(ses->secMode & (SECMODE_SIGN_REQUIRED | SECMODE_SIGN_ENABLED))
Error ---> pSMB->hdr.Flags2 |= SMBFLG2_SECURITY_SIGNATURE; rc = smb_init(SMB_COM_LOGOFF_ANDX, 2, 0 /* no tcon anymore */ , (void **) &pSMB, (void **) &smb_buffer_response); if (rc) { --------------------------------------------------------- # Maybe.... [BUG] check default branch above /home/yxie/linux-2.5.53/drivers/video/sis/sis_main.c:398:sisfb_validate_ mode: ERROR:BUFFER:398:398:Array bounds error (off < 0) (sisbios_mode[sisfb_mode_idx], max(off) = -1) } break; }
if(ivideo.chip < SIS_315H) {
Error ---> if(sisbios_mode[sisfb_mode_idx].xres > 1920) sisfb_mode_idx = -1; } /* TW: TODO: Validate modes available on either 300 or 310/325 series only */ --------------------------------------------------------- # Yes, problem.... [BUG] minor (recommend inserting break in the true branch of the if statement) /home/yxie/linux-2.5.53/drivers/video/sis/init301.c:2053:SiS_SetCRT2FIFO _310: ERROR:BUFFER:2053:2053:Array bounds error (off >= len) [RANGE] (CombCode[(int)temp3 + 1], len = 16, off = (sym_11686423, 1, 3, 1), max(off-len) = 0) /* get DRAM latency */ tempcl = (SiS_GetReg1(SiS_P3c4,0x17) >> 3) & 0x7; /* SR17[5:3] DRAM Queue depth */ tempch = (SiS_GetReg1(SiS_P3c4,0x17) >> 6) & 0x3; /* SR17[7:6] DRAM Grant length */
for (temp3 = 0; temp3 < 16; temp3 += 2) {
Error ---> if ((CombCode[temp3] == tempcl) && (CombCode[temp3+1] == tempch)) { temp3 = CRT2ThLow[temp3 >> 1]; } } --------------------------------------------------------- # Yes, looks wrong. [BUG] /home/yxie/linux-2.5.53/drivers/net/sk98lin/skxmac2.c:424:SkXmClrSrcChec k: ERROR:BUFFER:424:424:Array bounds error (off >= len) (*pByte + 7, len = 6, off = 7, min(off-len) = 1) SK_IOC IoC, /* IO context */ int Port) /* The XMAC to handle with belongs to this Port (MAC_1 + n) */ { SK_U16 ZeroAddr[3] = {0x0000, 0x0000, 0x0000};
Error ---> XM_OUTHASH(IoC, Port, XM_SRC_CHK, &ZeroAddr); } /* SkXmClrSrcCheck */
--------------------------------------------------------- # Summary for # buffer-specific errors = 87 # /dev/null-specific errors = 0 # Common errors = 0 # Total = 87 -
- 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