On Mon, Sep 29, 2014 at 4:48 AM, Michael S. Tsirkin <[email protected]> wrote: > On Sun, Sep 28, 2014 at 04:27:53PM -0700, Kees Cook wrote: >> 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 <[email protected]> > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
