Hello everyone, I've noticed a couple of SAC bugs in r8188eu. Recently two of them have been addressed by me and one by Michael Straube (sorry if I recall it wrongly). Prof. Julia Lawall wrote with Others in a paper that "[] Code executing in atomic context monopolizes a CPU core, and the progress of other threads that need to concurrently access the same resources is delayed. Thus the code execution in atomic context should complete as quickly as possible. Sleeping in atomic context is forbidden, as it can block a CPU core for a long period and may lead to a system hang. [] SAC bugs do not always cause problems in real execution, and they are often hard to reproduce at runtime. Recent studies [] have shown that SAC bugs have caused serious system hangs at runtime. Thus, it is necessary to detect and fix SAC bugs in kernel modules.". Probably the phrase "SAC bugs do not always cause problems in real execution" is the key to understand why very few people seems to care and fix them. While working on my last patch for this driver, I noticed that a Mutex is acquired while holding a Spinlock and while bottom halves are disabled. In staging/r8188eu/core/rtw_ioctl_set.c, the function rtw_set_802_11_disassociate() calls spin_lock_bh(). While holding that Spinlock, it calls _rtw_pwr_wakeup() that, in turn, does a call to msleep(). My first question is whether or not msleep() can be called in atomic context. 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? 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? That lock is used to protect struct "pwrctrl_priv". I didn't yet provide a patch because I'm pretty sure that I'm missing the good and correct reasons that led the author(s) to use a Mutex in that context... 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? Looking forward to reading what you all think about these two questions of mine. Regards, Fabio M. De Francesco
