[PATCH 0/7] staging: rtl8188eu: use safe version of list iterators

[PATCH 0/7] staging: rtl8188eu: use safe version of list iterators

[Dan Carpenter]

We converted a bunch of open coded list iterator loops into list_for_each()
but it turns out some of them should have been list_for_each_safe().  Or
even better list_for_each_entry_safe().

Btw, someone should get rid of the get_list_head() function.

regards,
dan carpenter

[PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode()

[Dan Carpenter]

This loop calls list_del_init() on the list iterator so it can result
in a forever loop.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/core/rtw_ap.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index bbecb07274f6..9399c17bfdbf 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1688,8 +1688,7 @@ void start_ap_mode(struct adapter *padapter)
 
 void stop_ap_mode(struct adapter *padapter)
 {
-	struct list_head *phead, *plist;
-	struct rtw_wlan_acl_node *paclnode;
+	struct rtw_wlan_acl_node *paclnode, *n;
 	struct sta_info *psta = NULL;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
@@ -1709,10 +1708,7 @@ void stop_ap_mode(struct adapter *padapter)
 
 	/* for ACL */
 	spin_lock_bh(&pacl_node_q->lock);
-	phead = get_list_head(pacl_node_q);
-	list_for_each(plist, phead) {
-		paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
-
+	list_for_each_entry_safe(paclnode, n, &pacl_node_q->queue, list) {
 		if (paclnode->valid) {
 			paclnode->valid = false;
 
-- 
2.30.2

[PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl()

[Dan Carpenter]

This loop calls list_del_init() on the list iterator so it has to use
the _safe() iterator or it leads to an endless loop.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
index b4d81d3a856c..42cfa1e95e2c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
@@ -5378,8 +5378,8 @@ u8 tx_beacon_hdl(struct adapter *padapter, unsigned char *pbuf)
 #ifdef CONFIG_88EU_AP_MODE
 	else { /* tx bc/mc frames after update TIM */
 		struct sta_info *psta_bmc;
-		struct list_head *xmitframe_plist, *xmitframe_phead;
-		struct xmit_frame *pxmitframe = NULL;
+		struct list_head *xmitframe_phead;
+		struct xmit_frame *pxmitframe, *n;
 		struct sta_priv *pstapriv = &padapter->stapriv;
 
 		/* for BC/MC Frames */
@@ -5392,11 +5392,8 @@ u8 tx_beacon_hdl(struct adapter *padapter, unsigned char *pbuf)
 			spin_lock_bh(&psta_bmc->sleep_q.lock);
 
 			xmitframe_phead = get_list_head(&psta_bmc->sleep_q);
-			list_for_each(xmitframe_plist, xmitframe_phead) {
-				pxmitframe = list_entry(xmitframe_plist,
-							struct xmit_frame,
-							list);
-
+			list_for_each_entry_safe(pxmitframe, n, xmitframe_phead,
+						 list) {
 				list_del_init(&pxmitframe->list);
 
 				psta_bmc->sleepq_len--;
-- 
2.30.2

[PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue()

[Dan Carpenter]

On some code paths the xmitframe_enqueue_for_sleeping_sta() function can
call list_del_init(&pxmitframe->list) which would lead to a forever loop
because "pxmitframe" is the list iterator.  Use the _safe version of the
iterator to prevent this.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/core/rtw_xmit.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index dcc29a74612d..f57e41f817ca 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -1729,17 +1729,15 @@ int xmitframe_enqueue_for_sleeping_sta(struct adapter *padapter, struct xmit_fra
 
 static void dequeue_xmitframes_to_sleeping_queue(struct adapter *padapter, struct sta_info *psta, struct __queue *pframequeue)
 {
-	struct list_head *plist, *phead;
+	struct list_head *phead;
 	u8	ac_index;
 	struct tx_servq	*ptxservq;
 	struct pkt_attrib	*pattrib;
-	struct xmit_frame	*pxmitframe;
+	struct xmit_frame	*pxmitframe, *n;
 	struct hw_xmit *phwxmits =  padapter->xmitpriv.hwxmits;
 
 	phead = get_list_head(pframequeue);
-	list_for_each(plist, phead) {
-		pxmitframe = list_entry(plist, struct xmit_frame, list);
-
+	list_for_each_entry_safe(pxmitframe, n, phead, list) {
 		xmitframe_enqueue_for_sleeping_sta(padapter, pxmitframe);
 
 		pattrib = &pxmitframe->attrib;
-- 
2.30.2

[PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit()

[Dan Carpenter]

These two loops call list_del_init() on the list iterator so they need to
use the _safe() iterator to prevent a forever loop.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/core/rtw_xmit.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index f57e41f817ca..d5489811c5bc 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -1796,17 +1796,14 @@ void wakeup_sta_to_xmit(struct adapter *padapter, struct sta_info *psta)
 {
 	u8 update_mask = 0, wmmps_ac = 0;
 	struct sta_info *psta_bmc;
-	struct list_head *xmitframe_plist, *xmitframe_phead;
-	struct xmit_frame *pxmitframe = NULL;
+	struct list_head *xmitframe_phead;
+	struct xmit_frame *pxmitframe, *n;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 
 	spin_lock_bh(&psta->sleep_q.lock);
 
 	xmitframe_phead = get_list_head(&psta->sleep_q);
-	list_for_each(xmitframe_plist, xmitframe_phead) {
-		pxmitframe = list_entry(xmitframe_plist, struct xmit_frame,
-					list);
-
+	list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
 		list_del_init(&pxmitframe->list);
 
 		switch (pxmitframe->attrib.priority) {
@@ -1881,10 +1878,7 @@ void wakeup_sta_to_xmit(struct adapter *padapter, struct sta_info *psta)
 		spin_lock_bh(&psta_bmc->sleep_q.lock);
 
 		xmitframe_phead = get_list_head(&psta_bmc->sleep_q);
-		list_for_each(xmitframe_plist, xmitframe_phead) {
-			pxmitframe = list_entry(xmitframe_plist,
-						struct xmit_frame, list);
-
+		list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
 			list_del_init(&pxmitframe->list);
 
 			psta_bmc->sleepq_len--;
-- 
2.30.2

[PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames()

[Dan Carpenter]

This loop calls list_del_init(&pxmitframe->list) and "pxmitframe" is the
list iterator so it leads to a forever loop.  We need to use a _safe()
iterator to fix this.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/core/rtw_xmit.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index d5489811c5bc..718dd20ff36c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -1912,17 +1912,14 @@ void wakeup_sta_to_xmit(struct adapter *padapter, struct sta_info *psta)
 void xmit_delivery_enabled_frames(struct adapter *padapter, struct sta_info *psta)
 {
 	u8 wmmps_ac = 0;
-	struct list_head *xmitframe_plist, *xmitframe_phead;
-	struct xmit_frame *pxmitframe = NULL;
+	struct list_head *xmitframe_phead;
+	struct xmit_frame *pxmitframe, *n;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 
 	spin_lock_bh(&psta->sleep_q.lock);
 
 	xmitframe_phead = get_list_head(&psta->sleep_q);
-	list_for_each(xmitframe_plist, xmitframe_phead) {
-		pxmitframe = list_entry(xmitframe_plist, struct xmit_frame,
-					list);
-
+	list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
 		switch (pxmitframe->attrib.priority) {
 		case 1:
 		case 2:
-- 
2.30.2

[PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete()

[Dan Carpenter]

This loop calls rtw_free_xmitframe(pxmitpriv, pxmitframe) which removes
"pxmitframe" (our list iterator) from the list.  So to prevent a forever
loop we need to use a safe list iterator.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 10a8dcc6ca95..19055a1a92c1 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -414,6 +414,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
 				  struct xmit_priv *pxmitpriv)
 {
 	struct xmit_frame *pxmitframe = NULL;
+	struct xmit_frame *n;
 	struct xmit_frame *pfirstframe = NULL;
 	struct xmit_buf *pxmitbuf;
 
@@ -422,7 +423,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
 	struct sta_info *psta = NULL;
 	struct tx_servq *ptxservq = NULL;
 
-	struct list_head *xmitframe_plist = NULL, *xmitframe_phead = NULL;
+	struct list_head *xmitframe_phead = NULL;
 
 	u32 pbuf;	/*  next pkt address */
 	u32 pbuf_tail;	/*  last pkt tail */
@@ -507,10 +508,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
 	spin_lock_bh(&pxmitpriv->lock);
 
 	xmitframe_phead = get_list_head(&ptxservq->sta_pending);
-	list_for_each(xmitframe_plist, xmitframe_phead) {
-		pxmitframe = list_entry(xmitframe_plist, struct xmit_frame,
-					list);
-
+	list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
 		pxmitframe->agg_num = 0; /*  not first frame of aggregation */
 		pxmitframe->pkt_offset = 0; /*  not first frame of aggregation, no need to reserve offset */
 
-- 
2.30.2

[PATCH 7/7] staging: rtl8188eu: delete some dead code

[Dan Carpenter]

Calling rtw_free_xmitframe() with a NULL "pxmitframe" parameter is a
no-op.  It appears that originally this code was part of a loop but it
was already dead code by the time that the driver was merged into the
kernel.

Fixes: 7bc88639ad36 ("staging: r8188eu: Add files for new driver - part 17")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
index 19055a1a92c1..d82dd22f2903 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
@@ -413,8 +413,7 @@ static u32 xmitframe_need_length(struct xmit_frame *pxmitframe)
 bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
 				  struct xmit_priv *pxmitpriv)
 {
-	struct xmit_frame *pxmitframe = NULL;
-	struct xmit_frame *n;
+	struct xmit_frame *pxmitframe, *n;
 	struct xmit_frame *pfirstframe = NULL;
 	struct xmit_buf *pxmitbuf;
 
@@ -443,8 +442,6 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
 		return false;
 
 	/* 3 1. pick up first frame */
-	rtw_free_xmitframe(pxmitpriv, pxmitframe);
-
 	pxmitframe = rtw_dequeue_xframe(pxmitpriv, pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
 	if (!pxmitframe) {
 		/*  no more xmit frame, release xmit buffer */
-- 
2.30.2

Re: [PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode()

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:17:11PM +0300, Dan Carpenter wrote:
> This loop calls list_del_init() on the list iterator so it can result
> in a forever loop.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index bbecb07274f6..9399c17bfdbf 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -1688,8 +1688,7 @@ void start_ap_mode(struct adapter *padapter)
>  
>  void stop_ap_mode(struct adapter *padapter)
>  {
> -	struct list_head *phead, *plist;
> -	struct rtw_wlan_acl_node *paclnode;
> +	struct rtw_wlan_acl_node *paclnode, *n;
>  	struct sta_info *psta = NULL;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> @@ -1709,10 +1708,7 @@ void stop_ap_mode(struct adapter *padapter)
>  
>  	/* for ACL */
>  	spin_lock_bh(&pacl_node_q->lock);
> -	phead = get_list_head(pacl_node_q);
> -	list_for_each(plist, phead) {
> -		paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
> -
> +	list_for_each_entry_safe(paclnode, n, &pacl_node_q->queue, list) {
>  		if (paclnode->valid) {
>  			paclnode->valid = false;
>  
> -- 
> 2.30.2
> 

Re: [PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl()

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:17:30PM +0300, Dan Carpenter wrote:
> This loop calls list_del_init() on the list iterator so it has to use
> the _safe() iterator or it leads to an endless loop.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> index b4d81d3a856c..42cfa1e95e2c 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme_ext.c
> @@ -5378,8 +5378,8 @@ u8 tx_beacon_hdl(struct adapter *padapter, unsigned char *pbuf)
>  #ifdef CONFIG_88EU_AP_MODE
>  	else { /* tx bc/mc frames after update TIM */
>  		struct sta_info *psta_bmc;
> -		struct list_head *xmitframe_plist, *xmitframe_phead;
> -		struct xmit_frame *pxmitframe = NULL;
> +		struct list_head *xmitframe_phead;
> +		struct xmit_frame *pxmitframe, *n;
>  		struct sta_priv *pstapriv = &padapter->stapriv;
>  
>  		/* for BC/MC Frames */
> @@ -5392,11 +5392,8 @@ u8 tx_beacon_hdl(struct adapter *padapter, unsigned char *pbuf)
>  			spin_lock_bh(&psta_bmc->sleep_q.lock);
>  
>  			xmitframe_phead = get_list_head(&psta_bmc->sleep_q);
> -			list_for_each(xmitframe_plist, xmitframe_phead) {
> -				pxmitframe = list_entry(xmitframe_plist,
> -							struct xmit_frame,
> -							list);
> -
> +			list_for_each_entry_safe(pxmitframe, n, xmitframe_phead,
> +						 list) {
>  				list_del_init(&pxmitframe->list);
>  
>  				psta_bmc->sleepq_len--;
> -- 
> 2.30.2
> 

Re: [PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue()

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:17:43PM +0300, Dan Carpenter wrote:
> On some code paths the xmitframe_enqueue_for_sleeping_sta() function can
> call list_del_init(&pxmitframe->list) which would lead to a forever loop
> because "pxmitframe" is the list iterator.  Use the _safe version of the
> iterator to prevent this.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/staging/rtl8188eu/core/rtw_xmit.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index dcc29a74612d..f57e41f817ca 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -1729,17 +1729,15 @@ int xmitframe_enqueue_for_sleeping_sta(struct adapter *padapter, struct xmit_fra
>  
>  static void dequeue_xmitframes_to_sleeping_queue(struct adapter *padapter, struct sta_info *psta, struct __queue *pframequeue)
>  {
> -	struct list_head *plist, *phead;
> +	struct list_head *phead;
>  	u8	ac_index;
>  	struct tx_servq	*ptxservq;
>  	struct pkt_attrib	*pattrib;
> -	struct xmit_frame	*pxmitframe;
> +	struct xmit_frame	*pxmitframe, *n;
>  	struct hw_xmit *phwxmits =  padapter->xmitpriv.hwxmits;
>  
>  	phead = get_list_head(pframequeue);
> -	list_for_each(plist, phead) {
> -		pxmitframe = list_entry(plist, struct xmit_frame, list);
> -
> +	list_for_each_entry_safe(pxmitframe, n, phead, list) {
>  		xmitframe_enqueue_for_sleeping_sta(padapter, pxmitframe);
>  
>  		pattrib = &pxmitframe->attrib;
> -- 
> 2.30.2
> 

Re: [PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit()

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:17:57PM +0300, Dan Carpenter wrote:
> These two loops call list_del_init() on the list iterator so they need to
> use the _safe() iterator to prevent a forever loop.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/staging/rtl8188eu/core/rtw_xmit.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index f57e41f817ca..d5489811c5bc 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -1796,17 +1796,14 @@ void wakeup_sta_to_xmit(struct adapter *padapter, struct sta_info *psta)
>  {
>  	u8 update_mask = 0, wmmps_ac = 0;
>  	struct sta_info *psta_bmc;
> -	struct list_head *xmitframe_plist, *xmitframe_phead;
> -	struct xmit_frame *pxmitframe = NULL;
> +	struct list_head *xmitframe_phead;
> +	struct xmit_frame *pxmitframe, *n;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  
>  	spin_lock_bh(&psta->sleep_q.lock);
>  
>  	xmitframe_phead = get_list_head(&psta->sleep_q);
> -	list_for_each(xmitframe_plist, xmitframe_phead) {
> -		pxmitframe = list_entry(xmitframe_plist, struct xmit_frame,
> -					list);
> -
> +	list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
>  		list_del_init(&pxmitframe->list);
>  
>  		switch (pxmitframe->attrib.priority) {
> @@ -1881,10 +1878,7 @@ void wakeup_sta_to_xmit(struct adapter *padapter, struct sta_info *psta)
>  		spin_lock_bh(&psta_bmc->sleep_q.lock);
>  
>  		xmitframe_phead = get_list_head(&psta_bmc->sleep_q);
> -		list_for_each(xmitframe_plist, xmitframe_phead) {
> -			pxmitframe = list_entry(xmitframe_plist,
> -						struct xmit_frame, list);
> -
> +		list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
>  			list_del_init(&pxmitframe->list);
>  
>  			psta_bmc->sleepq_len--;
> -- 
> 2.30.2
> 

Re: [PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames()

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:18:13PM +0300, Dan Carpenter wrote:
> This loop calls list_del_init(&pxmitframe->list) and "pxmitframe" is the
> list iterator so it leads to a forever loop.  We need to use a _safe()
> iterator to fix this.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/staging/rtl8188eu/core/rtw_xmit.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index d5489811c5bc..718dd20ff36c 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -1912,17 +1912,14 @@ void wakeup_sta_to_xmit(struct adapter *padapter, struct sta_info *psta)
>  void xmit_delivery_enabled_frames(struct adapter *padapter, struct sta_info *psta)
>  {
>  	u8 wmmps_ac = 0;
> -	struct list_head *xmitframe_plist, *xmitframe_phead;
> -	struct xmit_frame *pxmitframe = NULL;
> +	struct list_head *xmitframe_phead;
> +	struct xmit_frame *pxmitframe, *n;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  
>  	spin_lock_bh(&psta->sleep_q.lock);
>  
>  	xmitframe_phead = get_list_head(&psta->sleep_q);
> -	list_for_each(xmitframe_plist, xmitframe_phead) {
> -		pxmitframe = list_entry(xmitframe_plist, struct xmit_frame,
> -					list);
> -
> +	list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
>  		switch (pxmitframe->attrib.priority) {
>  		case 1:
>  		case 2:
> -- 
> 2.30.2
> 

Re: [PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete()

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:18:25PM +0300, Dan Carpenter wrote:
> This loop calls rtw_free_xmitframe(pxmitpriv, pxmitframe) which removes
> "pxmitframe" (our list iterator) from the list.  So to prevent a forever
> loop we need to use a safe list iterator.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> index 10a8dcc6ca95..19055a1a92c1 100644
> --- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> +++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> @@ -414,6 +414,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>  				  struct xmit_priv *pxmitpriv)
>  {
>  	struct xmit_frame *pxmitframe = NULL;
> +	struct xmit_frame *n;
>  	struct xmit_frame *pfirstframe = NULL;
>  	struct xmit_buf *pxmitbuf;
>  
> @@ -422,7 +423,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>  	struct sta_info *psta = NULL;
>  	struct tx_servq *ptxservq = NULL;
>  
> -	struct list_head *xmitframe_plist = NULL, *xmitframe_phead = NULL;
> +	struct list_head *xmitframe_phead = NULL;
>  
>  	u32 pbuf;	/*  next pkt address */
>  	u32 pbuf_tail;	/*  last pkt tail */
> @@ -507,10 +508,7 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>  	spin_lock_bh(&pxmitpriv->lock);
>  
>  	xmitframe_phead = get_list_head(&ptxservq->sta_pending);
> -	list_for_each(xmitframe_plist, xmitframe_phead) {
> -		pxmitframe = list_entry(xmitframe_plist, struct xmit_frame,
> -					list);
> -
> +	list_for_each_entry_safe(pxmitframe, n, xmitframe_phead, list) {
>  		pxmitframe->agg_num = 0; /*  not first frame of aggregation */
>  		pxmitframe->pkt_offset = 0; /*  not first frame of aggregation, no need to reserve offset */
>  
> -- 
> 2.30.2
> 

Re: [PATCH 7/7] staging: rtl8188eu: delete some dead code

[Guenter Roeck]

On Mon, Jun 07, 2021 at 09:19:00PM +0300, Dan Carpenter wrote:
> Calling rtw_free_xmitframe() with a NULL "pxmitframe" parameter is a
> no-op.  It appears that originally this code was part of a loop but it
> was already dead code by the time that the driver was merged into the
> kernel.
> 
> Fixes: 7bc88639ad36 ("staging: r8188eu: Add files for new driver - part 17")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
>  drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> index 19055a1a92c1..d82dd22f2903 100644
> --- a/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> +++ b/drivers/staging/rtl8188eu/hal/rtl8188eu_xmit.c
> @@ -413,8 +413,7 @@ static u32 xmitframe_need_length(struct xmit_frame *pxmitframe)
>  bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>  				  struct xmit_priv *pxmitpriv)
>  {
> -	struct xmit_frame *pxmitframe = NULL;
> -	struct xmit_frame *n;
> +	struct xmit_frame *pxmitframe, *n;
>  	struct xmit_frame *pfirstframe = NULL;
>  	struct xmit_buf *pxmitbuf;
>  
> @@ -443,8 +442,6 @@ bool rtl8188eu_xmitframe_complete(struct adapter *adapt,
>  		return false;
>  
>  	/* 3 1. pick up first frame */

Comment could be removed as well. Otherwise,

Thanks a lot for taking care of this, and sorry again for the mess I created.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> -	rtw_free_xmitframe(pxmitpriv, pxmitframe);
> -
>  	pxmitframe = rtw_dequeue_xframe(pxmitpriv, pxmitpriv->hwxmits, pxmitpriv->hwxmit_entry);
>  	if (!pxmitframe) {
>  		/*  no more xmit frame, release xmit buffer */
> -- 
> 2.30.2
>