Re: [PATCH] ath9k_htc: fix potential out of bounds access with invalid rxstatus->rs_keyix

[Dan Carpenter <dan.carpenter@oracle.com>]

On Sat, Apr 09, 2022 at 09:53:53AM +0200, Oleksij Rempel wrote:
> > diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> > index 6a850a0bfa8a..a23eaca0326d 100644
> > --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> > +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> > @@ -1016,6 +1016,14 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,
> >   		goto rx_next;
> >   	}
> > 
> > +	if (rxstatus->rs_keyix >= ATH_KEYMAX &&
> > +	    rxstatus->rs_keyix != ATH9K_RXKEYIX_INVALID) {
> > +		ath_dbg(common, ANY,
> > +			"Invalid keyix, dropping (keyix: %d)\n",
> > +			rxstatus->rs_keyix);
> > +		goto rx_next;
> > +	}
> > +
> >   	/* Get the RX status information */
> > 
> >   	memset(rx_status, 0, sizeof(struct ieee80211_rx_status));
> 
> Looks ok to me.

Thanks!

> 
> By the way, rs_keyix seems to have a wrong type. It is declared as u8, but used as s8.

That sounds like something outside the scope of the patch...
Why do you mean "used as s8"?  Which function are you talking about?

You made me panic briefly because ATH9K_RXKEYIX_INVALID is a u8 so it
needs to be u8.  I would have thought instinctively that u8 would be the
right type for an index like this.

regards,
dan carpenter