Message ID | 20190402070338.GB15171@kadam (mailing list archive) |
---|---|
State | Accepted |
Commit | 2cd2b42439ea7d1231b6b3f0eb0fe606f2ba5160 |
Delegated to: | Kalle Valo |
Headers | show |
Series | mwifiex: add a bounds check in mwifiex_process_sta_rx_packet() | expand |
On Tue, Apr 2, 2019 at 12:03 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > Smatch complains that "local_rx_pd->priority" can't be trusted because > it comes from skb->data and it can go up to 255 instead of being capped > in the 0-7 range. A few lines earlier, on the other side of the if > statement, we cap priority so it seems harmless to add a bounds check > here as well. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Seems right to me: Reviewed-by: Brian Norris <briannorris@chromium.org>
Dan Carpenter <dan.carpenter@oracle.com> wrote: > Smatch complains that "local_rx_pd->priority" can't be trusted because > it comes from skb->data and it can go up to 255 instead of being capped > in the 0-7 range. A few lines earlier, on the other side of the if > statement, we cap priority so it seems harmless to add a bounds check > here as well. > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> > Reviewed-by: Brian Norris <briannorris@chromium.org> Patch applied to wireless-drivers-next.git, thanks. 2cd2b42439ea mwifiex: add a bounds check in mwifiex_process_sta_rx_packet()
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_rx.c b/drivers/net/wireless/marvell/mwifiex/sta_rx.c index fb28a5c7f441..52a2ce2e78b0 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_rx.c +++ b/drivers/net/wireless/marvell/mwifiex/sta_rx.c @@ -250,7 +250,8 @@ int mwifiex_process_sta_rx_packet(struct mwifiex_private *priv, local_rx_pd->nf); } } else { - if (rx_pkt_type != PKT_TYPE_BAR) + if (rx_pkt_type != PKT_TYPE_BAR && + local_rx_pd->priority < MAX_NUM_TID) priv->rx_seq[local_rx_pd->priority] = seq_num; memcpy(ta, priv->curr_bss_params.bss_descriptor.mac_address, ETH_ALEN);
Smatch complains that "local_rx_pd->priority" can't be trusted because it comes from skb->data and it can go up to 255 instead of being capped in the 0-7 range. A few lines earlier, on the other side of the if statement, we cap priority so it seems harmless to add a bounds check here as well. Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> --- drivers/net/wireless/marvell/mwifiex/sta_rx.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)