Remove _enter_critical_mutex() and _exit_critical_mutex(). They are
unnecessary wrappers, respectively to mutex_lock_interruptible() and
to mutex_unlock(). They also have an odd interface that takes an unused
argument named pirqL of type unsigned long.
The original code enters the critical section if the mutex API is
interrupted while waiting to acquire the lock; therefore it could lead
to a race condition. Use mutex_lock() because it is uninterruptible and
so avoid that above-mentioned potential race condition.
Tested-by: Pavel Skripkin <paskripkin@gmail.com>
Reviewed-by: Pavel Skripkin <paskripkin@gmail.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Link: https://lore.kernel.org/r/20210828113656.6963-1-fmdefrancesco@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
While testing Fabio's patch I hit lockdep warning about possible
deadlock:
[ 252.063305] WARNING: possible recursive locking detected
[ 252.063642] 5.14.0+ #9 Tainted: G C
[ 252.063946] --------------------------------------------
[ 252.064282] ip/335 is trying to acquire lock:
[ 252.064560] ffff888009ebad28 (pmutex){+.+.}-{4:4}, at: usbctrl_vendorreq+0xc5/0x4a0 [r8188eu]
[ 252.065168]
[ 252.065168] but task is already holding lock:
[ 252.065536] ffffffffc021b3b8 (pmutex){+.+.}-{4:4}, at: netdev_open+0x3a/0x5f [r8188eu]
[ 252.066085]
[ 252.066085] other info that might help us debug this:
[ 252.066494] Possible unsafe locking scenario:
[ 252.066494]
[ 252.066866] CPU0
[ 252.067025] ----
[ 252.067184] lock(pmutex);
[ 252.067367] lock(pmutex);
There is one problem with this warning: there is no pmutex in this
driver, *BUT* all mutexes are initialized via private _rtw_mutex_init
API, which had struct mutex *pmutex argument.
So, all mutexes in this driver had same name in lockdep map. Of course,
lockdep will complain about any nested locking.
Fix it by open-coding _rtw_mutex_{init,free} wrappers, because we do not
need them at all.
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/20210904124747.30050-1-paskripkin@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
smatch says:
usb_intf.c:326 rtw_hw_suspend() warn: variable dereferenced before check 'padapter' (see line 323)
usb_intf.c:387 rtw_hw_resume() warn: variable dereferenced before check 'padapter' (see line 385)
There is only one caller of rtw_hw_suspend() and it does not check
padapter pointer, so let's just omit this check to make smatch happy.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/20210905204218.19317-1-paskripkin@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove rtw_hal_c2h_id_filter_ccx from hal/hal_intf.c and its one caller
from core/rtw_cmd.c. This function is a wrapper function which returns
the c2h_id_filter_ccx function pointer of struct hal_ops unconditionally.
As this function pointer is never set, and the function call's return
value is subsequently called inside an if condition, this could lead to
an attempt to deference a NULL pointer, which would crash the driver.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-14-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove static Efuse_PgPacketWrite_BT and its one caller from
core/rtw_efuse.c. This function is a wrapper function which calls
the Efuse_PgPacketWrite_BT function pointer of struct hal_ops
unconditionally. As this function pointer is never set, and this
function call is possible to reach, this could lead to an attempt to
deference a NULL pointer, which would crash the driver.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-12-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove rtw_hal_xmitframe_enqueue from hal/hal_intf.c and its one
caller from core/rtw_recv.c, and remove its declaration from
include/hal_intf.h as well. This is just a wrapper function that calls
the function pointer hal_xmitframe_enqueue in struct hal_ops if it
is set, which it never is.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-10-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove rtw_hal_interrupt_handler from hal/hal_intf.c, and remove its
declaration from include/hal_intf.h as well. This is just a wrapper
function that calls the function pointer interrupt_handler in struct
hal_ops if it is set, which it never is. In addition, this wrapper
function is unused anyway.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-8-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove rtw_hal_disable_interrupt from hal/hal_intf.c, and remove its
declaration from include/hal_intf.h as well. This is just a wrapper
function that calls the function pointer disable_interrupt in struct
hal_ops if it is set, which it never is. In addition, this wrapper
function is unused anyway.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-6-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove rtw_hal_enable_interrupt from hal/hal_intf.c, and remove its
declaration from include/hal_intf.h as well. This is just a wrapper
function that calls the function pointer enable_interrupt in struct
hal_ops if it is set, which it never is. In addition, this wrapper
function is unused anyway.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-4-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove rtw_hal_reset_security_engine and its one caller from
hal/hal_intf.c, and remove its declaration from include/hal_intf.h as
well. This is just a wrapper function that calls the function pointer
hal_reset_security_engine in struct hal_ops if it is set, which it never
is.
Signed-off-by: Phillip Potter <phil@philpotter.co.uk>
Link: https://lore.kernel.org/r/20210906010106.898-2-phil@philpotter.co.uk
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
