[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFEp6-10WCGvGrRMh0q1DKYK+C+qm9yh-C7bGgdEFccM9TUbdA@mail.gmail.com>
Date: Tue, 25 Nov 2025 10:02:13 +0100
From: Loic Poulain <loic.poulain@....qualcomm.com>
To: "Gustavo A. R. Silva" <gustavoars@...nel.org>
Cc: Sergey Ryazanov <ryazanov.s.a@...il.com>,
Johannes Berg <johannes@...solutions.net>,
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-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2][next] net: wwan: mhi_wwan_mbim: Avoid
-Wflex-array-member-not-at-end warning
On Tue, Nov 25, 2025 at 5:27 AM Gustavo A. R. Silva
<gustavoars@...nel.org> wrote:
>
> Use DEFINE_RAW_FLEX() to avoid a -Wflex-array-member-not-at-end warning.
>
> Remove fixed-size array struct usb_cdc_ncm_dpe16 dpe16[2]; from struct
> mbim_tx_hdr, so that flex-array member struct mbim_tx_hdr::ndp16.dpe16[]
> ends last in this structure.
>
> Compensate for this by using the DEFINE_RAW_FLEX() helper to declare the
> on-stack struct instance that contains struct usb_cdc_ncm_ndp16 as a
> member. Adjust the rest of the code, accordingly.
>
> So, with these changes fix the following warning:
>
> drivers/net/wwan/mhi_wwan_mbim.c:81:34: 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>
I just noticed there’s a V2, so:
Reviewed-by: Loic Poulain <loic.poulain@....qualcomm.com>
> ---
> Changes in v2:
> - Add code comment to prevent people from adding new members after
> flex struct member `struct usb_cdc_ncm_ndp16 ndp16;`
>
> v1:
> - Link: https://lore.kernel.org/linux-hardening/aSUubvYfGJ-BIeDq@kspp/
>
> drivers/net/wwan/mhi_wwan_mbim.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/wwan/mhi_wwan_mbim.c b/drivers/net/wwan/mhi_wwan_mbim.c
> index c814fbd756a1..313dc5207c93 100644
> --- a/drivers/net/wwan/mhi_wwan_mbim.c
> +++ b/drivers/net/wwan/mhi_wwan_mbim.c
> @@ -78,8 +78,9 @@ struct mhi_mbim_context {
>
> struct mbim_tx_hdr {
> struct usb_cdc_ncm_nth16 nth16;
> +
> + /* Must be last as it ends in a flexible-array member. */
> struct usb_cdc_ncm_ndp16 ndp16;
> - struct usb_cdc_ncm_dpe16 dpe16[2];
> } __packed;
>
> static struct mhi_mbim_link *mhi_mbim_get_link_rcu(struct mhi_mbim_context *mbim,
> @@ -107,20 +108,20 @@ static int mhi_mbim_get_link_mux_id(struct mhi_controller *cntrl)
> static struct sk_buff *mbim_tx_fixup(struct sk_buff *skb, unsigned int session,
> u16 tx_seq)
> {
> + DEFINE_RAW_FLEX(struct mbim_tx_hdr, mbim_hdr, ndp16.dpe16, 2);
> unsigned int dgram_size = skb->len;
> struct usb_cdc_ncm_nth16 *nth16;
> struct usb_cdc_ncm_ndp16 *ndp16;
> - struct mbim_tx_hdr *mbim_hdr;
>
> /* Only one NDP is sent, containing the IP packet (no aggregation) */
>
> /* Ensure we have enough headroom for crafting MBIM header */
> - if (skb_cow_head(skb, sizeof(struct mbim_tx_hdr))) {
> + if (skb_cow_head(skb, __struct_size(mbim_hdr))) {
> dev_kfree_skb_any(skb);
> return NULL;
> }
>
> - mbim_hdr = skb_push(skb, sizeof(struct mbim_tx_hdr));
> + mbim_hdr = skb_push(skb, __struct_size(mbim_hdr));
>
> /* Fill NTB header */
> nth16 = &mbim_hdr->nth16;
> @@ -133,12 +134,11 @@ static struct sk_buff *mbim_tx_fixup(struct sk_buff *skb, unsigned int session,
> /* Fill the unique NDP */
> ndp16 = &mbim_hdr->ndp16;
> ndp16->dwSignature = cpu_to_le32(USB_CDC_MBIM_NDP16_IPS_SIGN | (session << 24));
> - ndp16->wLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_ndp16)
> - + sizeof(struct usb_cdc_ncm_dpe16) * 2);
> + ndp16->wLength = cpu_to_le16(struct_size(ndp16, dpe16, 2));
> ndp16->wNextNdpIndex = 0;
>
> /* Datagram follows the mbim header */
> - ndp16->dpe16[0].wDatagramIndex = cpu_to_le16(sizeof(struct mbim_tx_hdr));
> + ndp16->dpe16[0].wDatagramIndex = cpu_to_le16(__struct_size(mbim_hdr));
> ndp16->dpe16[0].wDatagramLength = cpu_to_le16(dgram_size);
>
> /* null termination */
> @@ -584,7 +584,7 @@ static void mhi_mbim_setup(struct net_device *ndev)
> {
> ndev->header_ops = NULL; /* No header */
> ndev->type = ARPHRD_RAWIP;
> - ndev->needed_headroom = sizeof(struct mbim_tx_hdr);
> + ndev->needed_headroom = struct_size_t(struct mbim_tx_hdr, ndp16.dpe16, 2);
> ndev->hard_header_len = 0;
> ndev->addr_len = 0;
> ndev->flags = IFF_POINTOPOINT | IFF_NOARP;
> --
> 2.43.0
>
Powered by blists - more mailing lists