linux-kernel - Re: [PATCH v2 1/2] bitmap: Modify bitmap_set

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] [day] [month] [year] [list]

Message-ID: <CAHp75Vef2WDjD=DkoTV13Akk9yxwbF1oJH-=O4Kg_uxtY1qOdQ@mail.gmail.com>
Date:   Sun, 13 Dec 2020 16:55:56 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Syed Nayyar Waris <syednwaris@...il.com>
Cc:     Linus Walleij <linus.walleij@...aro.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        William Breathitt Gray <vilhelm.gray@...il.com>,
        Michal Simek <michal.simek@...inx.com>,
        Arnd Bergmann <arnd@...db.de>,
        Robert Richter <rrichter@...vell.com>,
        Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        "Zhang, Rui" <rui.zhang@...el.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amit.kucheria@...durent.com>,
        Linux-Arch <linux-arch@...r.kernel.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-arm Mailing List <linux-arm-kernel@...ts.infradead.org>,
        Linux PM <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] bitmap: Modify bitmap_set_value() to check bitmap length

On Sun, Dec 13, 2020 at 4:24 PM Syed Nayyar Waris <syednwaris@...il.com> wrote:
>
> Add explicit check to see if the value being written into the bitmap
> does not fall outside the bitmap.
> The situation that it is falling outside would never be possible in the
> code because the boundaries are required to be correct before the
> function is called. The responsibility is on the caller for ensuring the
> boundaries are correct.
> The code change is simply to silence the GCC warning messages
> because GCC is not aware that the boundaries have already been checked.
> As such, we're better off using __builtin_unreachable() here because we
> can avoid the latency of the conditional check entirely.
>
> Cc: Arnd Bergmann <arnd@...db.de>
> Signed-off-by: Syed Nayyar Waris <syednwaris@...il.com>
> Acked-by: William Breathitt Gray <vilhelm.gray@...il.com>
>
> lib/test_bitmap.c: Modify for_each_set_clump test
>
> Modify the test where bitmap_set_value() is called. bitmap_set_value()
> now takes an extra bitmap-width as second argument and the width of
> value is now present as the fourth argument.
>
> Signed-off-by: Syed Nayyar Waris <syednwaris@...il.com>
>
> gpio: xilinx: Modify bitmap_set_value() calls
>
> Modify the bitmap_set_value() calls. bitmap_set_value()
> now takes an extra bitmap width as second argument and the width of
> value is now present as the fourth argument.
>
> Cc: Michal Simek <michal.simek@...inx.com>
> Signed-off-by: Syed Nayyar Waris <syednwaris@...il.com>

Commit message here definitely needs more work.
First of all, it's now quite awkward to see this squashed stuff like this.
Second, it misses the warning examples it's talking about.
Third, it repeats some tags.
Fourth, it misses the Fixes tag.

Please, redone it correctly (one commit message with clear purpose and
example of warnings followed by Fixes tag) and resend a v3.

You may mention in the cover letter that this is squashed of three
patches from v1 (and give a link to lore.kernel.org).

-- 
With Best Regards,
Andy Shevchenko