linux-hardening - [PATCH] netfs: Use container

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 PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 17 May 2022 14:02:30 -0700
From:   Kees Cook <keescook@...omium.org>
To:     Jeff Layton <jlayton@...nel.org>
Cc:     Kees Cook <keescook@...omium.org>,
        David Howells <dhowells@...hat.com>,
        linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: [PATCH] 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>
---
If this looks good I can add it to my hardening tree, or if you want to
carry it, I can respin this without the earlier randstruct changes and
drop that patch from my tree?
---
 include/linux/netfs.h | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfs.h b/include/linux/netfs.h
index 0c33b715cbfd..cce5a9b53a8a 100644
--- a/include/linux/netfs.h
+++ b/include/linux/netfs.h
@@ -286,6 +286,17 @@ 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,
+ * but existing users want to avoid a substructure name space, so just
+ * use this internally to perform the needed container_of() offset
+ * casting, which will keep both FORTIFY_SOURCE and randstruct happy.
+ */
+struct netfs_i_c_pair {
+	struct inode inode;
+	struct netfs_i_context ctx;
+};
+
 /**
  * netfs_i_context - Get the netfs inode context from the inode
  * @inode: The inode to query
@@ -295,7 +306,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, inode)->ctx;
 }
 
 /**
@@ -307,7 +318,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, ctx)->inode;
 }
 
 /**
-- 
2.32.0