| Message ID | 20191016205716.2843-1-labbott@redhat.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | rtlwifi: Fix potential overflow on P2P code | expand |
> -----Original Message----- > From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-owner@vger.kernel.org] On Behalf > Of Laura Abbott > Sent: Thursday, October 17, 2019 4:57 AM > To: Pkshih; Kalle Valo > Cc: Laura Abbott; David S. Miller; linux-wireless@vger.kernel.org; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org; Nicolas Waisman > Subject: [PATCH] rtlwifi: Fix potential overflow on P2P code > > Nicolas Waisman noticed that even though noa_len is checked for > a compatible length it's still possible to overrun the buffers > of p2pinfo since there's no check on the upper bound of noa_num. > Bounds check noa_num against P2P_MAX_NOA_NUM. > > Reported-by: Nicolas Waisman <nico@semmle.com> > Signed-off-by: Laura Abbott <labbott@redhat.com> > --- > Compile tested only as this was reported to the security list. > --- > drivers/net/wireless/realtek/rtlwifi/ps.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/drivers/net/wireless/realtek/rtlwifi/ps.c b/drivers/net/wireless/realtek/rtlwifi/ps.c > index 70f04c2f5b17..c5cff598383d 100644 > --- a/drivers/net/wireless/realtek/rtlwifi/ps.c > +++ b/drivers/net/wireless/realtek/rtlwifi/ps.c > @@ -754,6 +754,13 @@ static void rtl_p2p_noa_ie(struct ieee80211_hw *hw, void *data, > return; > } else { > noa_num = (noa_len - 2) / 13; > + if (noa_num > P2P_MAX_NOA_NUM) { > + RT_TRACE(rtlpriv, COMP_INIT, DBG_LOUD, > + "P2P notice of absence: invalid noa_num.%d\n", > + noa_num); > + return; As the discussion at <security@kernel.org>, I think it'd be better to use the min between noa_num and P2P_MAX_NOA_NUM, and fall through the code instead of return. Because ignore all NoA isn't better than apply two of them. > + } > + > } > noa_index = ie[3]; > if (rtlpriv->psc.p2p_ps_info.p2p_ps_mode == > @@ -848,6 +855,13 @@ static void rtl_p2p_action_ie(struct ieee80211_hw *hw, void *data, > return; > } else { > noa_num = (noa_len - 2) / 13; > + if (noa_num > P2P_MAX_NOA_NUM) { > + RT_TRACE(rtlpriv, COMP_FW, DBG_LOUD, > + "P2P notice of absence: invalid noa_len.%d\n", > + noa_len); > + return; > + > + } > } > noa_index = ie[3]; > if (rtlpriv->psc.p2p_ps_info.p2p_ps_mode == > -- > 2.21.0
diff --git a/drivers/net/wireless/realtek/rtlwifi/ps.c b/drivers/net/wireless/realtek/rtlwifi/ps.c index 70f04c2f5b17..c5cff598383d 100644 --- a/drivers/net/wireless/realtek/rtlwifi/ps.c +++ b/drivers/net/wireless/realtek/rtlwifi/ps.c @@ -754,6 +754,13 @@ static void rtl_p2p_noa_ie(struct ieee80211_hw *hw, void *data, return; } else { noa_num = (noa_len - 2) / 13; + if (noa_num > P2P_MAX_NOA_NUM) { + RT_TRACE(rtlpriv, COMP_INIT, DBG_LOUD, + "P2P notice of absence: invalid noa_num.%d\n", + noa_num); + return; + } + } noa_index = ie[3]; if (rtlpriv->psc.p2p_ps_info.p2p_ps_mode == @@ -848,6 +855,13 @@ static void rtl_p2p_action_ie(struct ieee80211_hw *hw, void *data, return; } else { noa_num = (noa_len - 2) / 13; + if (noa_num > P2P_MAX_NOA_NUM) { + RT_TRACE(rtlpriv, COMP_FW, DBG_LOUD, + "P2P notice of absence: invalid noa_len.%d\n", + noa_len); + return; + + } } noa_index = ie[3]; if (rtlpriv->psc.p2p_ps_info.p2p_ps_mode ==
Nicolas Waisman noticed that even though noa_len is checked for a compatible length it's still possible to overrun the buffers of p2pinfo since there's no check on the upper bound of noa_num. Bounds check noa_num against P2P_MAX_NOA_NUM. Reported-by: Nicolas Waisman <nico@semmle.com> Signed-off-by: Laura Abbott <labbott@redhat.com> --- Compile tested only as this was reported to the security list. --- drivers/net/wireless/realtek/rtlwifi/ps.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)