Skip to contentSkip to conversation

wfx: prevent underflow in wfx_send_pds()

PATCH In review Dan Carpenter 3 participants 6 messages Aug 19, 2022 – Sep 2, 2022

Subsystems (2)
Kernel janitors Silicon labs wireless drivers (for WFxxx series)
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 wfx: prevent underflow in wfx_send_pds()

This does a "chunk_len - 4" subtraction later when it calls:

	ret = wfx_hif_configuration(wdev, buf + 4, chunk_len - 4);

so check for "chunk_len" is less than 4.

Fixes dcbecb497908 ("staging: wfx: allow new PDS format")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/silabs/wfx/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/silabs/wfx/main.c +1 -1
index e015bfb8d221..84d82ddded56 100644
@@ -181,7 +181,7 @@ int wfx_send_pds(struct wfx_dev *wdev, u8 *buf, size_t len)
 	while (len > 0) {
 		chunk_type = get_unaligned_le16(buf + 0);
 		chunk_len = get_unaligned_le16(buf + 2);
-		if (chunk_len > len) {
+		if (chunk_len < 4 || chunk_len > len) {
 			dev_err(wdev->dev, "PDS:%d: corrupted file\n", chunk_num);
 			return -EINVAL;
 		}
Signature · 1 line
2.35.1
maintainer Reviewed-by on v1 1/1
#1 lore raw
Quoted 5 lines · On Friday 19 August 2022 07:23:43 CEST Dan Carpenter wrote:

This does a "chunk_len - 4" subtraction later when it calls:

        ret = wfx_hif_configuration(wdev, buf + 4, chunk_len - 4);

so check for "chunk_len" is less than 4.

This patch also ensures that buf[4] won't overflow during:

    if (buf[4] != '{' || buf[chunk_len - 1] != '}')
    	dev_warn(wdev->dev, "PDS:%d: unexpected content\n", chunk_num);
Quoted 2 lines
Fixes dcbecb497908 ("staging: wfx: allow new PDS format")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jérôme Pouiller <[email protected]>
Signature · 1 line
Jérôme Pouiller
#2 lore raw
Quoted 9 lines · Jérôme Pouiller
Quoted 5 lines · On Friday 19 August 2022 07:23:43 CEST Dan Carpenter wrote:

This does a "chunk_len - 4" subtraction later when it calls:

        ret = wfx_hif_configuration(wdev, buf + 4, chunk_len - 4);

so check for "chunk_len" is less than 4.

This patch also ensures that buf[4] won't overflow during:

Quoted 7 lines
    if (buf[4] != '{' || buf[chunk_len - 1] != '}')
    	dev_warn(wdev->dev, "PDS:%d: unexpected content\n", chunk_num);
Quoted 2 lines
Fixes dcbecb497908 ("staging: wfx: allow new PDS format")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jérôme Pouiller <[email protected]>

BTW Jérôme, as you are the driver maintainer you can use Acked-by.

Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
maintainer
#3 lore raw
Quoted 14 lines · On Monday 29 August 2022 18:03:38 CEST Kalle Valo wrote:
Quoted 12 lines · Jérôme Pouiller
Quoted 5 lines · On Friday 19 August 2022 07:23:43 CEST Dan Carpenter wrote:

This does a "chunk_len - 4" subtraction later when it calls:

        ret = wfx_hif_configuration(wdev, buf + 4, chunk_len - 4);

so check for "chunk_len" is less than 4.

This patch also ensures that buf[4] won't overflow during:

    if (buf[4] != '{' || buf[chunk_len - 1] != '}')
      dev_warn(wdev->dev, "PDS:%d: unexpected content\n", chunk_num);
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Fixes dcbecb497908 ("staging: wfx: allow new PDS format")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jérôme Pouiller <[email protected]>

BTW Jérôme, as you are the driver maintainer you can use Acked-by.

Reviewed-by does not imply Acked-by?

Signature · 1 line
Jérôme Pouiller
#4 lore raw
Quoted 15 lines · Jérôme Pouiller
Quoted 14 lines · On Monday 29 August 2022 18:03:38 CEST Kalle Valo wrote:
Quoted 12 lines · Jérôme Pouiller
Quoted 5 lines · On Friday 19 August 2022 07:23:43 CEST Dan Carpenter wrote:

This does a "chunk_len - 4" subtraction later when it calls:

        ret = wfx_hif_configuration(wdev, buf + 4, chunk_len - 4);

so check for "chunk_len" is less than 4.

This patch also ensures that buf[4] won't overflow during:

    if (buf[4] != '{' || buf[chunk_len - 1] != '}')
      dev_warn(wdev->dev, "PDS:%d: unexpected content\n", chunk_num);
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Fixes dcbecb497908 ("staging: wfx: allow new PDS format")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jérôme Pouiller <[email protected]>

BTW Jérôme, as you are the driver maintainer you can use Acked-by.

Reviewed-by does not imply Acked-by?

Acked-by has "stronger" meaning and is meant to use by the maintainer of the code in question. So anyone can use Reviewed-by but only the maintainer should use Acked-by. My preference is that maintainers use Acked-by as then I can easily see from my patchwork script that the patch is ready to be applied.

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
#5 lore raw

Re: wifi: wfx: prevent underflow in wfx_send_pds()

In reply to Dan Carpenter
Quoted 9 lines · Dan Carpenter

This does a "chunk_len - 4" subtraction later when it calls:

	ret = wfx_hif_configuration(wdev, buf + 4, chunk_len - 4);

so check for "chunk_len" is less than 4.

Fixes dcbecb497908 ("staging: wfx: allow new PDS format")
Signed-off-by Dan Carpenter <[email protected]>
Reviewed-by Jérôme Pouiller <[email protected]>

Patch applied to wireless-next.git, thanks.

f97c81f5b7f8 wifi: wfx: prevent underflow in wfx_send_pds()

Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/patch/Yv8eX7Xv2ubUOvW7@kili/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches