Linux kernel nvram Integer Overflow

Keeping my promise to spender, here is an interesting email I just saw in lkml by Michael Buesch. So.. nvram is a classic CMOS/NV-RAM device driver for Linux which can be found at drivers/char/nvram.c. M. Buesch noticed (and also gave a nice PoC code) that the following routine:

static ssize_t nvram_write(struct file *file, const char __user *buf,
size_t count, loff_t *ppos)
{
unsigned char contents[NVRAM_BYTES];
unsigned i = *ppos;
unsigned char *tmp;
int len;

len = (NVRAM_BYTES – i) < count ? (NVRAM_BYTES - i) : count; if (copy_from_user(contents, buf, len)) return -EFAULT; spin_lock_irq(&rtc_lock); if (!__nvram_check_checksum()) goto checksum_err; for (tmp = contents; count-- > 0 && i < NVRAM_BYTES; ++i, ++tmp) __nvram_write_byte(*tmp, i); __nvram_set_checksum(); spin_unlock_irq(&rtc_lock); *ppos = i; return tmp - contents; checksum_err: spin_unlock_irq(&rtc_lock); return -EIO; } [/sourcecode] Can lead to a classic (thanks to Silvio) integer overflow during len calculation. Specifically, len is defined as a signed integer (which I don’t see the reason why) and then it calculates its legnth using:
(NVRAM_BYTES – i) < count ? (NVRAM_BYTES – i) : count
Variable 'i' represents the file offset as you can see from the function's scope where it is defined as loff_t *ppos. Also, count is 100% user controlled variable that represents the requested size in bytes. Since no check is being performed during NVRAM_BYTES – i substration an attacker can overflow 'i' and copy 'count' bytes to 'contents' during the copy_from_user() call. The proposed fix is:

int len;

+ if (i >= NVRAM_BYTES)
+ return -EINVAL;
len = (NVRAM_BYTES – i) < count ? (NVRAM_BYTES - i) : count; if (copy_from_user(contents, buf, len)) return -EFAULT; [/sourcecode] Which checks that 'i' cannot be larger than the size of 'contents' array. In case you wonder... Yes, this is a kernel stack smash but keep in mind that /dev/nvram isn't +rw in most cases. I checked this and it is present at least since 2.6.24 release of the Linux kernel. P.S.: spender, /dev/net/tun is +rw by default? :b