All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gustavo A. R. Silva" <gustavoars@kernel.org>
To: Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>
Cc: linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	linux-hardening@vger.kernel.org,
	Kees Cook <keescook@chromium.org>
Subject: [PATCH][next] RDMA/uverbs: Avoid -Wflex-array-member-not-at-end warnings
Date: Fri, 1 Mar 2024 12:37:45 -0600	[thread overview]
Message-ID: <ZeIgeZ5Sb0IZTOyt@neat> (raw)

-Wflex-array-member-not-at-end is coming in GCC-14, and we are getting
ready to enable it globally.

There are currently a couple of objects (`alloc_head` and `bundle`) in
`struct bundle_priv` that contain a couple of flexible structures:

struct bundle_priv {
        /* Must be first */
        struct bundle_alloc_head alloc_head;

	...

        /*
         * Must be last. bundle ends in a flex array which overlaps
         * internal_buffer.
         */
        struct uverbs_attr_bundle bundle;
        u64 internal_buffer[32];
};

So, in order to avoid ending up with a couple of flexible-array members
in the middle of a struct, we use the `struct_group_tagged()` helper to
separate the flexible array from the rest of the members in the flexible
structures:

struct uverbs_attr_bundle {
        struct_group_tagged(uverbs_attr_bundle_hdr, hdr,
		... the rest of the members
        );
        struct uverbs_attr attrs[];
};

With the change described above, we now declare objects of the type of
the tagged struct without embedding flexible arrays in the middle of
another struct:

struct bundle_priv {
        /* Must be first */
        struct bundle_alloc_head_hdr alloc_head;

        ...

        struct uverbs_attr_bundle_hdr bundle;
        u64 internal_buffer[32];
};

We also use `container_of()` whenever we need to retrieve a pointer
to the flexible structures.

Notice that the `bundle_size` computed in `uapi_compute_bundle_size()`
remains the same.

So, with these changes, fix the following warnings:

drivers/infiniband/core/uverbs_ioctl.c:45:34: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
   45 |         struct bundle_alloc_head alloc_head;
      |                                  ^~~~~~~~~~
drivers/infiniband/core/uverbs_ioctl.c:67:35: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
   67 |         struct uverbs_attr_bundle bundle;
      |                                   ^~~~~~

Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/infiniband/core/uverbs_ioctl.c | 78 +++++++++++++++-----------
 include/rdma/uverbs_ioctl.h            | 14 +++--
 2 files changed, 53 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/core/uverbs_ioctl.c b/drivers/infiniband/core/uverbs_ioctl.c
index d9799706c58e..f80da6a67e24 100644
--- a/drivers/infiniband/core/uverbs_ioctl.c
+++ b/drivers/infiniband/core/uverbs_ioctl.c
@@ -36,13 +36,15 @@
 #include "uverbs.h"
 
 struct bundle_alloc_head {
-	struct bundle_alloc_head *next;
+	struct_group_tagged(bundle_alloc_head_hdr, hdr,
+		struct bundle_alloc_head *next;
+	);
 	u8 data[];
 };
 
 struct bundle_priv {
 	/* Must be first */
-	struct bundle_alloc_head alloc_head;
+	struct bundle_alloc_head_hdr alloc_head;
 	struct bundle_alloc_head *allocated_mem;
 	size_t internal_avail;
 	size_t internal_used;
@@ -64,7 +66,7 @@ struct bundle_priv {
 	 * Must be last. bundle ends in a flex array which overlaps
 	 * internal_buffer.
 	 */
-	struct uverbs_attr_bundle bundle;
+	struct uverbs_attr_bundle_hdr bundle;
 	u64 internal_buffer[32];
 };
 
@@ -77,9 +79,10 @@ void uapi_compute_bundle_size(struct uverbs_api_ioctl_method *method_elm,
 			      unsigned int num_attrs)
 {
 	struct bundle_priv *pbundle;
+	struct uverbs_attr_bundle *bundle;
 	size_t bundle_size =
 		offsetof(struct bundle_priv, internal_buffer) +
-		sizeof(*pbundle->bundle.attrs) * method_elm->key_bitmap_len +
+		sizeof(*bundle->attrs) * method_elm->key_bitmap_len +
 		sizeof(*pbundle->uattrs) * num_attrs;
 
 	method_elm->use_stack = bundle_size <= sizeof(*pbundle);
@@ -107,7 +110,7 @@ __malloc void *_uverbs_alloc(struct uverbs_attr_bundle *bundle, size_t size,
 			     gfp_t flags)
 {
 	struct bundle_priv *pbundle =
-		container_of(bundle, struct bundle_priv, bundle);
+		container_of(&bundle->hdr, struct bundle_priv, bundle);
 	size_t new_used;
 	void *res;
 
@@ -149,7 +152,7 @@ static int uverbs_set_output(const struct uverbs_attr_bundle *bundle,
 			     const struct uverbs_attr *attr)
 {
 	struct bundle_priv *pbundle =
-		container_of(bundle, struct bundle_priv, bundle);
+		container_of(&bundle->hdr, struct bundle_priv, bundle);
 	u16 flags;
 
 	flags = pbundle->uattrs[attr->ptr_attr.uattr_idx].flags |
@@ -166,6 +169,8 @@ static int uverbs_process_idrs_array(struct bundle_priv *pbundle,
 				     struct ib_uverbs_attr *uattr,
 				     u32 attr_bkey)
 {
+	struct uverbs_attr_bundle *bundle =
+		container_of(&pbundle->bundle, struct uverbs_attr_bundle, hdr);
 	const struct uverbs_attr_spec *spec = &attr_uapi->spec;
 	size_t array_len;
 	u32 *idr_vals;
@@ -184,7 +189,7 @@ static int uverbs_process_idrs_array(struct bundle_priv *pbundle,
 		return -EINVAL;
 
 	attr->uobjects =
-		uverbs_alloc(&pbundle->bundle,
+		uverbs_alloc(bundle,
 			     array_size(array_len, sizeof(*attr->uobjects)));
 	if (IS_ERR(attr->uobjects))
 		return PTR_ERR(attr->uobjects);
@@ -209,7 +214,7 @@ static int uverbs_process_idrs_array(struct bundle_priv *pbundle,
 	for (i = 0; i != array_len; i++) {
 		attr->uobjects[i] = uverbs_get_uobject_from_file(
 			spec->u2.objs_arr.obj_type, spec->u2.objs_arr.access,
-			idr_vals[i], &pbundle->bundle);
+			idr_vals[i], bundle);
 		if (IS_ERR(attr->uobjects[i])) {
 			ret = PTR_ERR(attr->uobjects[i]);
 			break;
@@ -240,7 +245,9 @@ static int uverbs_process_attr(struct bundle_priv *pbundle,
 			       struct ib_uverbs_attr *uattr, u32 attr_bkey)
 {
 	const struct uverbs_attr_spec *spec = &attr_uapi->spec;
-	struct uverbs_attr *e = &pbundle->bundle.attrs[attr_bkey];
+	struct uverbs_attr_bundle *bundle =
+		container_of(&pbundle->bundle, struct uverbs_attr_bundle, hdr);
+	struct uverbs_attr *e = &bundle->attrs[attr_bkey];
 	const struct uverbs_attr_spec *val_spec = spec;
 	struct uverbs_obj_attr *o_attr;
 
@@ -288,7 +295,7 @@ static int uverbs_process_attr(struct bundle_priv *pbundle,
 		if (val_spec->alloc_and_copy && !uverbs_attr_ptr_is_inline(e)) {
 			void *p;
 
-			p = uverbs_alloc(&pbundle->bundle, uattr->len);
+			p = uverbs_alloc(bundle, uattr->len);
 			if (IS_ERR(p))
 				return PTR_ERR(p);
 
@@ -321,7 +328,7 @@ static int uverbs_process_attr(struct bundle_priv *pbundle,
 		 */
 		o_attr->uobject = uverbs_get_uobject_from_file(
 			spec->u.obj.obj_type, spec->u.obj.access,
-			uattr->data_s64, &pbundle->bundle);
+			uattr->data_s64, bundle);
 		if (IS_ERR(o_attr->uobject))
 			return PTR_ERR(o_attr->uobject);
 		__set_bit(attr_bkey, pbundle->uobj_finalize);
@@ -422,6 +429,8 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 				unsigned int num_attrs)
 {
 	int (*handler)(struct uverbs_attr_bundle *attrs);
+	struct uverbs_attr_bundle *bundle =
+		container_of(&pbundle->bundle, struct uverbs_attr_bundle, hdr);
 	size_t uattrs_size = array_size(sizeof(*pbundle->uattrs), num_attrs);
 	unsigned int destroy_bkey = pbundle->method_elm->destroy_bkey;
 	unsigned int i;
@@ -434,7 +443,7 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 	if (!handler)
 		return -EIO;
 
-	pbundle->uattrs = uverbs_alloc(&pbundle->bundle, uattrs_size);
+	pbundle->uattrs = uverbs_alloc(bundle, uattrs_size);
 	if (IS_ERR(pbundle->uattrs))
 		return PTR_ERR(pbundle->uattrs);
 	if (copy_from_user(pbundle->uattrs, pbundle->user_attrs, uattrs_size))
@@ -453,25 +462,23 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 		return -EINVAL;
 
 	if (pbundle->method_elm->has_udata)
-		uverbs_fill_udata(&pbundle->bundle,
-				  &pbundle->bundle.driver_udata,
+		uverbs_fill_udata(bundle, &pbundle->bundle.driver_udata,
 				  UVERBS_ATTR_UHW_IN, UVERBS_ATTR_UHW_OUT);
 	else
 		pbundle->bundle.driver_udata = (struct ib_udata){};
 
 	if (destroy_bkey != UVERBS_API_ATTR_BKEY_LEN) {
-		struct uverbs_obj_attr *destroy_attr =
-			&pbundle->bundle.attrs[destroy_bkey].obj_attr;
+		struct uverbs_obj_attr *destroy_attr = &bundle->attrs[destroy_bkey].obj_attr;
 
-		ret = uobj_destroy(destroy_attr->uobject, &pbundle->bundle);
+		ret = uobj_destroy(destroy_attr->uobject, bundle);
 		if (ret)
 			return ret;
 		__clear_bit(destroy_bkey, pbundle->uobj_finalize);
 
-		ret = handler(&pbundle->bundle);
+		ret = handler(bundle);
 		uobj_put_destroy(destroy_attr->uobject);
 	} else {
-		ret = handler(&pbundle->bundle);
+		ret = handler(bundle);
 	}
 
 	/*
@@ -481,10 +488,10 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 	 */
 	if (!ret && pbundle->method_elm->has_udata) {
 		const struct uverbs_attr *attr =
-			uverbs_attr_get(&pbundle->bundle, UVERBS_ATTR_UHW_OUT);
+			uverbs_attr_get(bundle, UVERBS_ATTR_UHW_OUT);
 
 		if (!IS_ERR(attr))
-			ret = uverbs_set_output(&pbundle->bundle, attr);
+			ret = uverbs_set_output(bundle, attr);
 	}
 
 	/*
@@ -501,6 +508,8 @@ static int ib_uverbs_run_method(struct bundle_priv *pbundle,
 static void bundle_destroy(struct bundle_priv *pbundle, bool commit)
 {
 	unsigned int key_bitmap_len = pbundle->method_elm->key_bitmap_len;
+	struct uverbs_attr_bundle *bundle =
+		container_of(&pbundle->bundle, struct uverbs_attr_bundle, hdr);
 	struct bundle_alloc_head *memblock;
 	unsigned int i;
 
@@ -508,20 +517,19 @@ static void bundle_destroy(struct bundle_priv *pbundle, bool commit)
 	i = -1;
 	while ((i = find_next_bit(pbundle->uobj_finalize, key_bitmap_len,
 				  i + 1)) < key_bitmap_len) {
-		struct uverbs_attr *attr = &pbundle->bundle.attrs[i];
+		struct uverbs_attr *attr = &bundle->attrs[i];
 
 		uverbs_finalize_object(
 			attr->obj_attr.uobject,
 			attr->obj_attr.attr_elm->spec.u.obj.access,
 			test_bit(i, pbundle->uobj_hw_obj_valid),
-			commit,
-			&pbundle->bundle);
+			commit, bundle);
 	}
 
 	i = -1;
 	while ((i = find_next_bit(pbundle->spec_finalize, key_bitmap_len,
 				  i + 1)) < key_bitmap_len) {
-		struct uverbs_attr *attr = &pbundle->bundle.attrs[i];
+		struct uverbs_attr *attr = &bundle->attrs[i];
 		const struct uverbs_api_attr *attr_uapi;
 		void __rcu **slot;
 
@@ -535,7 +543,7 @@ static void bundle_destroy(struct bundle_priv *pbundle, bool commit)
 
 		if (attr_uapi->spec.type == UVERBS_ATTR_TYPE_IDRS_ARRAY) {
 			uverbs_free_idrs_array(attr_uapi, &attr->objs_arr_attr,
-					       commit, &pbundle->bundle);
+					       commit, bundle);
 		}
 	}
 
@@ -578,7 +586,8 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
 			method_elm->bundle_size -
 			offsetof(struct bundle_priv, internal_buffer);
 		pbundle->alloc_head.next = NULL;
-		pbundle->allocated_mem = &pbundle->alloc_head;
+		pbundle->allocated_mem = container_of(&pbundle->alloc_head,
+						struct bundle_alloc_head, hdr);
 	} else {
 		pbundle = &onstack;
 		pbundle->internal_avail = sizeof(pbundle->internal_buffer);
@@ -596,8 +605,9 @@ static int ib_uverbs_cmd_verbs(struct ib_uverbs_file *ufile,
 	pbundle->user_attrs = user_attrs;
 
 	pbundle->internal_used = ALIGN(pbundle->method_elm->key_bitmap_len *
-					       sizeof(*pbundle->bundle.attrs),
-				       sizeof(*pbundle->internal_buffer));
+					       sizeof(*container_of(&pbundle->bundle,
+							struct uverbs_attr_bundle, hdr)->attrs),
+					       sizeof(*pbundle->internal_buffer));
 	memset(pbundle->bundle.attr_present, 0,
 	       sizeof(pbundle->bundle.attr_present));
 	memset(pbundle->uobj_finalize, 0, sizeof(pbundle->uobj_finalize));
@@ -700,11 +710,13 @@ void uverbs_fill_udata(struct uverbs_attr_bundle *bundle,
 		       unsigned int attr_out)
 {
 	struct bundle_priv *pbundle =
-		container_of(bundle, struct bundle_priv, bundle);
+		container_of(&bundle->hdr, struct bundle_priv, bundle);
+	struct uverbs_attr_bundle *bundle_aux =
+		container_of(&pbundle->bundle, struct uverbs_attr_bundle, hdr);
 	const struct uverbs_attr *in =
-		uverbs_attr_get(&pbundle->bundle, attr_in);
+		uverbs_attr_get(bundle_aux, attr_in);
 	const struct uverbs_attr *out =
-		uverbs_attr_get(&pbundle->bundle, attr_out);
+		uverbs_attr_get(bundle_aux, attr_out);
 
 	if (!IS_ERR(in)) {
 		udata->inlen = in->ptr_attr.len;
@@ -829,7 +841,7 @@ void uverbs_finalize_uobj_create(const struct uverbs_attr_bundle *bundle,
 				 u16 idx)
 {
 	struct bundle_priv *pbundle =
-		container_of(bundle, struct bundle_priv, bundle);
+		container_of(&bundle->hdr, struct bundle_priv, bundle);
 
 	__set_bit(uapi_bkey_attr(uapi_key_attr(idx)),
 		  pbundle->uobj_hw_obj_valid);
diff --git a/include/rdma/uverbs_ioctl.h b/include/rdma/uverbs_ioctl.h
index 06287de69cd2..e6c0de227fad 100644
--- a/include/rdma/uverbs_ioctl.h
+++ b/include/rdma/uverbs_ioctl.h
@@ -629,12 +629,14 @@ struct uverbs_attr {
 };
 
 struct uverbs_attr_bundle {
-	struct ib_udata driver_udata;
-	struct ib_udata ucore;
-	struct ib_uverbs_file *ufile;
-	struct ib_ucontext *context;
-	struct ib_uobject *uobject;
-	DECLARE_BITMAP(attr_present, UVERBS_API_ATTR_BKEY_LEN);
+	struct_group_tagged(uverbs_attr_bundle_hdr, hdr,
+		struct ib_udata driver_udata;
+		struct ib_udata ucore;
+		struct ib_uverbs_file *ufile;
+		struct ib_ucontext *context;
+		struct ib_uobject *uobject;
+		DECLARE_BITMAP(attr_present, UVERBS_API_ATTR_BKEY_LEN);
+	);
 	struct uverbs_attr attrs[];
 };
 
-- 
2.34.1


             reply	other threads:[~2024-03-01 18:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-01 18:37 Gustavo A. R. Silva [this message]
2024-03-02  0:14 ` [PATCH][next] RDMA/uverbs: Avoid -Wflex-array-member-not-at-end warnings Kees Cook
2024-03-03 13:41 ` Leon Romanovsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZeIgeZ5Sb0IZTOyt@neat \
    --to=gustavoars@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=keescook@chromium.org \
    --cc=leon@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rdma@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.