linux-kernel - Re: [GIT PULL] ext4 changes for the 6.4 merge window

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: <CAKwvOdm3gkAufWcWBqDMQNRXVqJjooFQ4Bi5YPHndWFCPScG+g@mail.gmail.com>
Date:   Wed, 26 Apr 2023 11:22:41 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     "Theodore Ts'o" <tytso@....edu>,
        Nathan Chancellor <nathan@...nel.org>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-mm@...ck.org
Subject: Re: [GIT PULL] ext4 changes for the 6.4 merge window

On Wed, Apr 26, 2023 at 11:11 AM Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
>
> On Wed, Apr 26, 2023 at 10:34 AM Nick Desaulniers
> <ndesaulniers@...gle.com> wrote:
> >
> > That's what clang's _Nonnull attribute does (with -Wnullability-extension).
>
> No, that's a warning about using it, not a warning about testing for
> NULL when it's there.
>
> I actually tested _Nonnull.  It seems to work for arguments. But it
> does not work for return values.

Ah, it does do something in the callee, not the caller:
https://godbolt.org/z/9dsPKGMWq

But I see your point; it would be nice to flag that the comparison
against NULL seems suspicious.

>
> Of course, maybe there's some other magic needed, but it does seem to
> be sadly not working for us.
>
> > But it's not toolchain portable, at the moment.  Would require changes
> > to clang to use the GNU C __attribute__ syntax, too (which I'm not
> > against adding support for).
>
> No need for using the __attribute__ syntax at all, that would _not_ be
> a show-stopper.

Ack.

>
> While it's true that it's the common syntax, and we sometimes use it
> explicitly because of that, it's by no means the only syntax, and we
> actually tend to try to have more legible wrappers around it.
>
> So, for example, we prefer using '__weak' instead of writing
> '__attribute__((__weak__))'.
>
> And no, it very much doesn't have to use __attibute__ at all. We
> already have things like
>
>     #define __diag(s)          _Pragma(__diag_str(GCC diagnostic s))
>
> so we already use other syntaxes.
>
> End result: if it actually worked, I'd happily do something like
>
>    #define __return_nonnull _Nonnull
>
> in <linux/compiler-clang.h>, with then <linux/compiler-gcc.h> then just having
>
>      #define __return_nonnull
>
> along with a big comment about how __attribute__((nonnull)) is
> horrible garbage that should never every be used.
>
>              Linus



-- 
Thanks,
~Nick Desaulniers