Re: [PATCH] staging: rtl8188eu: Avoid null pointer arithmetic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
---
  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@xxxxxxxxxxxx>

Thanks,

Larry

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel



[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux