[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20220518202212.2322058-1-keescook@chromium.org>
Date: Wed, 18 May 2022 13:22:12 -0700
From: Kees Cook <keescook@...omium.org>
To: Jeff Layton <jlayton@...nel.org>,
David Howells <dhowells@...hat.com>
Cc: Kees Cook <keescook@...omium.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 <sfrench@...ba.org>,
William Kucharski <william.kucharski@...cle.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
linux-kernel@...r.kernel.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-hardening@...r.kernel.org
Subject: [PATCH v2] netfs: Use container_of() for offset casting
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@...nel.org>
Link: https://lore.kernel.org/lkml/d2ad3a3d7bdd794c6efb562d2f2b655fb67756b9.camel@kernel.org
Cc: Jeff Layton <jlayton@...nel.org>
Cc: David Howells <dhowells@...hat.com>
Signed-off-by: Kees Cook <keescook@...omium.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 --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;
}
/**
--
2.32.0
Powered by blists - more mailing lists