linux-kernel - Re: [PATCH] word-at-a-time: use the same return type for has

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: <CAKwvOdk03FoKQ6g09rffj_mtwYHbL15_ctiuBo2Cno6WDvkSkg@mail.gmail.com>
Date:   Tue, 1 Aug 2023 15:45:40 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Arnd Bergmann <arnd@...db.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Nathan Chancellor <nathan@...nel.org>, Tom Rix <trix@...hat.com>,
        linux-arch@...r.kernel.org, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH] word-at-a-time: use the same return type for has_zero
 regardless of endianness

On Tue, Aug 1, 2023 at 3:22 PM <ndesaulniers@...gle.com> wrote:
>
> Compiling big-endian targets with Clang produces the diagnostic:
>
> fs/namei.c:2173:13: warning: use of bitwise '|' with boolean operands
> [-Wbitwise-instead-of-logical]
> } while (!(has_zero(a, &adata, &constants) | has_zero(b, &bdata, &constants)));
>           ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>                                            ||
> fs/namei.c:2173:13: note: cast one or both operands to int to silence
> this warning
>
> It appears that when has_zero was introduced, two definitions were
> produced with different signatures (in particular different return types).
>
> Looking at the usage in hash_name() in fs/namei.c, I suspect that
> has_zero() is meant to be invoked twice per while loop iteration; using
> logical-or would not update `bdata` when `a` did not have zeros. So I
> think it's preferred to always return an unsigned long rather than a
> bool then update the while loop in hash_name() to use a logical-or

s/then/than/

> rather than bitwise-or.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1832
> Fixes: 36126f8f2ed8 ("word-at-a-time: make the interfaces truly generic")
> Debugged-by: Nathan Chancellor <nathan@...nel.org>
> Signed-off-by: Nick Desaulniers <ndesaulniers@...gle.com>
> ---
>  include/asm-generic/word-at-a-time.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/word-at-a-time.h b/include/asm-generic/word-at-a-time.h
> index 20c93f08c993..95a1d214108a 100644
> --- a/include/asm-generic/word-at-a-time.h
> +++ b/include/asm-generic/word-at-a-time.h
> @@ -38,7 +38,7 @@ static inline long find_zero(unsigned long mask)
>         return (mask >> 8) ? byte : byte + 1;
>  }
>
> -static inline bool has_zero(unsigned long val, unsigned long *data, const struct word_at_a_time *c)
> +static inline unsigned long has_zero(unsigned long val, unsigned long *data, const struct word_at_a_time *c)
>  {
>         unsigned long rhs = val | c->low_bits;
>         *data = rhs;
>
> ---
> base-commit: 18b44bc5a67275641fb26f2c54ba7eef80ac5950
> change-id: 20230801-bitwise-7812b11e5fb7
>
> Best regards,
> --
> Nick Desaulniers <ndesaulniers@...gle.com>
>


-- 
Thanks,
~Nick Desaulniers