CVE-2009-3080: Linux kernel GDT ISA/EISA/PCI Disk Array Controllers Signedness Issue
This bug was discovered and reported by Dave Jones of Red Hat. The susceptible code resides in drivers/scsi/gdth.c which has the device driver code for ICP vortex GmbH. GDT ISA/EISA/PCI disk array controllers and Intel storage RAID controllers. Here is the buggy routine as seen in 2.6.31 release of the Linux kernel.
static int gdth_read_event(gdth_ha_str *ha, int handle, gdth_evt_str *estr) { gdth_evt_str *e; int eindex; ulong flags; TRACE2(("gdth_read_event() handle %d\n", handle)); spin_lock_irqsave(&ha->smp_lock, flags); if (handle == -1) eindex = eoldidx; else eindex = handle; estr->event_source = 0; if (eindex >= MAX_EVENTS) { spin_unlock_irqrestore(&ha->smp_lock, flags); return eindex; } e = &ebuffer[eindex]; if (e->event_source != 0) { if (eindex != elastidx) { if (++eindex == MAX_EVENTS) eindex = 0; } else { eindex = -1; } memcpy(estr, e, sizeof(gdth_evt_str)); } spin_unlock_irqrestore(&ha->smp_lock, flags); return eindex; }
As it is implied by its name, this function is used to read the event passed through its third argument that is a pointer to a ‘gdth_evt_str’ which as you can read in drivers/scsi/gdth_ioctl.h is a type definition for a structure that describes the event.
typedef struct { ulong32 first_stamp; ulong32 last_stamp; ushort same_count; ushort event_source; ushort event_idx; unchar application; unchar reserved; gdth_evt_data event_data; } PACKED gdth_evt_str;
Back to gdth_read_event() we can read that ‘eindex’ is initialized with the old index value if the handle passed to that routine by its first argument is -1, otherwise it will be initialized with handle’s value. The next check against ‘MAX_EVENTS’, which is the maximum buffer events and it is declared as 100 in drivers/scsi/gdth.h, can be bypassed since ‘eindex’ is a singed integer and it is perfectably legal to contain negative values which are obviously less than 100 and can also be something else instead of -1.
The subsequent operations that use that value as an index to events’ buffer ‘ebuffer[]’ as well as the final memcpy() from the possibly invalid ‘e’ pointer can result in a memory corruption situation.
This vulnerability was fixed using the patch below.
estr->event_source = 0; - if (eindex >= MAX_EVENTS) { + if (eindex < 0 || eindex >= MAX_EVENTS) { spin_unlock_irqrestore(&ha->smp_lock, flags); return eindex;
In order to check for negative values too.
So, before I end this post have a look at the following function which you’ll probably find useful…
static int ioc_event(void __user *arg) { gdth_ioctl_event evt; gdth_ha_str *ha; ulong flags; if (copy_from_user(&evt, arg, sizeof(gdth_ioctl_event))) ... if (evt.erase == 0xff) { ... } else if (evt.erase == 0) { evt.handle = gdth_read_event(ha, evt.handle, &evt.event); ... if (copy_to_user(arg, &evt, sizeof(gdth_ioctl_event))) return -EFAULT; return 0; }
Leave a Reply