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]
Date:   Wed, 26 Apr 2023 15:31:39 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
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 3:08 PM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
>
> Is this what you had in mind?
> ```
> void * _Nonnull foo (void)
> ...
> void bar (void) {
>     if (foo() == NULL) // maybe should warn that foo() returns _Nonnull?
>         bar();
> ...
> linus.c:8:15: warning: comparison of _Nonnull function call 'foo'
> equal to a null pointer is always false

Yes.

HOWEVER.

I suspect you will find that it gets complicated for more indirect
uses, and that may be why people have punted on this.

For example, let's say that you instead have

   void *bar(void) { return foo(); }

and 'bar()' gets inlined.

The obvious reaction to that is "ok, clearly the result is still
_Nonnull, and should warn if it is tested.

But that obvious reaction is actually completely wrong, because it may
be that the real code looks something like

   void *bar(void) {
#if CONFIG_XYZ
    if (somecondition) return NULL;
#endif
    return foo(); }

and the caller really *should* check for NULL - it's just that the
compiler never saw that case.

So only testing the direct return value of a function should warn.

And even that "direct return value" is not trivial. What happens if
you have something like this:

   void bar(void) { do_something(foo()); }

and "do_something()" ends up being inlined - and checks for its
argument for being NULL? Again, that "test against NULL" may well be
absolutely required in that context - because *other* call-sites will
pass in pointers that might be NULL.

Now, I don't know how clang works internally, but I suspect based just
on the size of your patch that your patch would get all of this
horribly wrong.

So doing a naked

    void *ptr = foo();
    if (!ptr) ...

should warn.

But doing the exact same thing, except the test for NULL came in some
other context that just got inlined, cannot warn.

I _suspect_ that the reason clang does what it does is that this is
just very complicated to do well.

It sounds easy to a human, but ...

          Linus

Powered by blists - more mailing lists