* [PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode()
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
@ 2021-06-07 18:17 ` Dan Carpenter
2021-06-09 10:33 ` Guenter Roeck
2021-06-07 18:17 ` [PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl() Dan Carpenter
` (5 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:17 UTC (permalink / raw)
To: Larry Finger, Guenter Roeck
Cc: Greg Kroah-Hartman, Martin Kaiser, Ivan Safonov, Paul McQuade,
Michael Straube, linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode()
2021-06-07 18:17 ` [PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode() Dan Carpenter
@ 2021-06-09 10:33 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:33 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Martin Kaiser, Ivan Safonov,
Paul McQuade, Michael Straube, linux-staging, kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl()
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
2021-06-07 18:17 ` [PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode() Dan Carpenter
@ 2021-06-07 18:17 ` Dan Carpenter
2021-06-09 10:34 ` Guenter Roeck
2021-06-07 18:17 ` [PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue() Dan Carpenter
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:17 UTC (permalink / raw)
To: Larry Finger, Guenter Roeck
Cc: Greg Kroah-Hartman, Ivan Safonov, Michael Straube,
Christophe JAILLET, Peilin Ye, linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl()
2021-06-07 18:17 ` [PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl() Dan Carpenter
@ 2021-06-09 10:34 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Ivan Safonov, Michael Straube,
Christophe JAILLET, Peilin Ye, linux-staging, kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue()
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
2021-06-07 18:17 ` [PATCH 1/7] staging: rtl8188eu: use safe iterator in stop_ap_mode() Dan Carpenter
2021-06-07 18:17 ` [PATCH 2/7] staging: rtl8188eu: use safe iterator in tx_beacon_hdl() Dan Carpenter
@ 2021-06-07 18:17 ` Dan Carpenter
2021-06-09 10:35 ` Guenter Roeck
2021-06-07 18:17 ` [PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit() Dan Carpenter
` (3 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:17 UTC (permalink / raw)
To: Larry Finger, Guenter Roeck
Cc: Greg Kroah-Hartman, Martin Kaiser, Ivan Safonov, Deborah Brouwer,
Simon Fong, Michael Straube, linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue()
2021-06-07 18:17 ` [PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue() Dan Carpenter
@ 2021-06-09 10:35 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:35 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Martin Kaiser, Ivan Safonov,
Deborah Brouwer, Simon Fong, Michael Straube, linux-staging,
kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit()
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
` (2 preceding siblings ...)
2021-06-07 18:17 ` [PATCH 3/7] staging: rtl8188eu: use safe iterator in dequeue_xmitframes_to_sleeping_queue() Dan Carpenter
@ 2021-06-07 18:17 ` Dan Carpenter
2021-06-09 10:36 ` Guenter Roeck
2021-06-07 18:18 ` [PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames() Dan Carpenter
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:17 UTC (permalink / raw)
To: Larry Finger
Cc: Greg Kroah-Hartman, Guenter Roeck, Ivan Safonov, Martin Kaiser,
Michael Straube, Simon Fong, linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit()
2021-06-07 18:17 ` [PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit() Dan Carpenter
@ 2021-06-09 10:36 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:36 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Ivan Safonov, Martin Kaiser,
Michael Straube, Simon Fong, linux-staging, kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames()
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
` (3 preceding siblings ...)
2021-06-07 18:17 ` [PATCH 4/7] staging: rtl8188eu: use safe iterator in wakeup_sta_to_xmit() Dan Carpenter
@ 2021-06-07 18:18 ` Dan Carpenter
2021-06-09 10:37 ` Guenter Roeck
2021-06-07 18:18 ` [PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete() Dan Carpenter
2021-06-07 18:19 ` [PATCH 7/7] staging: rtl8188eu: delete some dead code Dan Carpenter
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:18 UTC (permalink / raw)
To: Larry Finger, Guenter Roeck
Cc: Greg Kroah-Hartman, Martin Kaiser, Ivan Safonov, Simon Fong,
Michael Straube, linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames()
2021-06-07 18:18 ` [PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames() Dan Carpenter
@ 2021-06-09 10:37 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Martin Kaiser, Ivan Safonov,
Simon Fong, Michael Straube, linux-staging, kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete()
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
` (4 preceding siblings ...)
2021-06-07 18:18 ` [PATCH 5/7] staging: rtl8188eu: use safe iterator in xmit_delivery_enabled_frames() Dan Carpenter
@ 2021-06-07 18:18 ` Dan Carpenter
2021-06-09 10:37 ` Guenter Roeck
2021-06-07 18:19 ` [PATCH 7/7] staging: rtl8188eu: delete some dead code Dan Carpenter
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:18 UTC (permalink / raw)
To: Larry Finger, Guenter Roeck
Cc: Greg Kroah-Hartman, Michael Straube, Romain Perier, Allen Pais,
linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete()
2021-06-07 18:18 ` [PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete() Dan Carpenter
@ 2021-06-09 10:37 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:37 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Michael Straube, Romain Perier,
Allen Pais, linux-staging, kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 7/7] staging: rtl8188eu: delete some dead code
2021-06-07 18:15 [PATCH 0/7] staging: rtl8188eu: use safe version of list iterators Dan Carpenter
` (5 preceding siblings ...)
2021-06-07 18:18 ` [PATCH 6/7] staging: rtl8188eu: use safe iterator in rtl8188eu_xmitframe_complete() Dan Carpenter
@ 2021-06-07 18:19 ` Dan Carpenter
2021-06-09 10:39 ` Guenter Roeck
6 siblings, 1 reply; 15+ messages in thread
From: Dan Carpenter @ 2021-06-07 18:19 UTC (permalink / raw)
To: Larry Finger
Cc: Greg Kroah-Hartman, Guenter Roeck, Michael Straube,
Romain Perier, Allen Pais, linux-staging, kernel-janitors
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH 7/7] staging: rtl8188eu: delete some dead code
2021-06-07 18:19 ` [PATCH 7/7] staging: rtl8188eu: delete some dead code Dan Carpenter
@ 2021-06-09 10:39 ` Guenter Roeck
0 siblings, 0 replies; 15+ messages in thread
From: Guenter Roeck @ 2021-06-09 10:39 UTC (permalink / raw)
To: Dan Carpenter
Cc: Larry Finger, Greg Kroah-Hartman, Michael Straube, Romain Perier,
Allen Pais, linux-staging, kernel-janitors
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
>
^ permalink raw reply [flat|nested] 15+ messages in thread