Commit 874c8ca1 authored by David Howells's avatar David Howells Committed by Linus Torvalds
Browse files

netfs: Fix gcc-12 warning by embedding vfs inode in netfs_i_context



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 "netfs.inode" in those
filesystems.

Further, rename netfs_i_context to netfs_inode, get rid of the
netfs_inode() function that converted a netfs_i_context pointer to an
inode pointer (that can now be done with &ctx->inode) and rename the
netfs_i_context() function to netfs_inode() (which is now a wrapper
around container_of()).

Most of the changes were done with:

  perl -p -i -e 's/vfs_inode/netfs.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.

Dave Chinner suggested using a filesystem-specific VFS_I() function in
each filesystem to convert that filesystem's own inode wrapper struct
into the VFS inode struct[4].

Version #2:
 - Fix a couple of missed name changes due to a disabled cifs option.
 - Rename nfs_i_context to nfs_inode
 - Use "netfs" instead of "nic" as the member name in per-fs inode wrapper
   structs.

[ This also undoes commit 507160f4 ("netfs: gcc-12: temporarily
  disable '-Wattribute-warning' for now") that is no longer needed ]

Fixes: bc899ee1 ("netfs: Add a netfs inode context")
Reported-by: default avatarJeff Layton <jlayton@kernel.org>
Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
Reviewed-by: default avatarJeff Layton <jlayton@kernel.org>
Reviewed-by: default avatarKees Cook <keescook@chromium.org>
Reviewed-by: default avatarXiubo Li <xiubli@redhat.com>
cc: Jonathan Corbet <corbet@lwn.net>
cc: Eric Van Hensbergen <ericvh@gmail.com>
cc: Latchesar Ionkov <lucho@ionkov.net>
cc: Dominique Martinet <asmadeus@codewreck.org>
cc: Christian Schoenebeck <linux_oss@crudebyte.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: Ilya Dryomov <idryomov@gmail.com>
cc: Steve French <smfrench@gmail.com>
cc: William Kucharski <william.kucharski@oracle.com>
cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
cc: Dave Chinner <david@fromorbit.com>
cc: linux-doc@vger.kernel.org
cc: v9fs-developer@lists.sourceforge.net
cc: linux-afs@lists.infradead.org
cc: ceph-devel@vger.kernel.org
cc: linux-cifs@vger.kernel.org
cc: samba-technical@lists.samba.org
cc: linux-fsdevel@vger.kernel.org
cc: linux-hardening@vger.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]
Link: https://lore.kernel.org/r/20220524101205.GI2306852@dread.disaster.area/ [4]
Link: https://lore.kernel.org/r/165296786831.3591209.12111293034669289733.stgit@warthog.procyon.org.uk/ # v1
Link: https://lore.kernel.org/r/165305805651.4094995.7763502506786714216.stgit@warthog.procyon.org.uk

 # v2
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent 3d9f55c5
Loading
Loading
Loading
Loading
+18 −19
Original line number Diff line number Diff line
@@ -37,30 +37,31 @@ The network filesystem helper library needs a place to store a bit of state for
its use on each netfs inode it is helping to manage.  To this end, a context
structure is defined::

	struct netfs_i_context {
	struct netfs_inode {
		struct inode inode;
		const struct netfs_request_ops *ops;
		struct fscache_cookie *cache;
	};

A network filesystem that wants to use netfs lib must place one of these
directly after the VFS ``struct inode`` it allocates, usually as part of its
own struct.  This can be done in a way similar to the following::
A network filesystem that wants to use netfs lib must place one of these in its
inode wrapper struct instead of the VFS ``struct inode``.  This can be done in
a way similar to the following::

	struct my_inode {
		struct {
			/* These must be contiguous */
			struct inode		vfs_inode;
			struct netfs_i_context  netfs_ctx;
		};
		struct netfs_inode netfs; /* Netfslib context and vfs inode */
		...
	};

This allows netfslib to find its state by simple offset from the inode pointer,
thereby allowing the netfslib helper functions to be pointed to directly by the
VFS/VM operation tables.
This allows netfslib to find its state by using ``container_of()`` from the
inode pointer, thereby allowing the netfslib helper functions to be pointed to
directly by the VFS/VM operation tables.

The structure contains the following fields:

 * ``inode``

   The VFS inode structure.

 * ``ops``

   The set of operations provided by the network filesystem to netfslib.
@@ -78,14 +79,12 @@ To help deal with the per-inode context, a number helper functions are
provided.  Firstly, a function to perform basic initialisation on a context and
set the operations table pointer::

	void netfs_i_context_init(struct inode *inode,
	void netfs_inode_init(struct inode *inode,
			      const struct netfs_request_ops *ops);

then two functions to cast between the VFS inode structure and the netfs
context::
then a function to cast from the VFS inode structure to the netfs context::

	struct netfs_i_context *netfs_i_context(struct inode *inode);
	struct inode *netfs_inode(struct netfs_i_context *ctx);
	struct netfs_inode *netfs_node(struct inode *inode);

and finally, a function to get the cache cookie pointer from the context
attached to an inode (or NULL if fscache is disabled)::
+2 −2
Original line number Diff line number Diff line
@@ -62,12 +62,12 @@ void v9fs_cache_inode_get_cookie(struct inode *inode)
	version = cpu_to_le32(v9inode->qid.version);
	path = cpu_to_le64(v9inode->qid.path);
	v9ses = v9fs_inode2v9ses(inode);
	v9inode->netfs_ctx.cache =
	v9inode->netfs.cache =
		fscache_acquire_cookie(v9fs_session_cache(v9ses),
				       0,
				       &path, sizeof(path),
				       &version, sizeof(version),
				       i_size_read(&v9inode->vfs_inode));
				       i_size_read(&v9inode->netfs.inode));

	p9_debug(P9_DEBUG_FSC, "inode %p get cookie %p\n",
		 inode, v9fs_inode_cookie(v9inode));
+1 −1
Original line number Diff line number Diff line
@@ -625,7 +625,7 @@ static void v9fs_inode_init_once(void *foo)
	struct v9fs_inode *v9inode = (struct v9fs_inode *)foo;

	memset(&v9inode->qid, 0, sizeof(v9inode->qid));
	inode_init_once(&v9inode->vfs_inode);
	inode_init_once(&v9inode->netfs.inode);
}

/**
+3 −7
Original line number Diff line number Diff line
@@ -109,11 +109,7 @@ struct v9fs_session_info {
#define V9FS_INO_INVALID_ATTR 0x01

struct v9fs_inode {
	struct {
		/* These must be contiguous */
		struct inode	vfs_inode;	/* the VFS's inode record */
		struct netfs_i_context netfs_ctx; /* Netfslib context */
	};
	struct netfs_inode netfs; /* Netfslib context and vfs inode */
	struct p9_qid qid;
	unsigned int cache_validity;
	struct p9_fid *writeback_fid;
@@ -122,13 +118,13 @@ struct v9fs_inode {

static inline struct v9fs_inode *V9FS_I(const struct inode *inode)
{
	return container_of(inode, struct v9fs_inode, vfs_inode);
	return container_of(inode, struct v9fs_inode, netfs.inode);
}

static inline struct fscache_cookie *v9fs_inode_cookie(struct v9fs_inode *v9inode)
{
#ifdef CONFIG_9P_FSCACHE
	return netfs_i_cookie(&v9inode->vfs_inode);
	return netfs_i_cookie(&v9inode->netfs.inode);
#else
	return NULL;
#endif
+1 −1
Original line number Diff line number Diff line
@@ -140,7 +140,7 @@ static void v9fs_write_to_cache_done(void *priv, ssize_t transferred_or_error,
	    transferred_or_error != -ENOBUFS) {
		version = cpu_to_le32(v9inode->qid.version);
		fscache_invalidate(v9fs_inode_cookie(v9inode), &version,
				   i_size_read(&v9inode->vfs_inode), 0);
				   i_size_read(&v9inode->netfs.inode), 0);
	}
}

Loading