* [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic @ 2018-09-27 17:04 Aymen Qader 2018-09-27 20:52 ` Larry Finger 0 siblings, 1 reply; 3+ messages in thread From: Aymen Qader @ 2018-09-27 17:04 UTC (permalink / raw) Cc: Larry Finger, Aymen Qader, Greg Kroah-Hartman, devel, linux-kernel Avoid null pointer arithmetic in rtw_mlme_ext.c by skipping other field checks if the information element pointer is null. Signed-off-by: Aymen Qader <qader.aymen@gmail.com> --- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index 834053a0ae9d..8a3a71456cd0 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -2971,8 +2971,10 @@ static unsigned int OnAssocReq(struct adapter *padapter, /* checking SSID */ p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len, pkt_len - WLAN_HDR_A3_LEN - ie_offset); - if (!p) + if (!p) { status = _STATS_FAILURE_; + goto OnAssocReqFail; + } if (ie_len == 0) { /* broadcast ssid, however it is not allowed in assocreq */ status = _STATS_FAILURE_; -- 2.17.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic 2018-09-27 17:04 [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic Aymen Qader @ 2018-09-27 20:52 ` Larry Finger 2018-09-27 21:12 ` Aymen Qader 0 siblings, 1 reply; 3+ messages in thread From: Larry Finger @ 2018-09-27 20:52 UTC (permalink / raw) To: Aymen Qader; +Cc: Greg Kroah-Hartman, devel, linux-kernel On 9/27/18 12:04 PM, Aymen Qader wrote: > Avoid null pointer arithmetic in rtw_mlme_ext.c by skipping other field > checks if the information element pointer is null. > > Signed-off-by: Aymen Qader <qader.aymen@gmail.com> > --- > drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > index 834053a0ae9d..8a3a71456cd0 100644 > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > @@ -2971,8 +2971,10 @@ static unsigned int OnAssocReq(struct adapter *padapter, > /* checking SSID */ > p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len, > pkt_len - WLAN_HDR_A3_LEN - ie_offset); > - if (!p) > + if (!p) { > status = _STATS_FAILURE_; > + goto OnAssocReqFail; > + } > > if (ie_len == 0) { /* broadcast ssid, however it is not allowed in assocreq */ > status = _STATS_FAILURE_; I do not think this patch avoids any pointer arithmetic. If p is NULL, then ie_len will be zero and the branch with the memcmp() call, where the pointer arithmetic is done, will be skipped. That said, it is better to bail out with the first failure condition. I do not require the following, but the code would be even simpler if you test p and ie_len==0 in a single if statement and eliminate some code as in diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c index 1115050077e4..71722cec84a0 100644 --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c @@ -2982,11 +2982,10 @@ static unsigned int OnAssocReq(struct adapter *padapter, /* checking SSID */ p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len, pkt_len - WLAN_HDR_A3_LEN - ie_offset); - if (!p) - status = _STATS_FAILURE_; - if (ie_len == 0) { /* broadcast ssid, however it is not allowed in assocreq */ + if (!p || ie_len == 0) { /* broadcast ssid, however it is not allowed in assocreq */ status = _STATS_FAILURE_; + goto OnAssocReqFail; } else { /* check if ssid match */ if (memcmp((void *)(p+2), cur->Ssid.Ssid, cur->Ssid.SsidLength)) ACKed-by: Larry Finger <Larry.Finger@lwfinger.net> Thanks, Larry ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic 2018-09-27 20:52 ` Larry Finger @ 2018-09-27 21:12 ` Aymen Qader 0 siblings, 0 replies; 3+ messages in thread From: Aymen Qader @ 2018-09-27 21:12 UTC (permalink / raw) To: Larry Finger; +Cc: Greg Kroah-Hartman, devel, linux-kernel On Thu, Sep 27, 2018 at 03:52:53PM -0500, Larry Finger wrote: > On 9/27/18 12:04 PM, Aymen Qader wrote: > > Avoid null pointer arithmetic in rtw_mlme_ext.c by skipping other field > > checks if the information element pointer is null. > > > > Signed-off-by: Aymen Qader <qader.aymen@gmail.com> > > --- > > drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > index 834053a0ae9d..8a3a71456cd0 100644 > > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > > @@ -2971,8 +2971,10 @@ static unsigned int OnAssocReq(struct adapter *padapter, > > /* checking SSID */ > > p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len, > > pkt_len - WLAN_HDR_A3_LEN - ie_offset); > > - if (!p) > > + if (!p) { > > status = _STATS_FAILURE_; > > + goto OnAssocReqFail; > > + } > > if (ie_len == 0) { /* broadcast ssid, however it is not allowed in assocreq */ > > status = _STATS_FAILURE_; > > I do not think this patch avoids any pointer arithmetic. If p is NULL, then > ie_len will be zero and the branch with the memcmp() call, where the pointer > arithmetic is done, will be skipped. I'm sincerely sorry, you're completely right--that was a bad oversight from me, I should have checked more thoroughly. > > That said, it is better to bail out with the first failure condition. I do > not require the following, but the code would be even simpler if you test p > and ie_len==0 in a single if statement and eliminate some code as in > > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > index 1115050077e4..71722cec84a0 100644 > --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c > @@ -2982,11 +2982,10 @@ static unsigned int OnAssocReq(struct adapter *padapter, > /* checking SSID */ > p = rtw_get_ie(pframe + WLAN_HDR_A3_LEN + ie_offset, _SSID_IE_, &ie_len, > pkt_len - WLAN_HDR_A3_LEN - ie_offset); > - if (!p) > - status = _STATS_FAILURE_; > > - if (ie_len == 0) { /* broadcast ssid, however it is not allowed in > assocreq */ > + if (!p || ie_len == 0) { /* broadcast ssid, however it is not > allowed in assocreq */ > status = _STATS_FAILURE_; > + goto OnAssocReqFail; > } else { > /* check if ssid match */ > if (memcmp((void *)(p+2), cur->Ssid.Ssid, cur->Ssid.SsidLength)) > Yep, I understand that would be a lot better. If it's alright, I'll send this in with a v2 (w/ a more appropriate commit message). > > ACKed-by: Larry Finger <Larry.Finger@lwfinger.net> > > Thanks, > > Larry > Kind regards, Aymen ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-09-27 21:13 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-27 17:04 [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic Aymen Qader 2018-09-27 20:52 ` Larry Finger 2018-09-27 21:12 ` Aymen Qader
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
Powered by JetHome