linux-kernel - Re: [RFC PATCH v1 1/2] list: add type-safer list

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-=wg6PtRx9nBHVnrQkQkczGjXjO0E-oHXPPgPK_H+Nm=Czg@mail.gmail.com>
Date:   Thu, 10 Mar 2022 21:06:59 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Barnabás Pőcze <pobrn@...tonmail.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Xiaomeng Tong <xiam0nd.tong@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Jakob Koschel <jakobkoschel@...il.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [RFC PATCH v1 1/2] list: add type-safer list_head wrapper

On Thu, Mar 10, 2022 at 6:49 PM Barnabás Pőcze <pobrn@...tonmail.com> wrote:
>
> That is indeed a simpler thing to do, and doesn't need `offsetof()` at the
> declaration, but there are places - not many -  where the `list_head` member
> is inside a subobject, for example, so `member` now contains a period.

Ahh, very true. And very annoying. So close, yet so far, and no way I
can see to really deal with that.

And it's not even really all that rare. It may not be the _common_
case, but it's still fairly wide-spread and not some "one or two odd
places" thing.

This grep catches at least a subset of cases:

    git grep '\<list_for_each_entry(.*\.[a-z_0-9]*)'

and it's clearly all over.

As mentioned, I don't think that we have had huge problems with
getting the member name wrong. We do get a fair amount of checking in
that it obviously has to be part of the type we iterate over, and even
if you were to pick the wrong one, the result is a very simple "that
doesn't work".

But it would still undeniably be very nice to have some automatic
build-time checking for it.

Now, some checking could be done by just doing the "reverse" of what
that patch in

  https://lore.kernel.org/all/CAHk-=wiacQM76xec=Hr7cLchVZ8Mo9VDHmXRJzJ_EX4sOsApEA@mail.gmail.com/

does with 'list_traversal_head()', and have a

  #define list_traversal_entry()

that has a similar union with a type that also specifies the list
entry type (with that same "append marker to member name" model, which
still works fine with dots in the middle).

Then at least cross-checking that the type of the iterator matches
with the target list member is trivial, but _if_ a type has multiple
list entries, they often end up referring to the same type.

That very patch with 'struct task_struct' converted is actually an
example of exactly that: the <children,sibling> and <ptraced,
ptrace_entry> pairings have the exact same type tuple (all being
'struct task_struct', of course).

So it would strengthen the typechecking a bit in some cases, but
probably not all that noticeably.

            Linus