diff mbox series

[v2] netfs: Use container_of() for offset casting

Message ID 20220518202212.2322058-1-keescook@chromium.org (mailing list archive)
State New, archived
Headers show
Series [v2] netfs: Use container_of() for offset casting | expand

Commit Message

Kees Cook May 18, 2022, 8:22 p.m. UTC
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. Switch to using
an internally defined netfs_i_context/inode struct for doing a full
container_of() casting. This keeps both randstruct and __bos() happy
under GCC 12. Silences:

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);
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Reported-by: Jeff Layton <jlayton@kernel.org>
Link: https://lore.kernel.org/lkml/d2ad3a3d7bdd794c6efb562d2f2b655fb67756b9.camel@kernel.org
Cc: Jeff Layton <jlayton@kernel.org>
Cc: David Howells <dhowells@redhat.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v1: https://lore.kernel.org/lkml/20220517210230.864239-1-keescook@chromium.org
v2:
 - Add macro for keeping all netfs users on the same page
 - Update documentation and each netfs user
---
 Documentation/filesystems/netfs_library.rst | 12 ++++------
 fs/9p/v9fs.h                                |  7 ++----
 fs/afs/internal.h                           |  7 +-----
 fs/ceph/super.h                             |  7 ++----
 fs/cifs/cifsglob.h                          |  7 ++----
 include/linux/netfs.h                       | 26 +++++++++++++++++++--
 6 files changed, 36 insertions(+), 30 deletions(-)
diff mbox series

Patch

diff --git a/Documentation/filesystems/netfs_library.rst b/Documentation/filesystems/netfs_library.rst
index 69f00179fdfe..8024d442833e 100644
--- a/Documentation/filesystems/netfs_library.rst
+++ b/Documentation/filesystems/netfs_library.rst
@@ -43,15 +43,13 @@  structure is defined::
 	};
 
 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::
+directly after the VFS ``struct inode`` it allocates, either by using
+``struct netfs_i_c_pair`` or by using the ``DECLARE_NETFS_INODE()`` helper,
+which arranges ``struct inode`` and ``struct netfs_i_context`` together
+without a struct namespace::
 
 	struct my_inode {
-		struct {
-			/* These must be contiguous */
-			struct inode		vfs_inode;
-			struct netfs_i_context  netfs_ctx;
-		};
+		DECLARE_NETFS_INODE(vfs_inode, netfs_ctx);
 		...
 	};
 
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index ec0e8df3b2eb..595add687ac6 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -109,11 +109,8 @@  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 */
-	};
+	/* the VFS's inode record and the Netfslib context */
+	DECLARE_NETFS_INODE(vfs_inode, netfs_ctx);
 	struct p9_qid qid;
 	unsigned int cache_validity;
 	struct p9_fid *writeback_fid;
diff --git a/fs/afs/internal.h b/fs/afs/internal.h
index 7b7ef945dc78..e2cb94196828 100644
--- a/fs/afs/internal.h
+++ b/fs/afs/internal.h
@@ -619,12 +619,7 @@  enum afs_lock_state {
  * leak from one inode to another.
  */
 struct afs_vnode {
-	struct {
-		/* These must be contiguous */
-		struct inode	vfs_inode;	/* the VFS's inode record */
-		struct netfs_i_context netfs_ctx; /* Netfslib context */
-	};
-
+	DECLARE_NETFS_INODE(vfs_inode, netfs_ctx); /* VFS inode and Netfslib context */
 	struct afs_volume	*volume;	/* volume on which vnode resides */
 	struct afs_fid		fid;		/* the file identifier for this inode */
 	struct afs_file_status	status;		/* AFS status info for this file */
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index 20ceab74e871..7c36623bb42c 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -316,11 +316,8 @@  struct ceph_inode_xattrs_info {
  * Ceph inode.
  */
 struct ceph_inode_info {
-	struct {
-		/* These must be contiguous */
-		struct inode vfs_inode;
-		struct netfs_i_context netfs_ctx; /* Netfslib context */
-	};
+	/* the VFS's inode record and the Netfslib context */
+	DECLARE_NETFS_INODE(vfs_inode, netfs_ctx);
 	struct ceph_vino i_vino;   /* ceph ino + snap */
 
 	spinlock_t i_ceph_lock;
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 8de977c359b1..4a36dad99e32 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1405,11 +1405,8 @@  void cifsFileInfo_put(struct cifsFileInfo *cifs_file);
  */
 
 struct cifsInodeInfo {
-	struct {
-		/* These must be contiguous */
-		struct inode	vfs_inode;	/* the VFS's inode record */
-		struct netfs_i_context netfs_ctx; /* Netfslib context */
-	};
+	/* the VFS's inode record and the Netfslib context */
+	DECLARE_NETFS_INODE(vfs_inode, netfs_ctx);
 	bool can_cache_brlcks;
 	struct list_head llist;	/* locks helb by this inode */
 	/*
diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 0c33b715cbfd..7facb11c9ac7 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -286,6 +286,28 @@  extern void netfs_put_subrequest(struct netfs_io_subrequest *subreq,
 				 bool was_async, enum netfs_sreq_ref_trace what);
 extern void netfs_stats_show(struct seq_file *);
 
+/*
+ * The struct netfs_i_context instance must always follow the VFS inode, so
+ * struct netfs_i_c_pair enforces this. However, netfs users may want to
+ * avoid a sub-struct namespace, so they can alternatively use the
+ * DECLARE_NETFS_INODE macro to provide an anonymous union/struct wrapper,
+ * allowing netfs internals to still correctly use container_of() against
+ * the struct netfs_i_c_pair for casting between vfs_inode and netfs_ctx.
+ */
+struct netfs_i_c_pair {
+	struct inode		vfs_inode;
+	struct netfs_i_context	netfs_ctx;
+};
+
+#define DECLARE_NETFS_INODE(_inode, _ctx)			\
+	union {							\
+		struct {					\
+			struct inode		_inode;		\
+			struct netfs_i_context	_ctx;		\
+		};						\
+		struct netfs_i_c_pair		netfs_inode;	\
+	}
+
 /**
  * netfs_i_context - Get the netfs inode context from the inode
  * @inode: The inode to query
@@ -295,7 +317,7 @@  extern void netfs_stats_show(struct seq_file *);
  */
 static inline struct netfs_i_context *netfs_i_context(struct inode *inode)
 {
-	return (void *)inode + sizeof(*inode);
+	return &container_of(inode, struct netfs_i_c_pair, vfs_inode)->netfs_ctx;
 }
 
 /**
@@ -307,7 +329,7 @@  static inline struct netfs_i_context *netfs_i_context(struct inode *inode)
  */
 static inline struct inode *netfs_inode(struct netfs_i_context *ctx)
 {
-	return (void *)ctx - sizeof(struct inode);
+	return &container_of(ctx, struct netfs_i_c_pair, netfs_ctx)->vfs_inode;
 }
 
 /**