lkml.org 
[lkml]   [2022]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] netfs: Use container_of() for offset casting
From
Date
On Tue, 2022-05-17 at 14:02 -0700, Kees Cook 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. 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>
> ---
> 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;
> }
>
> /**

This patch didn't apply cleanly for me to a recent tree, but I was able
to wiggle it into place and it seemed to work.

Tested-by: Jeff Layton <jlayton@kernel.org>

\
 
 \ /
  Last update: 2022-05-18 00:33    [W:0.086 / U:0.260 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site