[CHECKER] 21 probable missing bounds checks

From: Ken Ashcraft
Date: Mon Apr 05 2004 - 19:50:56 EST

Next message: Rusty Russell: "Re: [PATCH] Drop exported symbols list if !modules"
Previous message: Robin Rosenberg: "Re: kernel stack challenge"
Next in thread: Ronald S. Bultje: "[PATCH/2.6.5] fix for potential integer overflow in zoran driver(was: [CHECKER] 21 probable missing bounds checks)"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

This is an update from a similar post on March 27, 2004. If you are a direct recipient of this email, it means that I believe that you are the maintainer for a file containing one of the errors below. I couldn't find maintainers for all of the files, so please forward this message if you know of someone who would care about any of the errors below.

# Total = 21
# BUGs | File Name
3 | /drivers/sbp2.c
2 | /mtd/ich2rom.c
2 | /mtd/pci.c
2 | /drivers/firmware_class.c
2 | /linux-2.6.3/binfmt_misc.c
2 | /mtd/map_funcs.c
1 | /scsi/qla_os.c
1 | /drivers/mem.c
1 | /drivers/csr.c
1 | /drivers/msi.c
1 | /usb/kobil_sct.c
1 | /i386/i8259.c
1 | /media/zoran_procfs.c
1 | /cpu/centaur.c

I found these errors by comparing implementations of the same interface. If functions are assigned to the same function pointer (same field of some struct), I assume that the functions are called from the same context. Therefore, they should treat their incoming parameters similarly. In this case, before using scalar parameters as array indices, the functions should either all perform bounds checks on those scalars or none should perform bounds checks. Any contradiction is an error.

There are 23 reports below. Each report contains first a reference to an EXAMPLE or a place where the parameter is checked. That reference is followed by a COUNTER(example) or a place where the parameter is not checked. After that is a code snippet from the counter example. The type of the function pointer (struct foo.bar) can be found in the COUNTER field: [COUNTER=struct foo.bar-param_num].

I was unsure about the validity of the errors where the function pointer is struct map_info.copy_to/from. The examples did not do a straightforward bounds check.

Your feedback is appreciated!

Thanks,
Ken Ashcraft

---------------------------------------------------------
[BUG] our favorite overflow for kmalloc
/home/kash/interface/linux-2.6.3/arch/i386/kernel/microcode.c:430:microcode_write: NOTE:BOUND: Checking arg len [EXAMPLE=file_operations.write-2]
/home/kash/interface/linux-2.6.3/drivers/char/mem.c:605:kmsg_write: ERROR:BOUND: Not checking arg count [COUNTER=file_operations.write-2] [fit=1] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=44] [counter=1] [z = 0.854981960070962] [fn-z = -4.35889894354067]

tmp = kmalloc(count + 1, GFP_KERNEL);
if (tmp == NULL)
return -ENOMEM;
ret = -EFAULT;

Error --->
if (!copy_from_user(tmp, buf, count)) {
tmp[count] = 0;
ret = printk("%s", tmp);
}
---------------------------------------------------------
[BUG] vmalloc(count + 1) can overflow
/home/kash/interface/linux-2.6.3/drivers/acpi/battery.c:591:acpi_battery_write_alarm: NOTE:BOUND: Checking arg count [EXAMPLE=proc_dir_entry.write_proc-2]
/home/kash/interface/linux-2.6.3/drivers/media/video/zoran_procfs.c:217:zoran_write_proc: ERROR:BOUND: Not checking arg count [COUNTER=proc_dir_entry.write_proc-2] [fit=2] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=13] [counter=1] [z = -0.367883603690978] [fn-z = -4.35889894354067]
KERN_ERR
"%s: write_proc: can not allocate memory\n",
ZR_DEVNAME(zr));
return -ENOMEM;
}

Error --->
if (copy_from_user(string, buffer, count)) {
vfree (string);
return -EFAULT;
}
---------------------------------------------------------
[BUG] overflow
/home/kash/interface/linux-2.6.3/arch/i386/kernel/apm.c:1447:do_read: NOTE:BOUND: Checking arg count [EXAMPLE=file_operations.read-2]
/home/kash/interface/linux-2.6.3/fs/binfmt_misc.c:570:bm_status_read: ERROR:BOUND: Not checking arg nbytes [COUNTER=file_operations.read-2] [fit=3] [fit_fn=2] [fn_ex=0] [fn_counter=1] [ex=40] [counter=3] [z = -0.59475471426012] [fn-z = -4.35889894354067]
return -EINVAL;
if (pos >= len)
return 0;
if (len < pos + nbytes)
nbytes = len - pos;

Error --->
if (copy_to_user(buf, s + pos, nbytes))
return -EFAULT;
*ppos = pos + nbytes;
return nbytes;
---------------------------------------------------------
[BUG] overflow
/home/kash/interface/linux-2.6.3/arch/i386/kernel/apm.c:1447:do_read: NOTE:BOUND: Checking arg count [EXAMPLE=file_operations.read-2]
/home/kash/interface/linux-2.6.3/fs/binfmt_misc.c:449:bm_entry_read: ERROR:BOUND: Not checking arg nbytes [COUNTER=file_operations.read-2] [fit=3] [fit_fn=3] [fn_ex=0] [fn_counter=1] [ex=40] [counter=3] [z = -0.59475471426012] [fn-z = -4.35889894354067]
if (pos >= len)
goto out;
if (len < pos + nbytes)
nbytes = len - pos;
res = -EFAULT;

Error --->
if (copy_to_user(buf, page + pos, nbytes))
goto out;
*ppos = pos + nbytes;
res = nbytes;
---------------------------------------------------------
[BUG] example is questionable: /home/kash/interface/linux-2.6.3/arch/i386/kernel/cpu/mtrr/cyrix.c:39:cyrix_get\_arr:
/home/kash/interface/linux-2.6.3/arch/i386/kernel/cpu/mtrr/amd.c:16:amd_get_mtrr: NOTE:BOUND: Checking arg reg [EXAMPLE=mtrr_ops.get-0]
/home/kash/interface/linux-2.6.3/arch/i386/kernel/cpu/mtrr/centaur.c:54:centaur_get_mcr: ERROR:BOUND: Not checking arg reg [COUNTER=mtrr_ops.get-0] [fit=4] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=3] [counter=1] [z = -1.83532587096449] [fn-z = -4.35889894354067]

static void
centaur_get_mcr(unsigned int reg, unsigned long *base,
unsigned int *size, mtrr_type * type)
{

Error --->
*base = centaur_mcr[reg].high >> PAGE_SHIFT;
*size = -(centaur_mcr[reg].low & 0xfffff000) >> PAGE_SHIFT;
*type = MTRR_TYPE_WRCOMB; /* If it is there, it is write-combining */
if (centaur_mcr_type == 1 && ((centaur_mcr[reg].low & 31) & 2))
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/arch/i386/kernel/io_apic.c:1935:end_level_ioapic_vector: NOTE:BOUND: Checking arg vector [EXAMPLE=hw_interrupt_type.end-0]
/home/kash/interface/linux-2.6.3/arch/i386/kernel/i8259.c:44:end_8259A_irq: ERROR:BOUND: Not checking arg irq [COUNTER=hw_interrupt_type.end-0] [fit=9] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=1] [counter=1] [z = -2.91998558035372] [fn-z = -4.35889894354067]

spinlock_t i8259A_lock = SPIN_LOCK_UNLOCKED;

static void end_8259A_irq (unsigned int irq)
{

Error --->
if (!(irq_desc[irq].status & (IRQ_DISABLED|IRQ_INPROGRESS)) &&
irq_desc[irq].action)
enable_8259A_irq(irq);
}
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/arch/i386/kernel/io_apic.c:1957:set_ioapic_affinity_vector: NOTE:BOUND: Checking arg vector [EXAMPLE=hw_interrupt_type.set_affinity-0]
/home/kash/interface/linux-2.6.3/drivers/pci/msi.c:97:set_msi_affinity: ERROR:BOUND: Not checking arg vector [COUNTER=hw_interrupt_type.set_affinity-0] [fit=7]
[fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=1] [counter=1] [z = -2.91998558035372] [fn-z = -4.35889894354067]
{
struct msi_desc *entry;
struct msg_address address;
unsigned int dest_id;

Error --->
entry = (struct msi_desc *)msi_desc[vector];
if (!entry || !entry->dev)
return;

---------------------------------------------------------
[BUG] maybe
/home/kash/interface/linux-2.6.3/drivers/scsi/qla2xxx/qla_os.c:587:qla2x00_sysfs_write_nvram: NOTE:BOUND: Checking arg count [EXAMPLE=bin_attribute.write-3]
/home/kash/interface/linux-2.6.3/drivers/base/firmware_class.c:201:firmware_data_write: ERROR:BOUND: Not checking arg count [COUNTER=bin_attribute.write-3] [fit=8] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=1] [counter=1] [z = -2.91998558035372] [fn-z = -4.35889894354067]

retval = fw_realloc_buffer(fw_priv, offset + count);
if (retval)
return retval;

Error --->
memcpy(fw->data + offset, buffer, count);

fw->size = max_t(size_t, offset + count, fw->size);

---------------------------------------------------------
[BUG] need lower bound
/home/kash/interface/linux-2.6.3/drivers/usb/serial/cyberjack.c:235:cyberjack_write: NOTE:BOUND: Checking arg count [EXAMPLE=usb_serial_device_type.write-3]
/home/kash/interface/linux-2.6.3/drivers/usb/serial/kobil_sct.c:444:kobil_write: ERROR:BOUND: Not checking arg count [COUNTER=usb_serial_device_type.write-3] [fit=10] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=3] [counter=2] [z = -3.59092423229804] [fn-z = -4.35889894354067]
return -ENOMEM;
}

// Copy data to buffer
if (from_user) {

Error --->
if (copy_from_user(priv->buf + priv->filled, buf, count)) {
return -EFAULT;
}
} else {
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/elan-104nc.c:185:elan_104nc_copy_to: NOTE:BOUND: Checking arg len [EXAMPLE=map_info.copy_to-3]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/ich2rom.c:90:ich2rom_copy_to:
ERROR:BOUND: Not checking arg len [COUNTER=map_info.copy_to-3] [fit=13] [fit_fn=3] [fn_ex=0] [fn_counter=1] [ex=4] [counter=3] [z = -4.59568283847783] [fn-z = -4.35889894354067]
mb();
}

static void ich2rom_copy_to(struct map_info *map, unsigned long to, const void *from, ssize_t len)
{

Error --->
memcpy_toio(addr(map, to), from, len);
}

static struct ich2rom_map_info ich2rom_map = {
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/elan-104nc.c:144:elan_104nc_copy_from: NOTE:BOUND: Checking arg len [EXAMPLE=map_info.copy_from-3]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/pci.c:245:mtd_pci_copyfrom: ERROR:BOUND: Not checking arg len [COUNTER=map_info.copy_from-3] [fit=14] [fit_fn=3] [fn_ex=0] [fn_counter=1] [ex=4] [counter=3] [z = -4.59568283847783] [fn-z = -4.35889894354067]
}

static void mtd_pci_copyfrom(struct map_info *_map, void *to, unsigned long from, ssize_t len)
{
struct map_pci_info *map = (struct map_pci_info *)_map;

Error --->
memcpy_fromio(to, map->base + map->translate(map, from), len);
}

static void mtd_pci_write8(struct map_info *_map, u8 val, unsigned long ofs)
---------------------------------------------------------
[BUG] one example does the copy in chunks
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/elan-104nc.c:185:elan_104nc_copy_to: NOTE:BOUND: Checking arg len [EXAMPLE=map_info.copy_to-3]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/pci.c:272:mtd_pci_copyto: ERROR:BOUND: Not checking arg len [COUNTER=map_info.copy_to-3] [fit=13] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=4] [counter=3] [z = -4.59568283847783] [fn-z = -4.35889894354067]
}

static void mtd_pci_copyto(struct map_info *_map, unsigned long to, const void *from, ssize_t len)
{
struct map_pci_info *map = (struct map_pci_info *)_map;

Error --->
memcpy_toio(map->base + map->translate(map, to), from, len);
}

static struct map_info mtd_pci_map = {
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/elan-104nc.c:144:elan_104nc_copy_from: NOTE:BOUND: Checking arg len [EXAMPLE=map_info.copy_from-3]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/ich2rom.c:67:ich2rom_copy_from: ERROR:BOUND: Not checking arg len [COUNTER=map_info.copy_from-3] [fit=14] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=4] [counter=3] [z = -4.59568283847783] [fn-z = -4.35889894354067]
return __raw_readl(addr(map, ofs));
}

static void ich2rom_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len)
{

Error --->
memcpy_fromio(to, addr(map, from), len);
}

static void ich2rom_write8(struct map_info *map, __u8 d, unsigned long ofs)
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/elan-104nc.c:144:elan_104nc_copy_from: NOTE:BOUND: Checking arg len [EXAMPLE=map_info.copy_from-3]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/map_funcs.c:73:simple_map_copy_from: ERROR:BOUND: Not checking arg len [COUNTER=map_info.copy_from-3] [fit=14] [fit_fn=2] [fn_ex=0] [fn_counter=1] [ex=4] [counter=3] [z = -4.59568283847783]
[fn-z = -4.35889894354067]
#endif /* CFI_B8 */
}

static void simple_map_copy_from(struct map_info *map, void *to, unsigned long from, ssize_t len)
{

Error --->
memcpy_fromio(to, map->virt + from, len);
}

static void simple_map_copy_to(struct map_info *map, unsigned long to, const void *from, ssize_t len)
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/elan-104nc.c:185:elan_104nc_copy_to: NOTE:BOUND: Checking arg len [EXAMPLE=map_info.copy_to-3]
/home/kash/interface/linux-2.6.3/drivers/mtd/maps/map_funcs.c:78:simple_map_copy_to: ERROR:BOUND: Not checking arg len [COUNTER=map_info.copy_to-3] [fit=13] [fit_fn=2] [fn_ex=0] [fn_counter=1] [ex=4] [counter=3] [z = -4.59568283847783] [fn-z = -4.35889894354067]
memcpy_fromio(to, map->virt + from, len);
}

static void simple_map_copy_to(struct map_info *map, unsigned long to, const void *from, ssize_t len)
{

Error --->
memcpy_toio(map->virt + to, from, len);
}

void simple_map_init(struct map_info *map)
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/ieee1394/csr.c:724:write_fcp: NOTE:BOUND: Checking arg length [EXAMPLE=hpsb_address_ops.write-5]
/home/kash/interface/linux-2.6.3/drivers/ieee1394/sbp2.c:1089:sbp2_handle_physdma_write: ERROR:BOUND: Not checking arg length [COUNTER=hpsb_address_ops.write-5] [fit=15] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=1] [counter=2] [z = -4.90076972114066] [fn-z = -4.35889894354067]
{

/*
* Manually put the data in the right place.
*/

Error --->
memcpy(bus_to_virt((u32)addr), data, length);
sbp2util_packet_dump(data, length, "sbp2 phys dma write by device", (u32)addr);
return(RCODE_COMPLETE);
}
---------------------------------------------------------
[BUG]
/home/kash/interface/linux-2.6.3/drivers/ieee1394/csr.c:724:write_fcp: NOTE:BOUND: Checking arg length [EXAMPLE=hpsb_address_ops.write-5]
/home/kash/interface/linux-2.6.3/drivers/ieee1394/sbp2.c:2437:sbp2_handle_status_write: ERROR:BOUND: Not checking arg length [COUNTER=hpsb_address_ops.write-5]
[fit=15] [fit_fn=2] [fn_ex=0] [fn_counter=1] [ex=1] [counter=2] [z = -4.90076972114066] [fn-z = -4.35889894354067]
}

/*
* Put response into scsi_id status fifo...
*/

Error --->
memcpy(&scsi_id->status_block, data, length);

/*
* Byte swap first two quadlets (8 bytes) of status for processing
---------------------------------------------------------
[BUG] example makes sure length is 4
/home/kash/interface/linux-2.6.3/drivers/ieee1394/cmp.c:191:pcr_read: NOTE:BOUND: Checking arg length [EXAMPLE=hpsb_address_ops.read-4]
/home/kash/interface/linux-2.6.3/drivers/ieee1394/sbp2.c:1105:sbp2_handle_physdma_read: ERROR:BOUND: Not checking arg length [COUNTER=hpsb_address_ops.read-4] [fit=16] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=1] [counter=2] [z = -4.90076972114066] [fn-z = -4.35889894354067]
{

/*
* Grab data from memory and send a read response.
*/

Error --->
memcpy(data, bus_to_virt((u32)addr), length);
sbp2util_packet_dump(data, length, "sbp2 phys dma read by device", (u32)addr);
return(RCODE_COMPLETE);
}
---------------------------------------------------------
[BUG] overflow
/home/kash/interface/linux-2.6.3/drivers/ieee1394/cmp.c:191:pcr_read: NOTE:BOUND: Checking arg length [EXAMPLE=hpsb_address_ops.read-4]
/home/kash/interface/linux-2.6.3/drivers/ieee1394/csr.c:271:read_maps: ERROR:BOUND: Not checking arg length [COUNTER=hpsb_address_ops.read-4] [fit=16] [fit_fn=2] [fn_ex=0] [fn_counter=1] [ex=1] [counter=2] [z = -4.90076972114066] [fn-z = -4.35889894354067]
- CSR_TOPOLOGY_MAP;
} else {
src = ((char *)host->csr.speed_map) + csraddr - CSR_SPEED_MAP;
}

Error --->
memcpy(buffer, src, length);
spin_unlock_irqrestore(&host->csr.lock, flags);
return RCODE_COMPLETE;
}
---------------------------------------------------------
[BUG] overflow
/home/kash/interface/linux-2.6.3/drivers/scsi/qla2xxx/qla_os.c:558:qla2x00_sysfs_read_nvram: NOTE:BOUND: Checking arg count [EXAMPLE=bin_attribute.read-3]
/home/kash/interface/linux-2.6.3/drivers/base/firmware_class.c:152:firmware_data_read: ERROR:BOUND: Not checking arg count [COUNTER=bin_attribute.read-3] [fit=17] [fit_fn=1] [fn_ex=0] [fn_counter=1] [ex=1] [counter=2] [z = -4.90076972114066] [fn-z = -4.35889894354067]
if (offset > fw->size)
return 0;
if (offset + count > fw->size)
count = fw->size - offset;

Error --->
memcpy(buffer, fw->data + offset, count);
return count;
}
static int
---------------------------------------------------------
[BUG] overflow
/home/kash/interface/linux-2.6.3/drivers/scsi/qla2xxx/qla_os.c:558:qla2x00_sysfs_read_nvram: NOTE:BOUND: Checking arg count [EXAMPLE=bin_attribute.read-3]
/home/kash/interface/linux-2.6.3/drivers/scsi/qla2xxx/qla_os.c:489:qla2x00_sysfs_read_fw_dump: ERROR:BOUND: Not checking arg count [COUNTER=bin_attribute.read-3] [fit=17] [fit_fn=2] [fn_ex=0] [fn_counter=1] [ex=1] [counter=2] [z = -4.90076972114066] [fn-z = -4.35889894354067]
if (off > ha->fw_dump_buffer_len)
return 0;
if (off + count > ha->fw_dump_buffer_len)
count = ha->fw_dump_buffer_len - off;

Error --->
memcpy(buf, &ha->fw_dump_buffer[off], count);

return (count);
}

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

Next message: Rusty Russell: "Re: [PATCH] Drop exported symbols list if !modules"
Previous message: Robin Rosenberg: "Re: kernel stack challenge"
Next in thread: Ronald S. Bultje: "[PATCH/2.6.5] fix for potential integer overflow in zoran driver(was: [CHECKER] 21 probable missing bounds checks)"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]