The ath6kl_cfg80211_add_key() function has an upper bounds check on
params->key_len which ensures that it can't go over WLAN_MAX_KEY_LEN but
it doesn't check for negatives.  This could potentially lead to memory
corruption.

Put a bounds check on negative values in cfg80211_validate_key_settings()
to prevent this sort of bug in the future.

Fixes: bdcd81707973 ("Add ath6kl cleaned up driver")
Cc: stable@vger.kernel.org
Signed-off-by: Dan Carpenter <error27@gmail.com>
---
This is from static analysis.  I can't think why a driver would ever
want a negative length and I think this is the safest solution.  But
I have not tested it.

 net/wireless/util.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/wireless/util.c b/net/wireless/util.c
index b78530c3e3f8..4552229eb2d2 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -397,6 +397,8 @@ int cfg80211_validate_key_settings(struct cfg80211_registered_device *rdev,
 		 * or not the driver supports this algorithm,
 		 * of course.
 		 */
+		if (params->key_len < 0)
+			return -EINVAL;
 		break;
 	}
 
-- 
2.53.0

On Thu, 2026-04-30 at 09:15 +0300, Dan Carpenter wrote:
> The ath6kl_cfg80211_add_key() function has an upper bounds check on
> params->key_len which ensures that it can't go over WLAN_MAX_KEY_LEN but
> it doesn't check for negatives.  This could potentially lead to memory
> corruption.
> 
> Put a bounds check on negative values in cfg80211_validate_key_settings()
> to prevent this sort of bug in the future.

Clearly this commit doesn't seem problematic, but I'm not sure I see the
path to it mattering? The key_len should only ever be set by
wext/nl80211, and that can't really end up with a negative length?

We should probably just make it a u8 there, no way it's ever bigger than
that, but I'm not seeing through why this would matter much right now.

johannes

On Thu, Apr 30, 2026 at 09:05:35AM +0200, Johannes Berg wrote:
> On Thu, 2026-04-30 at 09:15 +0300, Dan Carpenter wrote:
> > The ath6kl_cfg80211_add_key() function has an upper bounds check on
> > params->key_len which ensures that it can't go over WLAN_MAX_KEY_LEN but
> > it doesn't check for negatives.  This could potentially lead to memory
> > corruption.
> > 
> > Put a bounds check on negative values in cfg80211_validate_key_settings()
> > to prevent this sort of bug in the future.
> 
> Clearly this commit doesn't seem problematic, but I'm not sure I see the
> path to it mattering? The key_len should only ever be set by
> wext/nl80211, and that can't really end up with a negative length?
> 
> We should probably just make it a u8 there, no way it's ever bigger than
> that, but I'm not seeing through why this would matter much right now.

Ah.  Yeah.  You're right.  :/  Sorry for the noise.

regards,
dan carpenter