linux-kernel - Re: [GIT PULL] pid: use flex array

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-=whJJbmfBk_8v_vFn1NdJ9O-AKCrjY+EArkzgFp9h-sKHA@mail.gmail.com>
Date:   Fri, 30 Jun 2023 00:12:22 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Christian Brauner <brauner@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [GIT PULL] pid: use flex array

On Thu, 29 Jun 2023 at 23:51, Christian Brauner <brauner@...nel.org> wrote:
>
> I have no preference for either syntax. Both work. But this is probably
> more an objection to this being mixed in with the flex array change in
> the first place.

Yes. I looked at it, and tried to figure out if it was related
somehow, and decided that no, it can't possibly be, and must be just
an unrelated change.

> I did react to that in the original review here:
> https://lore.kernel.org/all/20230518-zuneigen-brombeeren-0a57cd32b1a7@brauner
> but then I grepped for it and saw it done in a few other places already

Yeah, we do end up growing new uses of 'use 0 as a pointer' almost as
quickly as we get rid of them.

We got rid of a couple just recently in commit dadeeffbe525 ("fbdev:
hitfb: Use NULL for pointers"), but yes, a quick

    git grep '\*)0\>'

shows many more.

And some of them are even ok. I don't think it's always wrong,
particularly if you then abstract it out.

So doing something like that

   #define PCI_IOBASE ((void __iomem *)0)

makes perfect sense. It's literally abstracting out something real (in
this case yes, it looks like a NULL pointer, but it's actually a
pointer with a strict type that just happens to have the value zero.

So that "NULL pointer with a type" concept makes sense, but it really
should be abstracted out, not be in the middle of some random code.

And that's *particularly* true when we already have the exact
abstraction for the situation that the code then uses (ie in this case
that "struct_size_t()" thing). Writing it out - in an ugly form -
using the disgusting traditional "constant integer zero can be cast to
any pointer" thing - just makes me go "Eugghhh!".

I mean, if this ugly part was just a small detail in a l;arger patch,
I probably would just have let it slide. It works. It is what it is.

But when three quarters of the patch was stuff I found questionable, I
just couldn't stomach it.

I'm looking at some of the grep hits, and I'm going "ok, that's a C++
programmer". I think there's a very real reason why so many of them
are in bpf code and in the bpf tests.

C++ made a *huge* fundamental design mistake early on wrt NULL, and a
generation of C++ programmers were poisoned by it and you had people
who swore until they were blue that 0 and NULL had to be the same
thing.

They were horribly and utterly wrong. But sometimes when it takes you
three decades to admit that you were wrong all that time, it's just
too painful to admit.

There are literally people who still can't admit to their mistake, and
refuse to use NULL, and use either 0 or 'nullptr'.

                  Linus