[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXsGnpQ0RpsEebfu@stanley.mountain>
Date: Thu, 29 Jan 2026 10:05:02 +0300
From: Dan Carpenter <dan.carpenter@...aro.org>
To: Minu Jin <s9430939@...er.com>
Cc: gregkh@...uxfoundation.org, bqn9090@...il.com,
abrahamadekunle50@...il.com, straube.linux@...il.com,
bryant.boatright@...ton.me, davidzalman.101@...il.com,
linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] staging: rtl8723bs: Fix potential race in
expire_timeout_chk
On Thu, Jan 29, 2026 at 11:32:14AM +0900, Minu Jin wrote:
> The expire_timeout_chk function currently do lock and unlock inside the
> loop before calling rtw_free_stainfo().
>
> This can be risky as the list might be changed
> when the lock is briefly released.
>
> To fix this, move expired sta_info entries into a local free_list while
> holding the lock, and then perform the actual freeing after the lock is
> released.
>
> Signed-off-by: Minu Jin <s9430939@...er.com>
> ---
> Changes in v2:
> - Use LIST_HEAD for init list (suggested by Dan Carpenter)
> - Replace list_for_each_safe with list_for_each_entry_safe
> - Clean up unused variable 'plist' and fix type of 'tmp' iterator.
> - Remove redundant "free free_list" comment.
Sorry, you have gone overboard this time. I only wanted you to clean up
the new code which you introduced in the patch. Please don't clean up
the existing code in bugfix patch. If you want to do that, it has to be
done separately.
>
> drivers/staging/rtl8723bs/core/rtw_ap.c | 27 ++++++++++++-------------
> 1 file changed, 13 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
> index 67197c7d4a4d..d0a26134b67d 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_ap.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
> @@ -172,45 +172,44 @@ static u8 chk_sta_is_alive(struct sta_info *psta)
>
> void expire_timeout_chk(struct adapter *padapter)
> {
> - struct list_head *phead, *plist, *tmp;
> + struct list_head *phead;
> u8 updated = false;
> - struct sta_info *psta = NULL;
> + struct sta_info *psta = NULL, *tmp;
> struct sta_priv *pstapriv = &padapter->stapriv;
> u8 chk_alive_num = 0;
> char chk_alive_list[NUM_STA];
> int i;
>
> + LIST_HEAD(free_list);
Delete the blank line before "LIST_HEAD(free_list);" Don't put a blank
line in the declaration block.
regards,
dan carpenter
Powered by blists - more mailing lists