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(-)
diff --git a/drivers/net/wireless/silabs/wfx/main.c b/drivers/net/wireless/silabs/wfx/main.c
index e015bfb8d221..84d82ddded56 100644
--- a/drivers/net/wireless/silabs/wfx/main.c
+++ b/drivers/net/wireless/silabs/wfx/main.c
@@ -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;
}
--
2.35.1
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);
> Fixes: dcbecb497908 ("staging: wfx: allow new PDS format")
> Signed-off-by: Dan Carpenter <[email protected]>
Reviewed-by: J?r?me Pouiller <[email protected]>
--
J?r?me Pouiller
Jérôme Pouiller <[email protected]> writes:
> 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);
>
>> 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.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Monday 29 August 2022 18:03:38 CEST Kalle Valo wrote:
> J?r?me Pouiller <[email protected]> writes:
>
> > 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);
> >
> >> 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?
--
J?r?me Pouiller
Jérôme Pouiller <[email protected]> writes:
> On Monday 29 August 2022 18:03:38 CEST Kalle Valo wrote:
>> Jérôme Pouiller <[email protected]> writes:
>>
>> > 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);
>> >
>> >> 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
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Dan Carpenter <[email protected]> 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.
>
> 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()
--
https://patchwork.kernel.org/project/linux-wireless/patch/Yv8eX7Xv2ubUOvW7@kili/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches