2023-07-09 05:52:22

by Zhang Shurong

[permalink] [raw]
Subject: [PATCH] staging: ks7010: potential buffer overflow in ks_wlan_set_encode_ext()

The "exc->key_len" is a u16 that comes from the user. If it's over
IW_ENCODING_TOKEN_MAX (64) that could lead to memory corruption.

Fixes: b121d84882b9 ("staging: ks7010: simplify calls to memcpy()")

Signed-off-by: Zhang Shurong <[email protected]>
---
drivers/staging/ks7010/ks_wlan_net.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/ks7010/ks_wlan_net.c b/drivers/staging/ks7010/ks_wlan_net.c
index e03c87f0bfe7..0fb97a79ad0b 100644
--- a/drivers/staging/ks7010/ks_wlan_net.c
+++ b/drivers/staging/ks7010/ks_wlan_net.c
@@ -1583,8 +1583,10 @@ static int ks_wlan_set_encode_ext(struct net_device *dev,
commit |= SME_WEP_FLAG;
}
if (enc->key_len) {
- memcpy(&key->key_val[0], &enc->key[0], enc->key_len);
- key->key_len = enc->key_len;
+ int key_len = clamp_val(enc->key_len, 0, IW_ENCODING_TOKEN_MAX);
+
+ memcpy(&key->key_val[0], &enc->key[0], key_len);
+ key->key_len = key_len;
commit |= (SME_WEP_VAL1 << index);
}
break;
--
2.30.2



2023-07-09 20:12:17

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: ks7010: potential buffer overflow in ks_wlan_set_encode_ext()

On Sun, Jul 09, 2023 at 09:21:45PM +0200, Markus Elfring wrote:
> > The "exc->key_len" is a u16 that comes from the user. If it's over
> > IW_ENCODING_TOKEN_MAX (64) that could lead to memory corruption.
>
> Please choose an imperative change suggestion.

Please stop reviewing staging patches, it is not helpful for anyone.

greg k-h

2023-07-10 06:36:47

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH] staging: ks7010: potential buffer overflow in ks_wlan_set_encode_ext()

On Sun, Jul 09, 2023 at 01:50:07PM +0800, Zhang Shurong wrote:
> The "exc->key_len" is a u16 that comes from the user. If it's over
> IW_ENCODING_TOKEN_MAX (64) that could lead to memory corruption.
>
> Fixes: b121d84882b9 ("staging: ks7010: simplify calls to memcpy()")
>
> Signed-off-by: Zhang Shurong <[email protected]>

Reviewed-by: Dan Carpenter <[email protected]>

regards,
dan carpenter