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

[Kees Cook]

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@xxxxxxxxxx>
> Signed-off-by: David Howells <dhowells@xxxxxxxxxx>
> cc: Kees Cook <keescook@xxxxxxxxxxxx>
> cc: Jonathan Corbet <corbet@xxxxxxx>
> cc: Eric Van Hensbergen <ericvh@xxxxxxxxx>
> cc: Latchesar Ionkov <lucho@xxxxxxxxxx>
> cc: Dominique Martinet <asmadeus@xxxxxxxxxxxxx>
> cc: Christian Schoenebeck <linux_oss@xxxxxxxxxxxxx>
> cc: Marc Dionne <marc.dionne@xxxxxxxxxxxx>
> cc: Xiubo Li <xiubli@xxxxxxxxxx>
> cc: Ilya Dryomov <idryomov@xxxxxxxxx>
> cc: Steve French <smfrench@xxxxxxxxx>
> cc: William Kucharski <william.kucharski@xxxxxxxxxx>
> cc: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
> cc: linux-doc@xxxxxxxxxxxxxxx
> cc: v9fs-developer@xxxxxxxxxxxxxxxxxxxxx
> cc: linux-afs@xxxxxxxxxxxxxxxxxxx
> cc: ceph-devel@xxxxxxxxxxxxxxx
> cc: linux-cifs@xxxxxxxxxxxxxxx
> cc: samba-technical@xxxxxxxxxxxxxxx
> cc: linux-fsdevek@xxxxxxxxxxxxxxx
> cc: linux-hardening@xxxxxxxxxxxxxxx
> Link: https://lore.kernel.org/r/d2ad3a3d7bdd794c6efb562d2f2b655fb67756b9.camel@xxxxxxxxxx/ [1]
> Link: https://lore.kernel.org/r/20220517210230.864239-1-keescook@xxxxxxxxxxxx/ [2]
> Link: https://lore.kernel.org/r/20220518202212.2322058-1-keescook@xxxxxxxxxxxx/ [3]

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

Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx>

-Kees

-- 
Kees Cook