[PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Martin Kaiser]

rtw_free_network_queue iterates over the scanned wireless networks and
calls _rtw_free_network for each of them. In some cases,
_rtw_free_network removes a network from the list.

We have to use list_for_each_safe if we remove list entries while we
iterate over a list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
Without this patch, it's easy to get the driver into an endless loop by
scanning, connecting and disconnecting repeatedly.

wpa_supplicant -B -Dwext -i wlan0 -c /path/to/my/config...
while true ; do
   sleep 1
   wpa_cli reconfigure     
   wpa_cli add_network
   wpa_cli set_network 0 ssid ...
   wpa_cli set_network 0 psk ...
   wpa_cli select_network 0
   sleep 6
   wpa_cli status
   wpa_cli disconnect 0
done

 drivers/staging/rtl8188eu/core/rtw_mlme.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 159465b073c2..14816ad51668 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -199,7 +199,7 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 
 void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
 {
-	struct list_head *phead, *plist;
+	struct list_head *phead, *plist, *temp;
 	struct wlan_network *pnetwork;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct __queue *scanned_queue = &pmlmepriv->scanned_queue;
@@ -207,7 +207,7 @@ void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
 	spin_lock_bh(&scanned_queue->lock);
 
 	phead = get_list_head(scanned_queue);
-	list_for_each(plist, phead) {
+	list_for_each_safe(plist, temp, phead) {
 		pnetwork = list_entry(plist, struct wlan_network, list);
 
 		_rtw_free_network(pmlmepriv, pnetwork, isfreeall);
-- 
2.20.1

[PATCH 2/6] staging: rtl8188eu: use safe iterator in rtw_free_all_stainfo

[Martin Kaiser]

This is another case where we may remove list entries while we iterate over
the list. Use list_for_each_safe to avoid an endless loop.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
Without this patch, unloading the module goes into an endless loop
sometimes.

 drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
index 7941ca0397ed..eda7a441acde 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
@@ -379,7 +379,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
 /*  free all stainfo which in sta_hash[all] */
 void rtw_free_all_stainfo(struct adapter *padapter)
 {
-	struct list_head *plist, *phead;
+	struct list_head *plist, *phead, *temp;
 	s32 index;
 	struct sta_info *psta = NULL;
 	struct sta_priv *pstapriv = &padapter->stapriv;
@@ -392,7 +392,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
 
 	for (index = 0; index < NUM_STA; index++) {
 		phead = &pstapriv->sta_hash[index];
-		list_for_each(plist, phead) {
+		list_for_each_safe(plist, temp, phead) {
 			psta = list_entry(plist, struct sta_info, hash_list);
 
 			if (pbcmc_stainfo != psta)
-- 
2.20.1

Re: [PATCH 2/6] staging: rtl8188eu: use safe iterator in rtw_free_all_stainfo

[Guenter Roeck]

On Sun, May 16, 2021 at 06:06:09PM +0200, Martin Kaiser wrote:
> This is another case where we may remove list entries while we iterate over
> the list. Use list_for_each_safe to avoid an endless loop.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> Without this patch, unloading the module goes into an endless loop
> sometimes.
> 
>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> index 7941ca0397ed..eda7a441acde 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> @@ -379,7 +379,7 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
>  /*  free all stainfo which in sta_hash[all] */
>  void rtw_free_all_stainfo(struct adapter *padapter)
>  {
> -	struct list_head *plist, *phead;
> +	struct list_head *plist, *phead, *temp;
>  	s32 index;
>  	struct sta_info *psta = NULL;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
> @@ -392,7 +392,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>  
>  	for (index = 0; index < NUM_STA; index++) {
>  		phead = &pstapriv->sta_hash[index];
> -		list_for_each(plist, phead) {
> +		list_for_each_safe(plist, temp, phead) {
>  			psta = list_entry(plist, struct sta_info, hash_list);
>  
>  			if (pbcmc_stainfo != psta)
> -- 
> 2.20.1
> 

[PATCH 3/6] staging: rtl8188eu: use safe iterator in expire_timeout_chk

[Martin Kaiser]

In the first loop in expire_timeout_chk, we may call rtw_free_stainfo and
remove an entry from auth_list.

In the second loop, we may call list_del_init on our list.

Use list_for_each_safe for both loops.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
compile-tested only

 drivers/staging/rtl8188eu/core/rtw_ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index ca9a321c4921..a410b42ecb6e 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -164,7 +164,7 @@ static u8 chk_sta_is_alive(struct sta_info *psta)
 
 void expire_timeout_chk(struct adapter *padapter)
 {
-	struct list_head *phead, *plist;
+	struct list_head *phead, *plist, *temp;
 	u8 updated = 0;
 	struct sta_info *psta = NULL;
 	struct sta_priv *pstapriv = &padapter->stapriv;
@@ -176,7 +176,7 @@ void expire_timeout_chk(struct adapter *padapter)
 
 	phead = &pstapriv->auth_list;
 	/* check auth_queue */
-	list_for_each(plist, phead) {
+	list_for_each_safe(plist, temp, phead) {
 		psta = list_entry(plist, struct sta_info, auth_list);
 
 		if (psta->expire_to > 0) {
-- 
2.20.1

Re: [PATCH 3/6] staging: rtl8188eu: use safe iterator in expire_timeout_chk

[Guenter Roeck]

On Sun, May 16, 2021 at 06:06:10PM +0200, Martin Kaiser wrote:
> In the first loop in expire_timeout_chk, we may call rtw_free_stainfo and
> remove an entry from auth_list.
> 
> In the second loop, we may call list_del_init on our list.
> 
> Use list_for_each_safe for both loops.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> compile-tested only
> 
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index ca9a321c4921..a410b42ecb6e 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -164,7 +164,7 @@ static u8 chk_sta_is_alive(struct sta_info *psta)
>  
>  void expire_timeout_chk(struct adapter *padapter)
>  {
> -	struct list_head *phead, *plist;
> +	struct list_head *phead, *plist, *temp;
>  	u8 updated = 0;
>  	struct sta_info *psta = NULL;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
> @@ -176,7 +176,7 @@ void expire_timeout_chk(struct adapter *padapter)
>  
>  	phead = &pstapriv->auth_list;
>  	/* check auth_queue */
> -	list_for_each(plist, phead) {
> +	list_for_each_safe(plist, temp, phead) {
>  		psta = list_entry(plist, struct sta_info, auth_list);
>  
>  		if (psta->expire_to > 0) {
> -- 
> 2.20.1
> 

[PATCH 4/6] staging: rtl8188eu: use safe iterator in rtw_acl_remove_sta

[Martin Kaiser]

Use list_for_each_safe, we may delete list items while iterating over
the list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
compile-tested only

 drivers/staging/rtl8188eu/core/rtw_ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index a410b42ecb6e..601974df4114 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1072,7 +1072,7 @@ int rtw_acl_add_sta(struct adapter *padapter, u8 *addr)
 
 int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
 {
-	struct list_head *plist, *phead;
+	struct list_head *plist, *phead, *temp;
 	struct rtw_wlan_acl_node *paclnode;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	struct wlan_acl_pool *pacl_list = &pstapriv->acl_list;
@@ -1083,7 +1083,7 @@ int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
 	spin_lock_bh(&pacl_node_q->lock);
 
 	phead = get_list_head(pacl_node_q);
-	list_for_each(plist, phead) {
+	list_for_each_safe(plist, temp, phead) {
 		paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
 
 		if (!memcmp(paclnode->addr, addr, ETH_ALEN)) {
-- 
2.20.1

Re: [PATCH 4/6] staging: rtl8188eu: use safe iterator in rtw_acl_remove_sta

[Guenter Roeck]

On Sun, May 16, 2021 at 06:06:11PM +0200, Martin Kaiser wrote:
> Use list_for_each_safe, we may delete list items while iterating over
> the list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> compile-tested only
> 
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index a410b42ecb6e..601974df4114 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -1072,7 +1072,7 @@ int rtw_acl_add_sta(struct adapter *padapter, u8 *addr)
>  
>  int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
>  {
> -	struct list_head *plist, *phead;
> +	struct list_head *plist, *phead, *temp;
>  	struct rtw_wlan_acl_node *paclnode;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	struct wlan_acl_pool *pacl_list = &pstapriv->acl_list;
> @@ -1083,7 +1083,7 @@ int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
>  	spin_lock_bh(&pacl_node_q->lock);
>  
>  	phead = get_list_head(pacl_node_q);
> -	list_for_each(plist, phead) {
> +	list_for_each_safe(plist, temp, phead) {
>  		paclnode = list_entry(plist, struct rtw_wlan_acl_node, list);
>  
>  		if (!memcmp(paclnode->addr, addr, ETH_ALEN)) {
> -- 
> 2.20.1
> 

[PATCH 5/6] staging: rtl8188eu: use safe iterator in rtw_sta_flush

[Martin Kaiser]

Use list_for_each_safe, we may delete list items while iterating over
the list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
compile-tested only

 drivers/staging/rtl8188eu/core/rtw_ap.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 601974df4114..8ffafc7eb316 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1579,7 +1579,7 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
 
 int rtw_sta_flush(struct adapter *padapter)
 {
-	struct list_head *phead, *plist;
+	struct list_head *phead, *plist, *temp;
 	struct sta_info *psta = NULL;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
@@ -1594,7 +1594,7 @@ int rtw_sta_flush(struct adapter *padapter)
 	spin_lock_bh(&pstapriv->asoc_list_lock);
 	phead = &pstapriv->asoc_list;
 	/* free sta asoc_queue */
-	list_for_each(plist, phead) {
+	list_for_each_safe(plist, temp, phead) {
 		psta = list_entry(plist, struct sta_info, asoc_list);
 
 		list_del_init(&psta->asoc_list);
-- 
2.20.1

Re: [PATCH 5/6] staging: rtl8188eu: use safe iterator in rtw_sta_flush

[Guenter Roeck]

On Sun, May 16, 2021 at 06:06:12PM +0200, Martin Kaiser wrote:
> Use list_for_each_safe, we may delete list items while iterating over
> the list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> compile-tested only
> 
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index 601974df4114..8ffafc7eb316 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -1579,7 +1579,7 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
>  
>  int rtw_sta_flush(struct adapter *padapter)
>  {
> -	struct list_head *phead, *plist;
> +	struct list_head *phead, *plist, *temp;
>  	struct sta_info *psta = NULL;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
> @@ -1594,7 +1594,7 @@ int rtw_sta_flush(struct adapter *padapter)
>  	spin_lock_bh(&pstapriv->asoc_list_lock);
>  	phead = &pstapriv->asoc_list;
>  	/* free sta asoc_queue */
> -	list_for_each(plist, phead) {
> +	list_for_each_safe(plist, temp, phead) {
>  		psta = list_entry(plist, struct sta_info, asoc_list);
>  
>  		list_del_init(&psta->asoc_list);
> -- 
> 2.20.1
> 

[PATCH 6/6] staging: rtl8188eu: use safe iterator in rtw_free_xmitframe_queue

[Martin Kaiser]

Use list_for_each_safe, we may delete list items while iterating over
the list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
compile-tested only

 drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 3763d188b892..62b8f178280e 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -1329,13 +1329,13 @@ s32 rtw_free_xmitframe(struct xmit_priv *pxmitpriv, struct xmit_frame *pxmitfram
 
 void rtw_free_xmitframe_queue(struct xmit_priv *pxmitpriv, struct __queue *pframequeue)
 {
-	struct list_head *plist, *phead;
+	struct list_head *plist, *phead, *temp;
 	struct	xmit_frame	*pxmitframe;
 
 	spin_lock_bh(&pframequeue->lock);
 
 	phead = get_list_head(pframequeue);
-	list_for_each(plist, phead) {
+	list_for_each_safe(plist, temp, phead) {
 		pxmitframe = list_entry(plist, struct xmit_frame, list);
 
 		rtw_free_xmitframe(pxmitpriv, pxmitframe);
-- 
2.20.1

Re: [PATCH 6/6] staging: rtl8188eu: use safe iterator in rtw_free_xmitframe_queue

[Guenter Roeck]

On Sun, May 16, 2021 at 06:06:13PM +0200, Martin Kaiser wrote:
> Use list_for_each_safe, we may delete list items while iterating over
> the list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> compile-tested only
> 
>  drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index 3763d188b892..62b8f178280e 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -1329,13 +1329,13 @@ s32 rtw_free_xmitframe(struct xmit_priv *pxmitpriv, struct xmit_frame *pxmitfram
>  
>  void rtw_free_xmitframe_queue(struct xmit_priv *pxmitpriv, struct __queue *pframequeue)
>  {
> -	struct list_head *plist, *phead;
> +	struct list_head *plist, *phead, *temp;
>  	struct	xmit_frame	*pxmitframe;
>  
>  	spin_lock_bh(&pframequeue->lock);
>  
>  	phead = get_list_head(pframequeue);
> -	list_for_each(plist, phead) {
> +	list_for_each_safe(plist, temp, phead) {
>  		pxmitframe = list_entry(plist, struct xmit_frame, list);
>  
>  		rtw_free_xmitframe(pxmitpriv, pxmitframe);
> -- 
> 2.20.1
> 

Re: [PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Guenter Roeck]

On Sun, May 16, 2021 at 06:06:08PM +0200, Martin Kaiser wrote:
> rtw_free_network_queue iterates over the scanned wireless networks and
> calls _rtw_free_network for each of them. In some cases,
> _rtw_free_network removes a network from the list.
> 
> We have to use list_for_each_safe if we remove list entries while we
> iterate over a list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> Without this patch, it's easy to get the driver into an endless loop by
> scanning, connecting and disconnecting repeatedly.
> 

Oops. Good catch. Thanks for noticing.

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

> wpa_supplicant -B -Dwext -i wlan0 -c /path/to/my/config...
> while true ; do
>    sleep 1
>    wpa_cli reconfigure     
>    wpa_cli add_network
>    wpa_cli set_network 0 ssid ...
>    wpa_cli set_network 0 psk ...
>    wpa_cli select_network 0
>    sleep 6
>    wpa_cli status
>    wpa_cli disconnect 0
> done
> 
>  drivers/staging/rtl8188eu/core/rtw_mlme.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 159465b073c2..14816ad51668 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -199,7 +199,7 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>  
>  void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
>  {
> -	struct list_head *phead, *plist;
> +	struct list_head *phead, *plist, *temp;
>  	struct wlan_network *pnetwork;
>  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>  	struct __queue *scanned_queue = &pmlmepriv->scanned_queue;
> @@ -207,7 +207,7 @@ void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
>  	spin_lock_bh(&scanned_queue->lock);
>  
>  	phead = get_list_head(scanned_queue);
> -	list_for_each(plist, phead) {
> +	list_for_each_safe(plist, temp, phead) {
>  		pnetwork = list_entry(plist, struct wlan_network, list);
>  
>  		_rtw_free_network(pmlmepriv, pnetwork, isfreeall);
> -- 
> 2.20.1
> 

Re: [PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Christophe JAILLET]

Le 16/05/2021 à 18:06, Martin Kaiser a écrit :
> rtw_free_network_queue iterates over the scanned wireless networks and
> calls _rtw_free_network for each of them. In some cases,
> _rtw_free_network removes a network from the list.
> 
> We have to use list_for_each_safe if we remove list entries while we
> iterate over a list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> Without this patch, it's easy to get the driver into an endless loop by
> scanning, connecting and disconnecting repeatedly.
> 
> wpa_supplicant -B -Dwext -i wlan0 -c /path/to/my/config...
> while true ; do
>     sleep 1
>     wpa_cli reconfigure
>     wpa_cli add_network
>     wpa_cli set_network 0 ssid ...
>     wpa_cli set_network 0 psk ...
>     wpa_cli select_network 0
>     sleep 6
>     wpa_cli status
>     wpa_cli disconnect 0
> done
> 
>   drivers/staging/rtl8188eu/core/rtw_mlme.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 159465b073c2..14816ad51668 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -199,7 +199,7 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>   
>   void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
>   {
> -	struct list_head *phead, *plist;
> +	struct list_head *phead, *plist, *temp;
>   	struct wlan_network *pnetwork;
>   	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>   	struct __queue *scanned_queue = &pmlmepriv->scanned_queue;
> @@ -207,7 +207,7 @@ void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
>   	spin_lock_bh(&scanned_queue->lock);
>   
>   	phead = get_list_head(scanned_queue);
> -	list_for_each(plist, phead) {
> +	list_for_each_safe(plist, temp, phead) {
>   		pnetwork = list_entry(plist, struct wlan_network, list);
>   
>   		_rtw_free_network(pmlmepriv, pnetwork, isfreeall);
> 
Nitpicking: 'list_for_each_entry_safe' could also be used to simplify code.

CJ

Re: [PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Martin Kaiser]

Thus wrote Christophe JAILLET (christophe.jaillet@wanadoo.fr):

> Le 16/05/2021 à 18:06, Martin Kaiser a écrit :
> > rtw_free_network_queue iterates over the scanned wireless networks and
> > calls _rtw_free_network for each of them. In some cases,
> > _rtw_free_network removes a network from the list.

> > We have to use list_for_each_safe if we remove list entries while we
> > iterate over a list.

> > Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> > Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> > ---
> > Without this patch, it's easy to get the driver into an endless loop by
> > scanning, connecting and disconnecting repeatedly.

> > wpa_supplicant -B -Dwext -i wlan0 -c /path/to/my/config...
> > while true ; do
> >     sleep 1
> >     wpa_cli reconfigure
> >     wpa_cli add_network
> >     wpa_cli set_network 0 ssid ...
> >     wpa_cli set_network 0 psk ...
> >     wpa_cli select_network 0
> >     sleep 6
> >     wpa_cli status
> >     wpa_cli disconnect 0
> > done

> >   drivers/staging/rtl8188eu/core/rtw_mlme.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)

> > diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > index 159465b073c2..14816ad51668 100644
> > --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> > @@ -199,7 +199,7 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
> >   void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
> >   {
> > -	struct list_head *phead, *plist;
> > +	struct list_head *phead, *plist, *temp;
> >   	struct wlan_network *pnetwork;
> >   	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
> >   	struct __queue *scanned_queue = &pmlmepriv->scanned_queue;
> > @@ -207,7 +207,7 @@ void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
> >   	spin_lock_bh(&scanned_queue->lock);
> >   	phead = get_list_head(scanned_queue);
> > -	list_for_each(plist, phead) {
> > +	list_for_each_safe(plist, temp, phead) {
> >   		pnetwork = list_entry(plist, struct wlan_network, list);
> >   		_rtw_free_network(pmlmepriv, pnetwork, isfreeall);

> Nitpicking: 'list_for_each_entry_safe' could also be used to simplify code.

Thanks for the hint, I just sent out v2.

Re: [PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Dan Carpenter]

Thanks for catching these...  I've created a new Smatch static checker
warning for this but it only works for list_for_each_entry().
Eventually someone would have run the coccinelle script to convert these
list_for_each loops into list_for_each_entry().  Otherwise you have to
parse container_of() and I've been meaning to do that for a while but I
haven't yet.

Anyway, I'm going to test it out overnight and see what it finds.  It's
sort a new use for the modification_hook(), before I had only ever used
it to silence warnings but this check uses it to trigger warnings.  So
perhaps it will generate a lot of false positives.  We'll see.

It sets the state of the iterator to &start at the start of the loop
and if it's not &start state at the end then it prints a warning.

regards,
dan carpenter

/*
 * Copyright (C) 2021 Oracle.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_extra.h"

static int my_id;

STATE(start);
STATE(watch);

static struct statement *iterator_stmt, *pre_stmt, *post_stmt;

static void set_watch(struct sm_state *sm, struct expression *mod_expr)
{
	set_state(my_id, sm->name, sm->sym, &watch);
}

static bool is_list_macro(const char *macro)
{
	if (strcmp(macro, "list_for_each_entry") == 0)
		return true;
	return false;
}

static void match_iterator_statement(struct statement *stmt)
{
	const char *macro;

	if (stmt->type != STMT_ITERATOR ||
	    !stmt->iterator_pre_statement ||
	    !stmt->iterator_post_statement)
		return;

	macro = get_macro_name(stmt->pos);
	if (!macro)
		return;
	if (!is_list_macro(macro))
		return;

	iterator_stmt = stmt;
	pre_stmt = stmt->iterator_pre_statement;
	post_stmt = stmt->iterator_post_statement;
}

static bool stmt_matches(struct expression *expr, struct statement *stmt)
{
	struct expression *tmp;
	struct statement *parent;

	if (!stmt)
		return false;
	while ((tmp = expr_get_parent_expr(expr)))
		expr = tmp;

	parent = expr_get_parent_stmt(expr);
	return parent == stmt;
}

static char *get_iterator_member(void)
{
	struct expression *expr;

	if (!iterator_stmt ||
	    !iterator_stmt->iterator_pre_condition)
		return NULL;

	expr = iterator_stmt->iterator_pre_condition;
	if (expr->type != EXPR_PREOP || expr->op != '!')
		return NULL;
	expr = strip_parens(expr->unop);
	if (expr->type != EXPR_COMPARE)
		return NULL;
	expr = strip_parens(expr->left);
	if (expr->type != EXPR_PREOP || expr->op != '&')
		return NULL;
	expr = strip_expr(expr->unop);
	if (expr->type != EXPR_DEREF || !expr->member)
		return NULL;
	return expr->member->name;
}

static void match_pre_statement(struct expression *expr)
{
	char *name, *member;
	struct symbol *sym;
	char buf[64];

	if (!stmt_matches(expr, pre_stmt))
		return;

	name = expr_to_var_sym(expr->left, &sym);
	if (!name)
		return;
	member = get_iterator_member();

	snprintf(buf, sizeof(buf), "%s->%s.next", name, member);
	set_state(my_id, buf, sym, &start);
}

static void match_post_statement(struct expression *expr)
{
	struct smatch_state *state;
	char *name, *member;
	struct symbol *sym;
	char buf[64];

	if (!stmt_matches(expr, post_stmt))
		return;

	name = expr_to_var_sym(expr->left, &sym);
	if (!name)
		return;
	member = get_iterator_member();

	snprintf(buf, sizeof(buf), "%s->%s.next", name, member);
	state = get_state(my_id, buf, sym);
	if (!state || state == &start)
		return;

	sm_warning("iterator '%s' changed during iteration", buf);
}

void check_list_set_inside(int id)
{
	my_id = id;

	if (option_project != PROJ_KERNEL)
		return;

	add_hook(match_iterator_statement, STMT_HOOK);
	add_hook(match_pre_statement, ASSIGNMENT_HOOK);
	add_hook(match_post_statement, ASSIGNMENT_HOOK);

	add_modification_hook(my_id, &set_watch);
}

Re: [PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Dan Carpenter]

On Mon, May 17, 2021 at 06:57:33PM +0300, Dan Carpenter wrote:
> Thanks for catching these...  I've created a new Smatch static checker
> warning for this but it only works for list_for_each_entry().
> Eventually someone would have run the coccinelle script to convert these
> list_for_each loops into list_for_each_entry().  Otherwise you have to
> parse container_of() and I've been meaning to do that for a while but I
> haven't yet.
> 
> Anyway, I'm going to test it out overnight and see what it finds.  It's
> sort a new use for the modification_hook(), before I had only ever used
> it to silence warnings but this check uses it to trigger warnings.  So
> perhaps it will generate a lot of false positives.  We'll see.
> 
> It sets the state of the iterator to &start at the start of the loop
> and if it's not &start state at the end then it prints a warning.
> 
> regards,
> dan carpenter
> 

That Smatch check didn't work at all.  :P  Back to the drawing board.

regards,
dan carpenter

Re: [PATCH 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Dan Carpenter]

On Tue, May 18, 2021 at 11:28:55AM +0300, Dan Carpenter wrote:
> On Mon, May 17, 2021 at 06:57:33PM +0300, Dan Carpenter wrote:
> > Thanks for catching these...  I've created a new Smatch static checker
> > warning for this but it only works for list_for_each_entry().
> > Eventually someone would have run the coccinelle script to convert these
> > list_for_each loops into list_for_each_entry().  Otherwise you have to
> > parse container_of() and I've been meaning to do that for a while but I
> > haven't yet.
> > 
> > Anyway, I'm going to test it out overnight and see what it finds.  It's
> > sort a new use for the modification_hook(), before I had only ever used
> > it to silence warnings but this check uses it to trigger warnings.  So
> > perhaps it will generate a lot of false positives.  We'll see.
> > 
> > It sets the state of the iterator to &start at the start of the loop
> > and if it's not &start state at the end then it prints a warning.
> > 
> > regards,
> > dan carpenter
> > 
> 
> That Smatch check didn't work at all.  :P  Back to the drawing board.
> 
> regards,
> dan carpenter

It's sort of working now.  I just had to get some ordering issues fixed.
Also I had to fix a bug in smatch_conditions.c...

The check itself works very well, but the heuristic is a bit off.  It
turns out we some times mess with the list iterator very intentionally.
For example, one thing people do is add a new item directly after the
current item.

About 8 out of the 32 warnings listed below are real issues.  I'm going
to send some patches and bug fixes for them.  A couple are too ancient
for and complicated to bother with.

regards,
dan carpenter

kernel/exit.c:1397 do_wait_thread() warn: iterator 'p->sibling.next' changed during iteration
kernel/exit.c:1411 ptrace_do_wait() warn: iterator 'p->ptrace_entry.next' changed during iteration
kernel/events/core.c:3220 perf_event_modify_attr() warn: iterator 'child->child_list.next' changed during iteration
kernel/events/core.c:5451 perf_event_for_each_child() warn: iterator 'child->child_list.next' changed during iteration
kernel/locking/locktorture.c:420 torture_ww_mutex_lock() warn: iterator 'll->link.next' changed during iteration
kernel/locking/test-ww_mutex.c:464 stress_reorder_work() warn: iterator 'll->link.next' changed during iteration
arch/x86/kvm/../../../virt/kvm/kvm_main.c:4866 vm_stat_clear() warn: iterator 'kvm->vm_list.next' changed during iteration
fs/btrfs/extent-tree.c:4230 find_free_extent() warn: iterator 'block_group->list.next' changed during iteration
fs/nilfs2/segment.c:1565 nilfs_segctor_update_payload_blocknr() warn: iterator 'bh->b_assoc_buffers.next' changed during iteration
fs/super.c:657 __iterate_supers() warn: iterator 'sb->s_list.next' changed during iteration
drivers/spi/spi-fsi.c:469 fsi_spi_transfer_one_message() warn: iterator 'transfer->transfer_list.next' changed during iteration
drivers/spi/spi-tegra210-quad.c:990 tegra_qspi_transfer_one_message() warn: iterator 'transfer->transfer_list.next' changed during iteration
drivers/spi/spi.c:3299 spi_split_transfers_maxsize() warn: iterator 'xfer->transfer_list.next' changed during iteration
drivers/staging/emxx_udc/emxx_udc.c:2079 _nbu2ss_nuke() warn: iterator 'req->queue.next' changed during iteration
drivers/pnp/quirks.c:59 quirk_awe32_resources() warn: iterator 'option->list.next' changed during iteration
drivers/usb/dwc3/gadget.c:1298 dwc3_prepare_trbs() warn: iterator 'req->list.next' changed during iteration
drivers/gpu/drm/ttm/ttm_device.c:145 ttm_device_swapout() warn: iterator 'bo->lru.next' changed during iteration
drivers/gpu/drm/ttm/ttm_execbuf_util.c:91 ttm_eu_reserve_buffers() warn: iterator 'entry->head.next' changed during iteration
drivers/gpu/drm/i915/gt/intel_reset.c:901 __intel_gt_unset_wedged() warn: iterator 'tl->link.next' changed during iteration
drivers/target/target_core_user.c:1264 tcmu_tmr_notify() warn: iterator 'se_cmd->state_list.next' changed during iteration
drivers/w1/w1.c:1265 w1_fini() warn: iterator 'dev->w1_master_entry.next' changed during iteration
drivers/scsi/libsas/sas_port.c:47 sas_resume_port() warn: iterator 'dev->dev_list_node.next' changed during iteration
drivers/scsi/lpfc/lpfc_hbadisc.c:993 lpfc_linkup_cleanup_nodes() warn: iterator 'ndlp->nlp_listp.next' changed during iteration
drivers/scsi/lpfc/lpfc_hbadisc.c:4983 lpfc_unreg_hba_rpis() warn: iterator 'ndlp->nlp_listp.next' changed during iteration
drivers/block/drbd/drbd_debugfs.c:311 seq_print_resource_transfer_log_summary() warn: iterator 'req->tl_requests.next' changed during iteration
drivers/infiniband/hw/mlx5/main.c:3285 mlx5_ib_init_multiport_master() warn: iterator 'mpi->list.next' changed during iteration
drivers/infiniband/core/verbs.c:1112 __ib_shared_qp_event_handler() warn: iterator 'event->element.qp->open_list.next' changed during iteration
net/core/devlink.c:10589 devlink_pernet_pre_exit() warn: iterator 'devlink->list.next' changed during iteration
net/bluetooth/l2cap_core.c:1726 l2cap_conn_ready() warn: iterator 'chan->list.next' changed during iteration
net/bluetooth/l2cap_core.c:6265 l2cap_ecred_reconf_rsp() warn: iterator 'chan->list.next' changed during iteration
net/bluetooth/l2cap_core.c:8209 l2cap_security_cfm() warn: iterator 'chan->list.next' changed during iteration
security/landlock/fs.c:347 hook_sb_delete() warn: iterator 'inode->i_sb_list.next' changed during iteration

/*
 * Copyright (C) 2021 Oracle.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, see http://www.gnu.org/copyleft/gpl.txt
 */

#include "smatch.h"
#include "smatch_extra.h"

static int my_id;

STATE(start);
STATE(watch);

static struct statement *iterator_stmt, *pre_stmt, *post_stmt;

static bool stmt_matches(struct expression *expr, struct statement *stmt)
{
	struct expression *tmp;
	struct statement *parent;

	if (!stmt)
		return false;
	while ((tmp = expr_get_parent_expr(expr)))
		expr = tmp;

	parent = expr_get_parent_stmt(expr);
	return parent == stmt;
}

static void set_watch(struct sm_state *sm, struct expression *mod_expr)
{
	if (mod_expr && stmt_matches(mod_expr, post_stmt))
		return;
	set_state(my_id, sm->name, sm->sym, &watch);
}

static bool is_list_macro(const char *macro)
{
	if (strcmp(macro, "list_for_each_entry") == 0)
		return true;
	return false;
}

static void match_iterator_statement(struct statement *stmt)
{
	const char *macro;

	if (stmt->type != STMT_ITERATOR ||
	    !stmt->iterator_pre_statement ||
	    !stmt->iterator_post_statement)
		return;

	macro = get_macro_name(stmt->pos);
	if (!macro)
		return;
	if (!is_list_macro(macro))
		return;

	iterator_stmt = stmt;
	pre_stmt = stmt->iterator_pre_statement;
	post_stmt = stmt->iterator_post_statement;
}

static char *get_iterator_member(void)
{
	struct expression *expr;

	if (!iterator_stmt ||
	    !iterator_stmt->iterator_pre_condition)
		return NULL;

	expr = iterator_stmt->iterator_pre_condition;
	if (expr->type != EXPR_PREOP || expr->op != '!')
		return NULL;
	expr = strip_parens(expr->unop);
	if (expr->type != EXPR_COMPARE)
		return NULL;
	expr = strip_parens(expr->left);
	if (expr->type != EXPR_PREOP || expr->op != '&')
		return NULL;
	expr = strip_expr(expr->unop);
	if (expr->type != EXPR_DEREF || !expr->member)
		return NULL;
	return expr->member->name;
}

static void match_pre_statement(struct expression *expr)
{
	char *name, *member;
	struct symbol *sym;
	char buf[64];

	if (!stmt_matches(expr, pre_stmt))
		return;

	name = expr_to_var_sym(expr->left, &sym);
	if (!name)
		return;
	member = get_iterator_member();

	snprintf(buf, sizeof(buf), "%s->%s.next", name, member);
	set_state(my_id, buf, sym, &start);
}

static void match_post_statement(struct expression *expr)
{
	struct smatch_state *state;
	char *name, *member;
	struct symbol *sym;
	char buf[64];

	if (!stmt_matches(expr, post_stmt))
		return;

	name = expr_to_var_sym(expr->left, &sym);
	if (!name)
		return;
	member = get_iterator_member();

	snprintf(buf, sizeof(buf), "%s->%s.next", name, member);
	state = get_state(my_id, buf, sym);
	if (!state || state == &start)
		return;

	sm_warning("iterator '%s' changed during iteration", buf);
}

void check_list_set_inside(int id)
{
	my_id = id;

	if (option_project != PROJ_KERNEL)
		return;

	add_hook(match_iterator_statement, STMT_HOOK);
	add_hook(match_pre_statement, ASSIGNMENT_HOOK_AFTER);
	add_hook(match_post_statement, ASSIGNMENT_HOOK);

	add_modification_hook(my_id, &set_watch);
}

[PATCH] staging: emxx_udc: fix loop in _nbu2ss_nuke()

[Dan Carpenter]

The _nbu2ss_ep_done() function calls:

	list_del_init(&req->queue);

which means that the loop will never exit.

Fixes: ca3d253eb967 ("Staging: emxx_udc: Iterate list using list_for_each_entry")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/staging/emxx_udc/emxx_udc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/emxx_udc/emxx_udc.c b/drivers/staging/emxx_udc/emxx_udc.c
index 741147a4f0fe..ecc5c9da9027 100644
--- a/drivers/staging/emxx_udc/emxx_udc.c
+++ b/drivers/staging/emxx_udc/emxx_udc.c
@@ -2064,7 +2064,7 @@ static int _nbu2ss_nuke(struct nbu2ss_udc *udc,
 			struct nbu2ss_ep *ep,
 			int status)
 {
-	struct nbu2ss_req *req;
+	struct nbu2ss_req *req, *n;
 
 	/* Endpoint Disable */
 	_nbu2ss_epn_exit(udc, ep);
@@ -2076,7 +2076,7 @@ static int _nbu2ss_nuke(struct nbu2ss_udc *udc,
 		return 0;
 
 	/* called with irqs blocked */
-	list_for_each_entry(req, &ep->queue, queue) {
+	list_for_each_entry_safe(req, n, &ep->queue, queue) {
 		_nbu2ss_ep_done(ep, req, status);
 	}
 
-- 
2.30.2

[PATCH] w1: fix loop in w1_fini()

[Dan Carpenter]

The __w1_remove_master_device() function calls:

	list_del(&dev->w1_master_entry);

So presumably this can cause an endless loop.

Fixes: 7785925dd8e0 ("[PATCH] w1: cleanups.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/w1/w1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index f2ae2e563dc5..8b2d82959ded 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -1259,10 +1259,10 @@ static int __init w1_init(void)
 
 static void __exit w1_fini(void)
 {
-	struct w1_master *dev;
+	struct w1_master *dev, *n;
 
 	/* Set netlink removal messages and some cleanup */
-	list_for_each_entry(dev, &w1_masters, w1_master_entry)
+	list_for_each_entry_safe(dev, n, &w1_masters, w1_master_entry)
 		__w1_remove_master_device(dev);
 
 	w1_fini_netlink();
-- 
2.30.2

Re: (subset) [PATCH] w1: fix loop in w1_fini()

[Krzysztof Kozlowski]

On Wed, 19 May 2021 17:17:45 +0300, Dan Carpenter wrote:
> The __w1_remove_master_device() function calls:
> 
> 	list_del(&dev->w1_master_entry);
> 
> So presumably this can cause an endless loop.
> 
> 
> [...]

Applied, thanks!

[1/1] w1: fix loop in w1_fini()
      https://git.kernel.org/krzk/linux-w1/c/83f3fcf96fcc7e5405b37d9424c7ef26bfa203f8

Best regards,
-- 
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

[bug report] {net, IB}/mlx5: Manage port association for multiport RoCE

[Dan Carpenter]

Hello Daniel Jurgens,

The patch 32f69e4be269: "{net, IB}/mlx5: Manage port association for
multiport RoCE" from Jan 4, 2018, leads to the following static
checker warning:

	drivers/infiniband/hw/mlx5/main.c:3285 mlx5_ib_init_multiport_master()
	warn: iterator 'mpi->list.next' changed during iteration

drivers/infiniband/hw/mlx5/main.c
  3285                  list_for_each_entry(mpi, &mlx5_ib_unaffiliated_port_list,
                                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We're iterating through the unafiliated list.

  3286                                      list) {
  3287                          if (dev->sys_image_guid == mpi->sys_image_guid &&
  3288                              (mlx5_core_native_port_num(mpi->mdev) - 1) == i) {
  3289                                  bound = mlx5_ib_bind_slave_port(dev, mpi);

The mlx5_ib_bind_slave_port() function returns true on success and
false on failure.  On the failure path it calls:

	mlx5_ib_unbind_slave_port(ibdev, mpi);

Which adds our "mpi" as the last item on the unaffiliated list.  I don't
think anything good can come from adding a list item to a list twice.

  3290                          }
  3291  
  3292                          if (bound) {
  3293                                  dev_dbg(mpi->mdev->device,
  3294                                          "removing port from unaffiliated list.\n");
  3295                                  mlx5_ib_dbg(dev, "port %d bound\n", i + 1);
  3296                                  list_del(&mpi->list);
  3297                                  break;
  3298                          }
  3299                  }
  3300                  if (!bound)
  3301                          mlx5_ib_dbg(dev, "no free port found for port %d\n",
  3302                                      i + 1);

regards,
dan carpenter

Re: [bug report] {net, IB}/mlx5: Manage port association for multiport RoCE

[Leon Romanovsky]

On Wed, May 19, 2021 at 05:18:33PM +0300, Dan Carpenter wrote:
> Hello Daniel Jurgens,
> 
> The patch 32f69e4be269: "{net, IB}/mlx5: Manage port association for
> multiport RoCE" from Jan 4, 2018, leads to the following static
> checker warning:
> 
> 	drivers/infiniband/hw/mlx5/main.c:3285 mlx5_ib_init_multiport_master()
> 	warn: iterator 'mpi->list.next' changed during iteration

Thanks Dan for the report, I'll prepare the fix.

> 
> drivers/infiniband/hw/mlx5/main.c
>   3285                  list_for_each_entry(mpi, &mlx5_ib_unaffiliated_port_list,
>                                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> We're iterating through the unafiliated list.
> 
>   3286                                      list) {
>   3287                          if (dev->sys_image_guid == mpi->sys_image_guid &&
>   3288                              (mlx5_core_native_port_num(mpi->mdev) - 1) == i) {
>   3289                                  bound = mlx5_ib_bind_slave_port(dev, mpi);
> 
> The mlx5_ib_bind_slave_port() function returns true on success and
> false on failure.  On the failure path it calls:
> 
> 	mlx5_ib_unbind_slave_port(ibdev, mpi);
> 
> Which adds our "mpi" as the last item on the unaffiliated list.  I don't
> think anything good can come from adding a list item to a list twice.
> 
>   3290                          }
>   3291  
>   3292                          if (bound) {
>   3293                                  dev_dbg(mpi->mdev->device,
>   3294                                          "removing port from unaffiliated list.\n");
>   3295                                  mlx5_ib_dbg(dev, "port %d bound\n", i + 1);
>   3296                                  list_del(&mpi->list);
>   3297                                  break;
>   3298                          }
>   3299                  }
>   3300                  if (!bound)
>   3301                          mlx5_ib_dbg(dev, "no free port found for port %d\n",
>   3302                                      i + 1);
> 
> regards,
> dan carpenter

[bug report] Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode

[Dan Carpenter]

Hello Luiz Augusto von Dentz,

The patch 15f02b910562: "Bluetooth: L2CAP: Add initial code for
Enhanced Credit Based Mode" from Mar 2, 2020, leads to the following
static checker warning:

	net/bluetooth/l2cap_core.c:6265 l2cap_ecred_reconf_rsp()
	warn: iterator 'chan->list.next' changed during iteration

net/bluetooth/l2cap_core.c
  6247  static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
  6248                                           struct l2cap_cmd_hdr *cmd, u16 cmd_len,
  6249                                           u8 *data)
  6250  {
  6251          struct l2cap_chan *chan;
  6252          struct l2cap_ecred_conn_rsp *rsp = (void *) data;
  6253          u16 result;
  6254  
  6255          if (cmd_len < sizeof(*rsp))
  6256                  return -EPROTO;
  6257  
  6258          result = __le16_to_cpu(rsp->result);
  6259  
  6260          BT_DBG("result 0x%4.4x", rsp->result);
  6261  
  6262          if (!result)
  6263                  return 0;
  6264  
  6265          list_for_each_entry(chan, &conn->chan_l, list) {
  6266                  if (chan->ident != cmd->ident)
  6267                          continue;
  6268  
  6269                  l2cap_chan_del(chan, ECONNRESET);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
This can call:

		list_del(&chan->list);

which will lead to an oops in the next iteration.

  6270          }
  6271  
  6272          return 0;
  6273  }

regards,
dan carpenter

[PATCH] scsi: libsas: use _safe() loop in sas_resume_port()

[Dan Carpenter]

If sas_notify_lldd_dev_found() fails then this code calls:

	sas_unregister_dev(port, dev);

which removes "dev", our list iterator, from the list.  This could
lead to an endless loop.  We need to use list_for_each_entry_safe().

Fixes: 303694eeee5e ("[SCSI] libsas: suspend / resume support")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/scsi/libsas/sas_port.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libsas/sas_port.c b/drivers/scsi/libsas/sas_port.c
index 19cf418928fa..e3d03d744713 100644
--- a/drivers/scsi/libsas/sas_port.c
+++ b/drivers/scsi/libsas/sas_port.c
@@ -25,7 +25,7 @@ static bool phy_is_wideport_member(struct asd_sas_port *port, struct asd_sas_phy
 
 static void sas_resume_port(struct asd_sas_phy *phy)
 {
-	struct domain_device *dev;
+	struct domain_device *dev, *n;
 	struct asd_sas_port *port = phy->port;
 	struct sas_ha_struct *sas_ha = phy->ha;
 	struct sas_internal *si = to_sas_internal(sas_ha->core.shost->transportt);
@@ -44,7 +44,7 @@ static void sas_resume_port(struct asd_sas_phy *phy)
 	 * 1/ presume every device came back
 	 * 2/ force the next revalidation to check all expander phys
 	 */
-	list_for_each_entry(dev, &port->dev_list, dev_list_node) {
+	list_for_each_entry_safe(dev, n, &port->dev_list, dev_list_node) {
 		int i, rc;
 
 		rc = sas_notify_lldd_dev_found(dev);
-- 
2.30.2

Re: [PATCH] scsi: libsas: use _safe() loop in sas_resume_port()

[John Garry]

On 19/05/2021 15:20, Dan Carpenter wrote:
> If sas_notify_lldd_dev_found() fails then this code calls:
> 
> 	sas_unregister_dev(port, dev);
> 
> which removes "dev", our list iterator, from the list.  This could
> lead to an endless loop.  We need to use list_for_each_entry_safe().
> 
> Fixes: 303694eeee5e ("[SCSI] libsas: suspend / resume support")
> Signed-off-by: Dan Carpenter<dan.carpenter@oracle.com>

Reviewed-by: John Garry <john.garry@huawei.com>

Re: [PATCH] scsi: libsas: use _safe() loop in sas_resume_port()

[Martin K. Petersen]

On Wed, 19 May 2021 17:20:27 +0300, Dan Carpenter wrote:

> If sas_notify_lldd_dev_found() fails then this code calls:
> 
> 	sas_unregister_dev(port, dev);
> 
> which removes "dev", our list iterator, from the list.  This could
> lead to an endless loop.  We need to use list_for_each_entry_safe().

Applied to 5.13/scsi-fixes, thanks!

[1/1] scsi: libsas: use _safe() loop in sas_resume_port()
      https://git.kernel.org/mkp/scsi/c/8c7e7b8486cd

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH v2 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Martin Kaiser]

rtw_free_network_queue iterates over the scanned wireless networks and
calls _rtw_free_network for each of them. In some cases,
_rtw_free_network removes a network from the list.

We have to use list_for_each_entry_safe if we remove list entries while
we iterate over a list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - use list_for_each_entry_safe

Without this patch, it's easy to get the driver into an endless loop by
scanning, connecting and disconnecting repeatedly.

wpa_supplicant -B -Dwext -i wlan0 -c /path/to/my/config...
while true ; do
   sleep 1
   wpa_cli reconfigure
   wpa_cli add_network
   wpa_cli set_network 0 ssid ...
   wpa_cli set_network 0 psk ...
   wpa_cli select_network 0
   sleep 6
   wpa_cli status
   wpa_cli disconnect 0
done

 drivers/staging/rtl8188eu/core/rtw_mlme.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
index 159465b073c2..ba73ac5325e2 100644
--- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
+++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
@@ -199,19 +199,17 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
 
 void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
 {
-	struct list_head *phead, *plist;
-	struct wlan_network *pnetwork;
+	struct list_head *phead;
+	struct wlan_network *pnetwork, *temp;
 	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 	struct __queue *scanned_queue = &pmlmepriv->scanned_queue;
 
 	spin_lock_bh(&scanned_queue->lock);
 
 	phead = get_list_head(scanned_queue);
-	list_for_each(plist, phead) {
-		pnetwork = list_entry(plist, struct wlan_network, list);
-
+	list_for_each_entry_safe(pnetwork, temp, phead, list)
 		_rtw_free_network(pmlmepriv, pnetwork, isfreeall);
-	}
+
 	spin_unlock_bh(&scanned_queue->lock);
 }
 
-- 
2.20.1

[PATCH v2 2/6] staging: rtl8188eu: use safe iterator in rtw_free_all_stainfo

[Martin Kaiser]

This is another case where we may remove list entries while we iterate over
the list. Use list_for_each_entry_safe to avoid an endless loop.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - use list_for_each_entry_safe

Without this patch, unloading the module goes into an endless loop
sometimes.

 drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
index 7941ca0397ed..5af7af5f5a5a 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
@@ -379,9 +379,9 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
 /*  free all stainfo which in sta_hash[all] */
 void rtw_free_all_stainfo(struct adapter *padapter)
 {
-	struct list_head *plist, *phead;
+	struct list_head *phead;
 	s32 index;
-	struct sta_info *psta = NULL;
+	struct sta_info *psta, *temp;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	struct sta_info *pbcmc_stainfo = rtw_get_bcmc_stainfo(padapter);
 
@@ -392,9 +392,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
 
 	for (index = 0; index < NUM_STA; index++) {
 		phead = &pstapriv->sta_hash[index];
-		list_for_each(plist, phead) {
-			psta = list_entry(plist, struct sta_info, hash_list);
-
+		list_for_each_entry_safe(psta, temp, phead, hash_list) {
 			if (pbcmc_stainfo != psta)
 				rtw_free_stainfo(padapter, psta);
 		}
-- 
2.20.1

Re: [PATCH v2 2/6] staging: rtl8188eu: use safe iterator in rtw_free_all_stainfo

[Guenter Roeck]

On Mon, May 17, 2021 at 10:18:22PM +0200, Martin Kaiser wrote:
> This is another case where we may remove list entries while we iterate over
> the list. Use list_for_each_entry_safe to avoid an endless loop.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> v2:
>  - use list_for_each_entry_safe
> 
> Without this patch, unloading the module goes into an endless loop
> sometimes.
> 
>  drivers/staging/rtl8188eu/core/rtw_sta_mgt.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> index 7941ca0397ed..5af7af5f5a5a 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_sta_mgt.c
> @@ -379,9 +379,9 @@ u32 rtw_free_stainfo(struct adapter *padapter, struct sta_info *psta)
>  /*  free all stainfo which in sta_hash[all] */
>  void rtw_free_all_stainfo(struct adapter *padapter)
>  {
> -	struct list_head *plist, *phead;
> +	struct list_head *phead;
>  	s32 index;
> -	struct sta_info *psta = NULL;
> +	struct sta_info *psta, *temp;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	struct sta_info *pbcmc_stainfo = rtw_get_bcmc_stainfo(padapter);
>  
> @@ -392,9 +392,7 @@ void rtw_free_all_stainfo(struct adapter *padapter)
>  
>  	for (index = 0; index < NUM_STA; index++) {
>  		phead = &pstapriv->sta_hash[index];
> -		list_for_each(plist, phead) {
> -			psta = list_entry(plist, struct sta_info, hash_list);
> -
> +		list_for_each_entry_safe(psta, temp, phead, hash_list) {
>  			if (pbcmc_stainfo != psta)
>  				rtw_free_stainfo(padapter, psta);
>  		}
> -- 
> 2.20.1
> 

[PATCH v2 3/6] staging: rtl8188eu: use safe iterator in expire_timeout_chk

[Martin Kaiser]

In the first loop in expire_timeout_chk, we may call rtw_free_stainfo and
remove an entry from auth_list.

In the second loop, we may call list_del_init on our list.

Use list_for_each_entry_safe for both loops.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - use list_for_each_entry_safe
 - update both loops (I forgot the 2nd loop in v1)

 drivers/staging/rtl8188eu/core/rtw_ap.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index ca9a321c4921..6d7c96f1aa42 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -164,9 +164,9 @@ static u8 chk_sta_is_alive(struct sta_info *psta)
 
 void expire_timeout_chk(struct adapter *padapter)
 {
-	struct list_head *phead, *plist;
+	struct list_head *phead;
 	u8 updated = 0;
-	struct sta_info *psta = NULL;
+	struct sta_info *psta, *temp;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	u8 chk_alive_num = 0;
 	char chk_alive_list[NUM_STA];
@@ -176,9 +176,7 @@ void expire_timeout_chk(struct adapter *padapter)
 
 	phead = &pstapriv->auth_list;
 	/* check auth_queue */
-	list_for_each(plist, phead) {
-		psta = list_entry(plist, struct sta_info, auth_list);
-
+	list_for_each_entry_safe(psta, temp, phead, auth_list) {
 		if (psta->expire_to > 0) {
 			psta->expire_to--;
 			if (psta->expire_to == 0) {
@@ -206,9 +204,7 @@ void expire_timeout_chk(struct adapter *padapter)
 
 	phead = &pstapriv->asoc_list;
 	/* check asoc_queue */
-	list_for_each(plist, phead) {
-		psta = list_entry(plist, struct sta_info, asoc_list);
-
+	list_for_each_entry_safe(psta, temp, phead, asoc_list) {
 		if (chk_sta_is_alive(psta) || !psta->expire_to) {
 			psta->expire_to = pstapriv->expire_to;
 			psta->keep_alive_trycnt = 0;
-- 
2.20.1

Re: [PATCH v2 3/6] staging: rtl8188eu: use safe iterator in expire_timeout_chk

[Guenter Roeck]

On Mon, May 17, 2021 at 10:18:23PM +0200, Martin Kaiser wrote:
> In the first loop in expire_timeout_chk, we may call rtw_free_stainfo and
> remove an entry from auth_list.
> 
> In the second loop, we may call list_del_init on our list.
> 
> Use list_for_each_entry_safe for both loops.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> v2:
>  - use list_for_each_entry_safe
>  - update both loops (I forgot the 2nd loop in v1)
> 
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index ca9a321c4921..6d7c96f1aa42 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -164,9 +164,9 @@ static u8 chk_sta_is_alive(struct sta_info *psta)
>  
>  void expire_timeout_chk(struct adapter *padapter)
>  {
> -	struct list_head *phead, *plist;
> +	struct list_head *phead;
>  	u8 updated = 0;
> -	struct sta_info *psta = NULL;
> +	struct sta_info *psta, *temp;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	u8 chk_alive_num = 0;
>  	char chk_alive_list[NUM_STA];
> @@ -176,9 +176,7 @@ void expire_timeout_chk(struct adapter *padapter)
>  
>  	phead = &pstapriv->auth_list;
>  	/* check auth_queue */
> -	list_for_each(plist, phead) {
> -		psta = list_entry(plist, struct sta_info, auth_list);
> -
> +	list_for_each_entry_safe(psta, temp, phead, auth_list) {
>  		if (psta->expire_to > 0) {
>  			psta->expire_to--;
>  			if (psta->expire_to == 0) {
> @@ -206,9 +204,7 @@ void expire_timeout_chk(struct adapter *padapter)
>  
>  	phead = &pstapriv->asoc_list;
>  	/* check asoc_queue */
> -	list_for_each(plist, phead) {
> -		psta = list_entry(plist, struct sta_info, asoc_list);
> -
> +	list_for_each_entry_safe(psta, temp, phead, asoc_list) {
>  		if (chk_sta_is_alive(psta) || !psta->expire_to) {
>  			psta->expire_to = pstapriv->expire_to;
>  			psta->keep_alive_trycnt = 0;
> -- 
> 2.20.1
> 

[PATCH v2 4/6] staging: rtl8188eu: use safe iterator in rtw_acl_remove_sta

[Martin Kaiser]

Use list_for_each_entry_safe, we may delete list items while iterating
over the list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - use list_for_each_entry_safe

 drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index 6d7c96f1aa42..d297d5301153 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1068,8 +1068,8 @@ int rtw_acl_add_sta(struct adapter *padapter, u8 *addr)
 
 int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
 {
-	struct list_head *plist, *phead;
-	struct rtw_wlan_acl_node *paclnode;
+	struct list_head *phead;
+	struct rtw_wlan_acl_node *paclnode, *temp;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	struct wlan_acl_pool *pacl_list = &pstapriv->acl_list;
 	struct __queue *pacl_node_q = &pacl_list->acl_node_q;
@@ -1079,9 +1079,7 @@ int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
 	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, temp, phead, list) {
 		if (!memcmp(paclnode->addr, addr, ETH_ALEN)) {
 			if (paclnode->valid) {
 				paclnode->valid = false;
-- 
2.20.1

Re: [PATCH v2 4/6] staging: rtl8188eu: use safe iterator in rtw_acl_remove_sta

[Guenter Roeck]

On Mon, May 17, 2021 at 10:18:24PM +0200, Martin Kaiser wrote:
> Use list_for_each_entry_safe, we may delete list items while iterating
> over the list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> v2:
>  - use list_for_each_entry_safe
> 
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index 6d7c96f1aa42..d297d5301153 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -1068,8 +1068,8 @@ int rtw_acl_add_sta(struct adapter *padapter, u8 *addr)
>  
>  int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
>  {
> -	struct list_head *plist, *phead;
> -	struct rtw_wlan_acl_node *paclnode;
> +	struct list_head *phead;
> +	struct rtw_wlan_acl_node *paclnode, *temp;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	struct wlan_acl_pool *pacl_list = &pstapriv->acl_list;
>  	struct __queue *pacl_node_q = &pacl_list->acl_node_q;
> @@ -1079,9 +1079,7 @@ int rtw_acl_remove_sta(struct adapter *padapter, u8 *addr)
>  	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, temp, phead, list) {
>  		if (!memcmp(paclnode->addr, addr, ETH_ALEN)) {
>  			if (paclnode->valid) {
>  				paclnode->valid = false;
> -- 
> 2.20.1
> 

[PATCH v2 5/6] staging: rtl8188eu: use safe iterator in rtw_sta_flush

[Martin Kaiser]

Use list_for_each_entry_safe, we may delete list items while iterating
over the list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - use list_for_each_entry_safe

 drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
index d297d5301153..bbecb07274f6 100644
--- a/drivers/staging/rtl8188eu/core/rtw_ap.c
+++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
@@ -1573,8 +1573,8 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
 
 int rtw_sta_flush(struct adapter *padapter)
 {
-	struct list_head *phead, *plist;
-	struct sta_info *psta = NULL;
+	struct list_head *phead;
+	struct sta_info *psta, *temp;
 	struct sta_priv *pstapriv = &padapter->stapriv;
 	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
 	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
@@ -1588,9 +1588,7 @@ int rtw_sta_flush(struct adapter *padapter)
 	spin_lock_bh(&pstapriv->asoc_list_lock);
 	phead = &pstapriv->asoc_list;
 	/* free sta asoc_queue */
-	list_for_each(plist, phead) {
-		psta = list_entry(plist, struct sta_info, asoc_list);
-
+	list_for_each_entry_safe(psta, temp, phead, asoc_list) {
 		list_del_init(&psta->asoc_list);
 		pstapriv->asoc_list_cnt--;
 
-- 
2.20.1

Re: [PATCH v2 5/6] staging: rtl8188eu: use safe iterator in rtw_sta_flush

[Guenter Roeck]

On Mon, May 17, 2021 at 10:18:25PM +0200, Martin Kaiser wrote:
> Use list_for_each_entry_safe, we may delete list items while iterating
> over the list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> v2:
>  - use list_for_each_entry_safe
> 
>  drivers/staging/rtl8188eu/core/rtw_ap.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_ap.c b/drivers/staging/rtl8188eu/core/rtw_ap.c
> index d297d5301153..bbecb07274f6 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_ap.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_ap.c
> @@ -1573,8 +1573,8 @@ u8 ap_free_sta(struct adapter *padapter, struct sta_info *psta,
>  
>  int rtw_sta_flush(struct adapter *padapter)
>  {
> -	struct list_head *phead, *plist;
> -	struct sta_info *psta = NULL;
> +	struct list_head *phead;
> +	struct sta_info *psta, *temp;
>  	struct sta_priv *pstapriv = &padapter->stapriv;
>  	struct mlme_ext_priv *pmlmeext = &padapter->mlmeextpriv;
>  	struct mlme_ext_info	*pmlmeinfo = &pmlmeext->mlmext_info;
> @@ -1588,9 +1588,7 @@ int rtw_sta_flush(struct adapter *padapter)
>  	spin_lock_bh(&pstapriv->asoc_list_lock);
>  	phead = &pstapriv->asoc_list;
>  	/* free sta asoc_queue */
> -	list_for_each(plist, phead) {
> -		psta = list_entry(plist, struct sta_info, asoc_list);
> -
> +	list_for_each_entry_safe(psta, temp, phead, asoc_list) {
>  		list_del_init(&psta->asoc_list);
>  		pstapriv->asoc_list_cnt--;
>  
> -- 
> 2.20.1
> 

[PATCH v2 6/6] staging: rtl8188eu: use safe iterator in rtw_free_xmitframe_queue

[Martin Kaiser]

Use list_for_each_entry_safe, we may delete list items while iterating
over the list.

Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
---
v2:
 - use list_for_each_entry_safe

 drivers/staging/rtl8188eu/core/rtw_xmit.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 3763d188b892..dcc29a74612d 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -1329,17 +1329,15 @@ s32 rtw_free_xmitframe(struct xmit_priv *pxmitpriv, struct xmit_frame *pxmitfram
 
 void rtw_free_xmitframe_queue(struct xmit_priv *pxmitpriv, struct __queue *pframequeue)
 {
-	struct list_head *plist, *phead;
-	struct	xmit_frame	*pxmitframe;
+	struct list_head *phead;
+	struct	xmit_frame	*pxmitframe, *temp;
 
 	spin_lock_bh(&pframequeue->lock);
 
 	phead = get_list_head(pframequeue);
-	list_for_each(plist, phead) {
-		pxmitframe = list_entry(plist, struct xmit_frame, list);
-
+	list_for_each_entry_safe(pxmitframe, temp, phead, list)
 		rtw_free_xmitframe(pxmitpriv, pxmitframe);
-	}
+
 	spin_unlock_bh(&pframequeue->lock);
 }
 
-- 
2.20.1

Re: [PATCH v2 6/6] staging: rtl8188eu: use safe iterator in rtw_free_xmitframe_queue

[Guenter Roeck]

On Mon, May 17, 2021 at 10:18:26PM +0200, Martin Kaiser wrote:
> Use list_for_each_entry_safe, we may delete list items while iterating
> over the list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> v2:
>  - use list_for_each_entry_safe
> 
>  drivers/staging/rtl8188eu/core/rtw_xmit.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> index 3763d188b892..dcc29a74612d 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
> @@ -1329,17 +1329,15 @@ s32 rtw_free_xmitframe(struct xmit_priv *pxmitpriv, struct xmit_frame *pxmitfram
>  
>  void rtw_free_xmitframe_queue(struct xmit_priv *pxmitpriv, struct __queue *pframequeue)
>  {
> -	struct list_head *plist, *phead;
> -	struct	xmit_frame	*pxmitframe;
> +	struct list_head *phead;
> +	struct	xmit_frame	*pxmitframe, *temp;
>  
>  	spin_lock_bh(&pframequeue->lock);
>  
>  	phead = get_list_head(pframequeue);
> -	list_for_each(plist, phead) {
> -		pxmitframe = list_entry(plist, struct xmit_frame, list);
> -
> +	list_for_each_entry_safe(pxmitframe, temp, phead, list)
>  		rtw_free_xmitframe(pxmitpriv, pxmitframe);
> -	}
> +
>  	spin_unlock_bh(&pframequeue->lock);
>  }
>  
> -- 
> 2.20.1
> 

Re: [PATCH v2 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Guenter Roeck]

On Mon, May 17, 2021 at 10:18:21PM +0200, Martin Kaiser wrote:
> rtw_free_network_queue iterates over the scanned wireless networks and
> calls _rtw_free_network for each of them. In some cases,
> _rtw_free_network removes a network from the list.
> 
> We have to use list_for_each_entry_safe if we remove list entries while
> we iterate over a list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>

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

> ---
> v2:
>  - use list_for_each_entry_safe
> 
> Without this patch, it's easy to get the driver into an endless loop by
> scanning, connecting and disconnecting repeatedly.
> 
> wpa_supplicant -B -Dwext -i wlan0 -c /path/to/my/config...
> while true ; do
>    sleep 1
>    wpa_cli reconfigure
>    wpa_cli add_network
>    wpa_cli set_network 0 ssid ...
>    wpa_cli set_network 0 psk ...
>    wpa_cli select_network 0
>    sleep 6
>    wpa_cli status
>    wpa_cli disconnect 0
> done
> 
>  drivers/staging/rtl8188eu/core/rtw_mlme.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/staging/rtl8188eu/core/rtw_mlme.c b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> index 159465b073c2..ba73ac5325e2 100644
> --- a/drivers/staging/rtl8188eu/core/rtw_mlme.c
> +++ b/drivers/staging/rtl8188eu/core/rtw_mlme.c
> @@ -199,19 +199,17 @@ struct wlan_network *rtw_find_network(struct __queue *scanned_queue, u8 *addr)
>  
>  void rtw_free_network_queue(struct adapter *padapter, u8 isfreeall)
>  {
> -	struct list_head *phead, *plist;
> -	struct wlan_network *pnetwork;
> +	struct list_head *phead;
> +	struct wlan_network *pnetwork, *temp;
>  	struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
>  	struct __queue *scanned_queue = &pmlmepriv->scanned_queue;
>  
>  	spin_lock_bh(&scanned_queue->lock);
>  
>  	phead = get_list_head(scanned_queue);
> -	list_for_each(plist, phead) {
> -		pnetwork = list_entry(plist, struct wlan_network, list);
> -
> +	list_for_each_entry_safe(pnetwork, temp, phead, list)
>  		_rtw_free_network(pmlmepriv, pnetwork, isfreeall);
> -	}
> +
>  	spin_unlock_bh(&scanned_queue->lock);
>  }
>  
> -- 
> 2.20.1
> 

Re: [PATCH v2 1/6] staging: rtl8188eu: use safe iterator in rtw_free_network_queue

[Dan Carpenter]

On Mon, May 17, 2021 at 10:18:21PM +0200, Martin Kaiser wrote:
> rtw_free_network_queue iterates over the scanned wireless networks and
> calls _rtw_free_network for each of them. In some cases,
> _rtw_free_network removes a network from the list.
> 
> We have to use list_for_each_entry_safe if we remove list entries while
> we iterate over a list.
> 
> Fixes: 23017c8842d2 ("staging: rtl8188eu: Use list iterators and helpers")
> Signed-off-by: Martin Kaiser <martin@kaiser.cx>
> ---
> v2:
>  - use list_for_each_entry_safe
> 

I hadn't intended for you to redo the patch otherwise I would have
asked you to include endless loop script in all the commit messages
and rename "temp" (temperatures) to "tmp" (temporary).

You have to learn to let go, patches are never perfect.  But let's say
that these v2 patch are perfect and add a:

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

Don't resend!  :)

regards,
dan carpenter