mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [CHECKER] 21 probable missing bounds checks
@ 2004-04-06  0:45 Ken Ashcraft
  2004-04-07 18:47 ` [PATCH/2.6.5] fix for potential integer overflow in zoran driver (was: [CHECKER] 21 probable missing bounds checks) Ronald S. Bultje
  0 siblings, 1 reply; 2+ messages in thread
From: Ken Ashcraft @ 2004-04-06  0:45 UTC (permalink / raw)
  To: linux-kernel, cananian, kanoj, rbultje, laurent.pinchart, ranty,
	linuxusb, bcollins, weihs, support, arjanv
  Cc: mc

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



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2004-04-08  0:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-04-06  0:45 [CHECKER] 21 probable missing bounds checks Ken Ashcraft
2004-04-07 18:47 ` [PATCH/2.6.5] fix for potential integer overflow in zoran driver (was: [CHECKER] 21 probable missing bounds checks) Ronald S. Bultje

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®