linux-kernel - Re: [PATCH] drivers: staging: r8188eu: Fix sleep-in-atomic-context bug in rtw_join_timeout

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

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

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

Message-ID: <77ff753.140ebd.183f93dfb04.Coremail.duoming@zju.edu.cn>
Date:   Fri, 21 Oct 2022 14:32:12 +0800 (GMT+08:00)
From:   duoming@....edu.cn
To:     "Greg KH" <gregkh@...uxfoundation.org>
Cc:     linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Larry.Finger@...inger.net, phil@...lpotter.co.uk,
        paskripkin@...il.com, martin@...ser.cx, straube.linux@...il.com,
        kuba@...nel.org
Subject: Re: [PATCH] drivers: staging: r8188eu: Fix sleep-in-atomic-context
 bug in rtw_join_timeout_handler

Hello,

On Thu, 20 Oct 2022 17:46:47 +0200 Greg KH wrote:

> On Tue, Oct 18, 2022 at 04:34:24PM +0800, Duoming Zhou wrote:
> > The rtw_join_timeout_handler() is a timer handler that
> > runs in atomic context, but it could call msleep().
> > As a result, the sleep-in-atomic-context bug will happen.
> > The process is shown below:
> > 
> >      (atomic context)
> > rtw_join_timeout_handler
> 
> Wait, how is this an atomic timeout?

Because this function is defined as a timer handler of "assoc_timer".

The following is the detail:

void rtw_init_mlme_timer(struct adapter *padapter)
{
	struct	mlme_priv *pmlmepriv = &padapter->mlmepriv;

	timer_setup(&pmlmepriv->assoc_timer, rtw_join_timeout_handler, 0);
        ...
}

https://elixir.bootlin.com/linux/latest/source/drivers/staging/r8188eu/os_dep/mlme_linux.c#L36

> When can that happen?

When the adapter trys to join and selects scanning queue successfully,
the assoc_timer will be actived. If this process is timeout, the callback
function rtw_join_timeout_handler will run.

> >  _rtw_join_timeout_handler
> >   rtw_do_join
> >    rtw_select_and_join_from_scanned_queue
> >     rtw_indicate_disconnect
> >      rtw_lps_ctrl_wk_cmd
> >       lps_ctrl_wk_hdl
> >        LPS_Leave
> >         LPS_RF_ON_check
> >          msleep //sleep in atomic context
> 
> How was this found?
> 
> > Fix by removing msleep() and replacing with mdelay().
> 
> Wouldn't people have seen an error already if msleep() was really called
> in atomic context?

I am sorry, This is the false alarm.

rtw_indicate_disconnect()
  -->rtw_lps_ctrl_wk_cmd(padapter, LPS_CTRL_DISCONNECT, 1);

u8 rtw_lps_ctrl_wk_cmd(struct adapter *padapter, u8 lps_ctrl_type, u8 enqueue)
{
...
if (enqueue) {
...
}else {
    lps_ctrl_wk_hdl(padapter, lps_ctrl_type);
}

The enqueue equals to 1 and the lps_ctrl_wk_hdl() will not execute.

I will check carefully and avoid false alarm next time. Thank you for your reply.

Best regards,
Duoming Zhou