Re: [PATCH] tun: make sure interface usage can not overflow

On Mon, Sep 29, 2014 at 4:48 AM, Michael S. Tsirkin [off-list ref] wrote:

On Sun, Sep 28, 2014 at 04:27:53PM -0700, Kees Cook wrote:

quoted

This makes the size argument a const, since it is always populated by
the caller. Additionally double-checks to make sure the copy_from_user
can never overflow, keeping CONFIG_DEBUG_STRICT_USER_COPY_CHECKS happy:

   In function 'copy_from_user',
       inlined from '__tun_chr_ioctl' at drivers/net/tun.c:1871:7:
       ... copy_from_user() buffer size is not provably correct

Signed-off-by: Kees Cook <redacted>

What exactly is the issue here?
__tun_chr_ioctl is called with sizeof(struct compat_ifreq)
or  sizeof (struct ifreq) as the last argument.

Correct. There is no vulnerability here; I am attempting to both make
the code more defensive to future changes, and to keep
CONFIG_DEBUG_STRICT_USER_COPY_CHECKS happy.

So this looks like a false positive, but
CONFIG_DEBUG_STRICT_USER_COPY_CHECKS machinery is supposed
to avoid false positives.

The support in GCC is currently a bit faulty, and it seems that it
didn't notice the two callers were static values, so instead, adding
an explicit test keeps it happy.

On which architecture is this?

This is on x86, but with CONFIG_DEBUG_STRICT_USER_COPY_CHECKS
correctly enabled (gcc after 4.6 broke its ability to correctly
optimize), which I've been playing with trying to get gcc working
again. I sent the patch because it seems like it's a reasonable
defensive change to make.

If you want to look more deeply, there's some background here:
https://code.google.com/p/chromium/issues/detail?id=371036

https://git.kernel.org/cgit/linux/kernel/git/kees/linux.git/commit/?h=gcc-bug&id=92dd7154932d8775a05dfd3de5564124c05a4150

Thanks,

-Kees

-- 
Kees Cook
Chrome OS Security