Re: [PATCH v2 0/2] Lock and Pointer guards

[Linus Torvalds]

On Thu, Jun 8, 2023 at 9:47 AM Kees Cook <keescook@xxxxxxxxxxxx> wrote:
>
> I am a little worried about how (any version so far of) this API could go
> wrong, e.g. if someone uses this and does "return p" instead of "return
> no_free_ptr(p)",

Absolutely. I think the whole "don't always cleanup" is quite
dangerous, and maybe not worth it, but it _is_ a fairly common
pattern.

>     I was hoping we could do
> something like this to the end of automatic_kfree_wrapper():
>
>        *(void **)pp = NULL;

That would be a lovely thing, but as you found out, it fundamentally
cannot work.

The whole point of "cleanup" is that it is done when the variable -
not trh *value* - goes out of scope.

So when you have that

        return var; /* oops, forgot to disable cleanup */

situation, by definition "var" hasn't gone out of scope until _after_
you read the value and return that value, so pretty much by definition
it cannot make a difference to assign something to 'pp' in the cleanup
code.

> The point being, if we can proactively make this hard to shoot ourselves in
> the foot, that would be nice. :)

So the good thing is that things like KASAN would immediately notice,
and since this all happens (presumably) for the success case, it's not
about some unlikely error case.

I also think that -fanalyzer might just catch it (as part of
-Wanalyzer-use-after-free) at compile-time, but I didn't check. So
if/when people start using -fanalyze on the kernel, those things would
be caught early.

So while this "forget to avoid cleanup" is a worry, I think it ends up
likely being one that is fairly easy to avoid with other checks in
place.

Famous last words.

             Linus