linux-kernel - Re: cleanup: Make no_free_ptr() __must

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: <CAHk-=wiOXePAqytCk6JuiP6MeePL6ksDYptE54hmztiGLYihjA@mail.gmail.com>
Date:   Wed, 16 Aug 2023 08:00:17 +0000
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        dan.j.williams@...el.com, linux-kernel@...r.kernel.org,
        Nick Desaulniers <ndesaulniers@...gle.com>
Subject: Re: cleanup: Make no_free_ptr() __must_check

On Wed, 16 Aug 2023 at 07:23, Rasmus Villemoes <linux@...musvillemoes.dk> wrote:
>
> Ah, ok, I thought the purpose was to ensure the p expression gets
> evaluated. Well, we can still do without the temp var and weird comma or
> statement expressions:

Well, the statement expression version with that types temporary
pointer was much better than yours, which just randomly returns 'void
*' and then lets the user assign it to any random pointer with no
warning.

But I think you can add just a 'typeof(p)' cast to it and that should
work out ok. I do absolutely hate how

But all of those macros seem to be fundamentally buggy because 'p'
gets evaluated twice. It could have side effects, even when just
having its address taken.

At that point the whole "comma or statement expression" discussion is
entirely immaterial.

So I think it needs to be something like

  #define __get_and_free_ptr(p) \
      ({ __auto_type __ptr = &(p); \
         __auto_type __val = *__ptr; \
         *__ptr = NULL;  __val; })

to deal with the "assign NULL and return old value without double evaluation".

And then you can have a wrapper macro to do the __must_check part,
something like

  static inline __must_check
  const volatile void * __must_check_fn(const volatile void *val)
  { return val; }

    #define no_free_ptr(p) \
        ((typeof(p)) __must_check_fn(__get_and_free_ptr(p)))

the above is entirely untested. Of course.

               Linus