linux-kernel - Re: [PATCH] netfs: Fix gcc-12 warning by embedding vfs inode in netfs_i

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, 19 May 2022 07:05:49 -0700
From:   Kees Cook <keescook@...omium.org>
To:     David Howells <dhowells@...hat.com>
Cc:     jlayton@...nel.org, Jonathan Corbet <corbet@....net>,
        Eric Van Hensbergen <ericvh@...il.com>,
        Latchesar Ionkov <lucho@...kov.net>,
        Dominique Martinet <asmadeus@...ewreck.org>,
        Christian Schoenebeck <linux_oss@...debyte.com>,
        Marc Dionne <marc.dionne@...istor.com>,
        Xiubo Li <xiubli@...hat.com>,
        Ilya Dryomov <idryomov@...il.com>,
        Steve French <smfrench@...il.com>,
        William Kucharski <william.kucharski@...cle.com>,
        "Matthew Wilcox (Oracle)" <willy@...radead.org>,
        linux-doc@...r.kernel.org, v9fs-developer@...ts.sourceforge.net,
        linux-afs@...ts.infradead.org, ceph-devel@...r.kernel.org,
        linux-cifs@...r.kernel.org, samba-technical@...ts.samba.org,
        linux-fsdevek@...r.kernel.org, linux-hardening@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] netfs: Fix gcc-12 warning by embedding vfs inode in
 netfs_i_context

On Thu, May 19, 2022 at 02:44:28PM +0100, David Howells wrote:
> While randstruct was satisfied with using an open-coded "void *" offset
> cast for the netfs_i_context <-> inode casting, __builtin_object_size() as
> used by FORTIFY_SOURCE was not as easily fooled.  This was causing the
> following complaint[1] from gcc v12:
> 
> In file included from ./include/linux/string.h:253,
>                  from ./include/linux/ceph/ceph_debug.h:7,
>                  from fs/ceph/inode.c:2:
> In function 'fortify_memset_chk',
>     inlined from 'netfs_i_context_init' at ./include/linux/netfs.h:326:2,
>     inlined from 'ceph_alloc_inode' at fs/ceph/inode.c:463:2:
> ./include/linux/fortify-string.h:242:25: warning: call to '__write_overflow_field' declared with attribute warning:
> detected write beyond size of field (1st parameter); maybe use struct_group()? [-Wattribute-warning]
>   242 |                         __write_overflow_field(p_size_field, size);
>       |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Fix this by embedding a struct inode into struct netfs_i_context (which
> should perhaps be renamed to struct netfs_inode).  The struct inode
> vfs_inode fields are then removed from the 9p, afs, ceph and cifs inode
> structs and vfs_inode is then simply changed to "nic.inode" in those
> filesystems.
> 
> Most of the changes were done with:
> 
>   perl -p -i -e 's/vfs_inode/nic.inode/'g \
> 	`git grep -l 'vfs_inode' -- fs/{9p,afs,ceph,cifs}/*.[ch]`
> 
> Kees suggested doing it with a pair structure[2] and a special declarator
> to insert that into the network filesystem's inode wrapper[3], but I think
> it's cleaner to embed it - and then it doesn't matter if struct
> randomisation reorders things.
> 
> Fixes: bc899ee1c898 ("netfs: Add a netfs inode context")
> Reported-by: Jeff Layton <jlayton@...nel.org>
> Signed-off-by: David Howells <dhowells@...hat.com>
> cc: Kees Cook <keescook@...omium.org>
> cc: Jonathan Corbet <corbet@....net>
> cc: Eric Van Hensbergen <ericvh@...il.com>
> cc: Latchesar Ionkov <lucho@...kov.net>
> cc: Dominique Martinet <asmadeus@...ewreck.org>
> cc: Christian Schoenebeck <linux_oss@...debyte.com>
> cc: Marc Dionne <marc.dionne@...istor.com>
> cc: Xiubo Li <xiubli@...hat.com>
> cc: Ilya Dryomov <idryomov@...il.com>
> cc: Steve French <smfrench@...il.com>
> cc: William Kucharski <william.kucharski@...cle.com>
> cc: "Matthew Wilcox (Oracle)" <willy@...radead.org>
> cc: linux-doc@...r.kernel.org
> cc: v9fs-developer@...ts.sourceforge.net
> cc: linux-afs@...ts.infradead.org
> cc: ceph-devel@...r.kernel.org
> cc: linux-cifs@...r.kernel.org
> cc: samba-technical@...ts.samba.org
> cc: linux-fsdevek@...r.kernel.org
> cc: linux-hardening@...r.kernel.org
> Link: https://lore.kernel.org/r/d2ad3a3d7bdd794c6efb562d2f2b655fb67756b9.camel@kernel.org/ [1]
> Link: https://lore.kernel.org/r/20220517210230.864239-1-keescook@chromium.org/ [2]
> Link: https://lore.kernel.org/r/20220518202212.2322058-1-keescook@chromium.org/ [3]

Works for me! :) Thanks for building the alternative.

Reviewed-by: Kees Cook <keescook@...omium.org>

-Kees

-- 
Kees Cook