linux-kernel - Re: [PATCH] s390: disable -Warray-bounds

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:   Thu, 9 Jun 2022 11:20:02 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     David Howells <dhowells@...hat.com>
Cc:     Dave Chinner <dchinner@...hat.com>,
        Kees Cook <keescook@...omium.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Shawn Guo <shawnguo@...nel.org>,
        Sascha Hauer <s.hauer@...gutronix.de>,
        Fabio Estevam <festevam@...il.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        Heiko Carstens <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev <agordeev@...ux.ibm.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] s390: disable -Warray-bounds

On Thu, Jun 9, 2022 at 7:14 AM David Howells <dhowells@...hat.com> wrote:
>
> Note that Dave Chinner would rather I converted code like:
>
>         struct myfs_inode *myfsinode = xyz;
>         myfsinode->netfs.inode.i_ino = 123;
>
> to something like:
>
>         struct myfs_inode *myfsinode = xyz;
>         struct inode *inode = VFS_I(myfsinode);
>         inode->i_ino = 123;
>
> where the translation is wrapped inside a VFS_I() macro in every filesystem
> and wants this across all filesystems.

What? No. That's absolutely disgusting.

Maybe I'm mis-undestanding.

The usual way filesystems should handle this is that they have their
own inode information that contains a 'struct inode', and then they
have an inline function to go from that generic VFS inode to their one
using "container_of()".

And yeah, maybe they call that container_of() thing MYINODE() or
something, although I think an inline function without the ugly
all-uppercase is right.

But the way they go the other way is literally to just dereference the
inode that they have, ie they just use a

        if (S_ISREG(inode->vfs_inode.i_mode)) ..

kind pattern. There's no reason or excuse to try to "wrap" that, and
it would be a big step backwards to introduce some kind of VFS_I()
macro.

There's also no reason to make that generic. At no point should you
ever go from "random filesystem inode" to "actual generic VFS inode"
in some uncontrolled manner.

But maybe Dave is talking about something else, and I'm missing the point.

            Linus