netdev - Re: [PATCH v3][next] net/mlx5e: Avoid a hundred -Wflex-array-member-not-at-end warnings

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

Hash Suite for Android: free password hash cracker in your pocket

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

Message-ID: <Z79iP0glNCZOznu4@x130>
Date: Wed, 26 Feb 2025 10:49:35 -0800
From: Saeed Mahameed <saeed@...nel.org>
To: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: Saeed Mahameed <saeedm@...dia.com>, Leon Romanovsky <leon@...nel.org>,
	Tariq Toukan <tariqt@...dia.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3][next] net/mlx5e: Avoid a hundred
 -Wflex-array-member-not-at-end warnings

On 26 Feb 13:47, Gustavo A. R. Silva wrote:
>-Wflex-array-member-not-at-end was introduced in GCC-14, and we are
>getting ready to enable it, globally.
>
>So, in this particular case, we create a new `struct mlx5e_umr_wqe_hdr`
>to enclose the header part of flexible structure `struct mlx5e_umr_wqe`.
>This is, all the members except the flexible arrays `inline_mtts`,
>`inline_klms` and `inline_ksms` in the anonymous union. We then replace
>the header part with `struct mlx5e_umr_wqe_hdr hdr;` in `struct
>mlx5e_umr_wqe`, and change the type of the object currently causing
>trouble `umr_wqe` from `struct mlx5e_umr_wqe` to `struct
>mlx5e_umr_wqe_hdr` --this last bit gets rid of the flex-array-in-the-middle
>part and avoid the warnings.
>
>Also, no new members should be added to `struct mlx5e_umr_wqe`, instead
>any new members must be included in the header structure `struct
>mlx5e_umr_wqe_hdr`. To enforce this, we use `static_assert()`, ensuring
>that the memory layout of both the flexible structure and the newly
>created header struct remain consistent.
>
>The next step is to refactor the rest of the related code accordingly,
>which means adding a bunch of `hdr.` wherever needed.
>
>Lastly, we use `container_of()` whenever we need to retrieve a pointer
>to the flexible structure `struct mlx5e_umr_wqe`.
>
>So, with these changes, fix 125 of the following warnings:
>
>drivers/net/ethernet/mellanox/mlx5/core/en.h:664:48: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
>
>Signed-off-by: Gustavo A. R. Silva <gustavoars@...nel.org>
>---
>Changes in v3:
> - Update error message in the assertion.
> - Also, this part is intentionally left as-is (to keep the assertion
>   as close as possible to the flex struct):
>
>| CHECK: Please use a blank line after function/struct/union/enum declarations
>| #63: FILE: drivers/net/ethernet/mellanox/mlx5/core/en.h:249:
>| };
>| +static_assert(offsetof(struct mlx5e_umr_wqe, inline_mtts) == sizeof(struct mlx5e_umr_wqe_hdr),
>
>Changes in v2:
> - Split the header members of `struct mlx5e_umr_wqe` into a
>   separate `struct mlx5e_umr_wqe_hdr`, and refactor the code
>   accordingly. (Jakub)
> - Update the changelog text.
> - Link: https://lore.kernel.org/linux-hardening/Z76FE8oZO2Ssuj9T@kspp/
>
>v1:
> - Link: https://lore.kernel.org/linux-hardening/Z6GCJY8G9EzASrwQ@kspp/
>
> drivers/net/ethernet/mellanox/mlx5/core/en.h  | 10 +++++++--
> .../ethernet/mellanox/mlx5/core/en/xsk/rx.c   |  6 ++---
> .../net/ethernet/mellanox/mlx5/core/en_main.c |  8 ++++---
> .../net/ethernet/mellanox/mlx5/core/en_rx.c   | 22 +++++++++----------
> 4 files changed, 27 insertions(+), 19 deletions(-)
>
>diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>index 979fc56205e1..90de40521029 100644
>--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
>+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
>@@ -232,16 +232,22 @@ struct mlx5e_rx_wqe_cyc {
> 	DECLARE_FLEX_ARRAY(struct mlx5_wqe_data_seg, data);
> };
>
>-struct mlx5e_umr_wqe {
>+struct mlx5e_umr_wqe_hdr {
> 	struct mlx5_wqe_ctrl_seg       ctrl;
> 	struct mlx5_wqe_umr_ctrl_seg   uctrl;
> 	struct mlx5_mkey_seg           mkc;
>+};
>+
>+struct mlx5e_umr_wqe {
>+	struct mlx5e_umr_wqe_hdr hdr;

You missed or ignored my comment on v0, anyway:

Can we have struct mlx5e_umr_wq_hdr defined anonymously within
mlx5e_umr_wqe? Let's avoid namespace pollution.