lkml.org 
[lkml]   [2020]   [Aug]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v2] overflow: Add __must_check attribute to check_*() helpers
    From
    Date
    On 17/08/2020 11.08, David Sterba wrote:
    > On Sat, Aug 15, 2020 at 10:09:24AM -0700, Kees Cook wrote:
    >>
    >> +/*
    >> + * Allows for effectively applying __must_check to a macro so we can have
    >> + * both the type-agnostic benefits of the macros while also being able to
    >> + * enforce that the return value is, in fact, checked.
    >> + */
    >> +static inline bool __must_check __must_check_overflow(bool overflow)
    >> +{
    >> + return unlikely(overflow);
    >
    > How does the 'unlikely' hint propagate through return? It is in a static
    > inline so compiler has complete information in order to use it, but I'm
    > curious if it actually does.

    I wondered the same thing, but as I noted in a reply in the v1 thread,
    that pattern is used in kernel/sched/, and the scheduler is a far more
    critical path than anywhere these might be used, so if it's good enough
    for kernel/sched/, it should be good enough here. I have no idea how one
    could write a piece of non-trivial code to see if the hint actually
    makes a difference.

    >
    > In case the hint gets dropped, the fix would probably be
    >
    > #define check_add_overflow(a, b, d) unlikely(__must_check_overflow(({ \
    > typeof(a) __a = (a); \
    > typeof(b) __b = (b); \
    > typeof(d) __d = (d); \
    > (void) (&__a == &__b); \
    > (void) (&__a == __d); \
    > __builtin_add_overflow(__a, __b, __d); \
    > })))
    >

    Well, maybe, but I'd be a little worried that the !! that unlikely()
    slabs on its argument may count as a use of that argument, hence
    nullifying the __must_check which is the main point - the unlikely just
    being something we can add for free while touching this code. Haven't
    tested it, though.

    Rasmus

    \
     
     \ /
      Last update: 2020-08-17 11:35    [W:5.929 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site