lkml.org 
[lkml]   [2016]   [Aug]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 2/2] mm/usercopy: enable usercopy size checking for modern versions of gcc
    On Tue, Aug 23, 2016 at 10:37:43PM -0400, Kees Cook wrote:
    > On Tue, Aug 23, 2016 at 3:28 PM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
    > > This is a revert of:
    > >
    > > 2fb0815c9ee6 ("gcc4: disable __compiletime_object_size for GCC 4.6+")
    > >
    > > The goal of that commit was to silence the "provably correct" gcc
    > > warnings. But it went too far: it also disabled the runtime warnings.
    > >
    > > Now that the pretty much useless gcc warnings have been properly
    > > disposed of with the previous patch, re-enable this checking on modern
    > > versions of gcc so we can get the runtime warnings again.
    >
    > As far as I know, this will still be broken since it's
    > __builtin_object_size() that is buggy. Maybe I'm misunderstanding
    > which piece is busted, though?

    What specifically is buggy with __builtin_object_size()? Looking at the
    generated code for a few of the "provably correct" warning sites, the
    values generated by __builtin_object_size() are correct.

    I think the problem is really related to the compile-time warning
    function attribute used by __copy_to_user_overflow(). The warning is
    printed when gcc *can* determine the object size but it *can't*
    determine the copy size. The warning just means that, even though the
    object has a const size, gcc isn't able to prove that the overflow won't
    happen.

    As an example, here's one of the warnings:

    In file included from /home/jpoimboe/git/linux/include/linux/uaccess.h:5:0,
    from /home/jpoimboe/git/linux/arch/x86/include/asm/stacktrace.h:9,
    from /home/jpoimboe/git/linux/arch/x86/include/asm/perf_event.h:246,
    from /home/jpoimboe/git/linux/include/linux/perf_event.h:24,
    from /home/jpoimboe/git/linux/kernel/sys.c:16:
    In function ‘copy_to_user.part.10’,
    inlined from ‘copy_to_user’,
    inlined from ‘override_release.part.11’ at /home/jpoimboe/git/linux/kernel/sys.c:1136:9:
    /home/jpoimboe/git/linux/arch/x86/include/asm/uaccess.h:723:46: warning: call to ‘__copy_to_user_overflow’ declared with attribute warning: copy_to_user() buffer size is not provably correct
    #define __copy_to_user_overflow(size, count) __copy_to_user_overflow()
    ^~~~~~~~~~~~~~~~~~~~~~~~~
    /home/jpoimboe/git/linux/arch/x86/include/asm/uaccess.h:791:3: note: in expansion of macro ‘__copy_to_user_overflow’
    __copy_to_user_overflow(sz, n);
    ^~~~~~~~~~~~~~~~~~~~~~~

    This is from override_release()'s use of copy_to_user(). The object
    code shows that __builtin_object_size() correctly reports 65 bytes for
    the 'buf' object size. But the copy size ('copy + 1') isn't known at
    compile-time. Thus the (bogus) warning.

    Maybe I'm missing something but I don't even see a gcc bug. To me it
    looks like a mismatch in expectations between the code and the compiler.

    --
    Josh

    \
     
     \ /
      Last update: 2016-09-17 09:57    [W:2.276 / U:0.364 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site