netdev - Re: [PATCH net v2] net: Properly typecast int values to set sk_max_pacing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <818BA4CC-AAA9-4D4F-9EF4-438405DA1020@akamai.com>
Date:   Thu, 22 Oct 2020 17:45:08 +0000
From:   "Li, Ke" <keli@...mai.com>
To:     Eric Dumazet <edumazet@...gle.com>
CC:     netdev <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        "Jakub Kicinski" <kuba@...nel.org>, "kli@...l.edu" <kli@...l.edu>,
        "Li, Ji" <jli@...mai.com>
Subject: Re: [PATCH net v2] net: Properly typecast int values to set
 sk_max_pacing_rate

Thanks for the info, Eric!

That explains why checkpatch.pl hadn't complained with v1 patch.

Best,
-Ke

On 10/22/20, 7:19 AM, "Eric Dumazet" <edumazet@...gle.com> wrote:

    On Thu, Oct 22, 2020 at 10:43 AM Li, Ke <keli@...mai.com> wrote:
    >
    > Thank you, Eric!
    >
    > Nice to know the recent change to wrap-at-100. Will this be reflected somewhere, like, in Documentation/process/coding-style.rst?
    >

    commit bdc48fa11e46f867ea4d75fa59ee87a7f48be144
    Author: Joe Perches <joe@...ches.com>
    Date:   Fri May 29 16:12:21 2020 -0700

        checkpatch/coding-style: deprecate 80-column warning

        Yes, staying withing 80 columns is certainly still _preferred_.  But
        it's not the hard limit that the checkpatch warnings imply, and other
        concerns can most certainly dominate.

        Increase the default limit to 100 characters.  Not because 100
        characters is some hard limit either, but that's certainly a "what are
        you doing" kind of value and less likely to be about the occasional
        slightly longer lines.

        Miscellanea:

         - to avoid unnecessary whitespace changes in files, checkpatch will no
           longer emit a warning about line length when scanning files unless
           --strict is also used

         - Add a bit to coding-style about alignment to open parenthesis

        Signed-off-by: Joe Perches <joe@...ches.com>
        Signed-off-by: Linus Torvalds <torvalds@...ux-foundation.org>



    > Best,
    > -Ke
    >
    > On 10/22/20, 12:49 AM, "Eric Dumazet" <edumazet@...gle.com> wrote:
    >
    >     On Thu, Oct 22, 2020 at 8:42 AM Ke Li <keli@...mai.com> wrote:
    >     >
    >     > In setsockopt(SO_MAX_PACING_RATE) on 64bit systems, sk_max_pacing_rate,
    >     > after extended from 'u32' to 'unsigned long', takes unintentionally
    >     > hiked value whenever assigned from an 'int' value with MSB=1, due to
    >     > binary sign extension in promoting s32 to u64, e.g. 0x80000000 becomes
    >     > 0xFFFFFFFF80000000.
    >     >
    >     > Thus inflated sk_max_pacing_rate causes subsequent getsockopt to return
    >     > ~0U unexpectedly. It may also result in increased pacing rate.
    >     >
    >     > Fix by explicitly casting the 'int' value to 'unsigned int' before
    >     > assigning it to sk_max_pacing_rate, for zero extension to happen.
    >     >
    >     > Fixes: 76a9ebe811fb ("net: extend sk_pacing_rate to unsigned long")
    >     > Signed-off-by: Ji Li <jli@...mai.com>
    >     > Signed-off-by: Ke Li <keli@...mai.com>
    >     > Cc: Eric Dumazet <edumazet@...gle.com>
    >     > ---
    >     > v2: wrap the line in net/core/filter.c to less than 80 chars.
    >
    >     SGTM (the other version was also fine, the 80 chars rule has been
    >     relaxed/changed to 100 recently)
    >
    >     Reviewed-by: Eric Dumazet <edumazet@...gle.com>
    >