From: "Gustavo A. R. Silva" <gustavo@embeddedor.com>
To: Erick Archer <erick.archer@gmx.com>,
Kalle Valo <kvalo@kernel.org>,
Johannes Berg <johannes.berg@intel.com>,
"Gustavo A. R. Silva" <gustavoars@kernel.org>,
Kees Cook <keescook@chromium.org>
Cc: linux-wireless@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-hardening@vger.kernel.org
Subject: Re: [PATCH] mwl8k: Avoid overlapping composite structs that contain flex-arrays
Date: Sat, 16 Mar 2024 12:59:11 -0600 [thread overview]
Message-ID: <cfc4c4c0-83f8-437c-8146-6b86968db67b@embeddedor.com> (raw)
In-Reply-To: <20240316150712.4633-1-erick.archer@gmx.com>
[..]
>
> Link: https://github.com/KSPP/linux/issues/202 [1]
> Signed-off-by: Erick Archer <erick.archer@gmx.com>
> ---
> Hi everyone,
>
> This patch is based on my understanding of the code. Any comments would
> be greatly appreciated.
Thanks for looking into this. :)
I'm currently in the process of trying a general solution for all these
composite structures without having to use two separate structs, avoid too
much code churn, and continue allowing for __counted_by() annotations at
the same time.
I'll be sending a bunch of patches once the merge window closes. So, for
now, I think it's wise to wait for those patches.
More comments below.
[..]
> diff --git a/drivers/net/wireless/marvell/mwl8k.c b/drivers/net/wireless/marvell/mwl8k.c
> index ce8fea76dbb2..57de32ba4efc 100644
> --- a/drivers/net/wireless/marvell/mwl8k.c
> +++ b/drivers/net/wireless/marvell/mwl8k.c
> @@ -586,13 +586,17 @@ static int mwl8k_request_firmware(struct mwl8k_priv *priv, char *fw_image,
> return 0;
> }
>
> -struct mwl8k_cmd_pkt {
> +struct mwl8k_cmd_pkt_hdr {
> __le16 code;
> __le16 length;
> __u8 seq_num;
> __u8 macid;
> __le16 result;
> - char payload[];
> +} __packed;
> +
> +struct mwl8k_cmd_pkt {
> + struct mwl8k_cmd_pkt_hdr header;
> + char payload[];
> } __packed;
One of the problems with this is that `struct mwl8k_cmd_pkt` is candidate for a
`__counted_by()` annotation:
@@ -592,7 +592,7 @@ struct mwl8k_cmd_pkt {
__u8 seq_num;
__u8 macid;
__le16 result;
- char payload[];
+ char payload[] __counted_by(length);
} __packed;
and with the changes you propose, that is not possible anymore because the counter
member must be at the same level or in an anonymous struct also at the same level
as `payload`.
Thanks
--
Gustavo
next prev parent reply other threads:[~2024-03-16 19:00 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-16 15:07 [PATCH] mwl8k: Avoid overlapping composite structs that contain flex-arrays Erick Archer
2024-03-16 18:59 ` Gustavo A. R. Silva [this message]
2024-03-17 15:22 ` Erick Archer
2024-03-17 20:07 ` Gustavo A. R. Silva
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=cfc4c4c0-83f8-437c-8146-6b86968db67b@embeddedor.com \
--to=gustavo@embeddedor.com \
--cc=erick.archer@gmx.com \
--cc=gustavoars@kernel.org \
--cc=johannes.berg@intel.com \
--cc=keescook@chromium.org \
--cc=kvalo@kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-wireless@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).