[PATCH] wifi: wfx: validate num_tx_confs against HIF message length

[PATCH] wifi: wfx: validate num_tx_confs against HIF message length

[Aamir Ahmed]

wfx_hif_multi_tx_confirm() trusts the device-supplied
num_tx_confs field without checking whether the HIF message
is large enough to contain that many entries. A malformed
or corrupted message with a large num_tx_confs value causes
out-of-bounds reads from the tx_conf_payload flexible array.

Validate that the HIF body is large enough for the fixed
header and the claimed number of confirmation entries before
iterating.

Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
---
 drivers/net/wireless/silabs/wfx/hif_rx.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/silabs/wfx/hif_rx.c b/drivers/net/wireless/silabs/wfx/hif_rx.c
index 64ca8acb8e4f..683fb67916c8 100644
--- a/drivers/net/wireless/silabs/wfx/hif_rx.c
+++ b/drivers/net/wireless/silabs/wfx/hif_rx.c
@@ -62,9 +62,15 @@ static int wfx_hif_multi_tx_confirm(struct wfx_dev *wdev,
 				    const struct wfx_hif_msg *hif, const void *buf)
 {
 	const struct wfx_hif_cnf_multi_transmit *body = buf;
+	int hif_body_len = le16_to_cpu(hif->len) - sizeof(*hif);
 	int i;
 
-	WARN(body->num_tx_confs <= 0, "corrupted message");
+	if (hif_body_len < (int)sizeof(*body) ||
+	    body->num_tx_confs > (hif_body_len - sizeof(*body)) /
+				 sizeof(body->tx_conf_payload[0])) {
+		dev_err(wdev->dev, "corrupted multi tx confirm\n");
+		return -EINVAL;
+	}
 	for (i = 0; i < body->num_tx_confs; i++)
 		wfx_tx_confirm_cb(wdev, &body->tx_conf_payload[i]);
 	return 0;
-- 
2.43.0

Re: [PATCH] wifi: wfx: validate num_tx_confs against HIF message length

[Jérôme Pouiller]

On Monday 7 September 2026 04:59:00 Central European Summer Time Aamir Ahmed wrote:
> wfx_hif_multi_tx_confirm() trusts the device-supplied
> num_tx_confs field without checking whether the HIF message
> is large enough to contain that many entries. A malformed
> or corrupted message with a large num_tx_confs value causes
> out-of-bounds reads from the tx_conf_payload flexible array.
> 
> Validate that the HIF body is large enough for the fixed
> header and the claimed number of confirmation entries before
> iterating.
> 
> Fixes: 9bca45f3d692 ("staging: wfx: allow to send 802.11 frames")
> Signed-off-by: Aamir Ahmed <elb12345@hotmail.co.uk>
> ---
>  drivers/net/wireless/silabs/wfx/hif_rx.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/silabs/wfx/hif_rx.c b/drivers/net/wireless/silabs/wfx/hif_rx.c
> index 64ca8acb8e4f..683fb67916c8 100644
> --- a/drivers/net/wireless/silabs/wfx/hif_rx.c
> +++ b/drivers/net/wireless/silabs/wfx/hif_rx.c
> @@ -62,9 +62,15 @@ static int wfx_hif_multi_tx_confirm(struct wfx_dev *wdev,
>                                     const struct wfx_hif_msg *hif, const void *buf)
>  {
>         const struct wfx_hif_cnf_multi_transmit *body = buf;
> +       int hif_body_len = le16_to_cpu(hif->len) - sizeof(*hif);
>         int i;
> 
> -       WARN(body->num_tx_confs <= 0, "corrupted message");
> +       if (hif_body_len < (int)sizeof(*body) ||
> +           body->num_tx_confs > (hif_body_len - sizeof(*body)) /
> +                                sizeof(body->tx_conf_payload[0])) {
> +               dev_err(wdev->dev, "corrupted multi tx confirm\n");
> +               return -EINVAL;
> +       }

body->num_tx_confs is already used in rx_helper(). So, the
error detection happens to late.

Until now, I have considered the device semi-trusted, so skipping this
test does not shock me.

(I tend to prefer the readability of the code rather to clutter it with
plenty of error paths impossible to reach and impossible to test)


-- 
Jérôme Pouiller

Re: [PATCH] wifi: wfx: validate num_tx_confs against HIF message length

[Aamir Ahmed]

On Mon, Sep 07, 2026 at 09:52:03PM +0200, Jerome Pouiller wrote:
> body->num_tx_confs is already used in rx_helper(). So, the
> error detection happens to late.

Yes that makes sense. The check should live in rx_helper(), next to
the existing hif->len consistency check, so that the tx_buffers_used
accounting never sees an incorrect count either.

> Until now, I have considered the device semi-trusted, so skipping this
> test does not shock me.
>
> (I tend to prefer the readability of the code rather to clutter it with
> plenty of error paths impossible to reach and impossible to test)

Understood. If you would take a single check in rx_helper() I am happy
to send that as v2; otherwise I can drop it. Thanks.

Kind Regards

Aamir A.