linux-kernel - Re: [RFC] staging: r8188eu: Sleeping in atomic context (SAC) bugs

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220207092133.GE1951@kadam>
Date:   Mon, 7 Feb 2022 12:21:33 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@...il.com>
Cc:     linux-staging@...ts.linux.dev, gregkh@...uxfoundation.org,
        phil@...lpotter.co.uk, larry.finger@...inger.net,
        julia.lawall@...ia.fr, linux-kernel@...r.kernel.org,
        Michael Straube <michael.straube@...d.uni-goettingen.de>,
        martin@...ser.cx
Subject: Re: [RFC] staging: r8188eu: Sleeping in atomic context (SAC) bugs

On Mon, Feb 07, 2022 at 01:02:17AM +0100, Fabio M. De Francesco wrote:
> My first question is whether or not msleep() can be called in atomic context.

You are not allowed to call msleep() in atomic context.  The Smatch
check for sleeping in atomic did not look for msleep so I will update
it.

> If
> I understand its semantics and implementation it seems that it should be forbidden.
> What about changing it to mdelay()? Again, it seems that mdelay() spins without 
> sleeping so it should be safe. Isn't it?

mdelay() is does not sleep, but it's not necessarily a good idea to
delay for a long time while holding a spinlock.

> 
> Furthermore, I noticed that _rtw_pwr_wakeup() calls ips_leave(). The latter 
> acquires the "pwrpriv->lock" Mutex. Aren't we forbidden to call Mutexes while 
> holding Spinlocks?

Correct.  You cannot take a mutex while holding a spinlock.

Where is the spinlock in taken in the code you're talking about?  If
it's rtw_set_802_11_disassociate() then that's fine.  The check for
if (check_fwstate(pmlmepriv, _FW_LINKED)) { will prevent ips_leave()
from being called.

> My second question is: should we substitute that Mutex with a Spinlock and use 
> it everywhere else the struct "pwrctrl_priv" must be protected in the driver?

Changing mutexes to spinlocks is tricky.  I can't review your proposed
patch before you send it.

regards,
dan carpenter