linux-kernel - Re: [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 30 May 2022 11:05:57 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Larry Finger <Larry.Finger@...inger.net>
Cc:     gregkh@...uxfoundation.org, phil@...lpotter.co.uk,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] staging: r8188eu: Fix undersized array in
 rtw_xmit.c

On Sat, May 28, 2022 at 07:47:11PM -0500, Larry Finger wrote:
> Compiling with -warray-bounds yields the following warning:
> 
> drivers/staging/r8188eu/core/rtw_xmit.c: In function ‘rtw_alloc_hwxmits’:
> drivers/staging/r8188eu/core/rtw_xmit.c:1493:24: warning: array subscript 4 is outside array bounds of ‘void[64]’ [-Warray-bounds]
>  1493 |                 hwxmits[4] .sta_queue = &pxmitpriv->be_pending;
>       |                 ~~~~~~~^~~

This patch is not correct.  The hwxmits[4] is nonsense dead code.  It
has an extra element and the other four element are in the wrong order.
If we applied this patch then we would have to update rtw_get_sta_pending()
to re-order the *(ac) = 2; assignments to match the format.  And to set
*ac = 4 on one path.

The correct fix is to just delete the dead code.

regards,
dan carpenter

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 3d8e9dea7651..2a93e7fa1142 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -1483,19 +1483,10 @@ int rtw_alloc_hwxmits(struct adapter *padapter)
 
 	hwxmits = pxmitpriv->hwxmits;
 
-	if (pxmitpriv->hwxmit_entry == 5) {
-		hwxmits[0] .sta_queue = &pxmitpriv->bm_pending;
-		hwxmits[1] .sta_queue = &pxmitpriv->vo_pending;
-		hwxmits[2] .sta_queue = &pxmitpriv->vi_pending;
-		hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
-		hwxmits[4] .sta_queue = &pxmitpriv->be_pending;
-	} else if (pxmitpriv->hwxmit_entry == 4) {
-		hwxmits[0] .sta_queue = &pxmitpriv->vo_pending;
-		hwxmits[1] .sta_queue = &pxmitpriv->vi_pending;
-		hwxmits[2] .sta_queue = &pxmitpriv->be_pending;
-		hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
-	} else {
-	}
+	hwxmits[0] .sta_queue = &pxmitpriv->vo_pending;
+	hwxmits[1] .sta_queue = &pxmitpriv->vi_pending;
+	hwxmits[2] .sta_queue = &pxmitpriv->be_pending;
+	hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
 
 	return 0;
 }