Syzbot reported use-after-free in rtl8712_dl_fw(). The problem was in
race condition between r871xu_dev_remove() ->ndo_open() callback.
It's easy to see from crash log, that driver accesses released firmware
in ->ndo_open() callback. It may happen, since driver was releasing
firmware _before_ unregistering netdev. Fix it by moving
unregister_netdev() before cleaning up resources.
Call Trace:
...
rtl871x_open_fw drivers/staging/rtl8712/hal_init.c:83 [inline]
rtl8712_dl_fw+0xd95/0xe10 drivers/staging/rtl8712/hal_init.c:170
rtl8712_hal_init drivers/staging/rtl8712/hal_init.c:330 [inline]
rtl871x_hal_init+0xae/0x180 drivers/staging/rtl8712/hal_init.c:394
netdev_open+0xe6/0x6c0 drivers/staging/rtl8712/os_intfs.c:380
__dev_open+0x2bc/0x4d0 net/core/dev.c:1484
Freed by task 1306:
...
release_firmware+0x1b/0x30 drivers/base/firmware_loader/main.c:1053
r871xu_dev_remove+0xcc/0x2c0 drivers/staging/rtl8712/usb_intf.c:599
usb_unbind_interface+0x1d8/0x8d0 drivers/usb/core/driver.c:458
Fixes: 8c213fa591 ("staging: r8712u: Use asynchronous firmware loading")
Cc: stable <stable@vger.kernel.org>
Reported-and-tested-by: syzbot+c55162be492189fb4f51@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/20211019211718.26354-1-paskripkin@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 406f42fa0d ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.
Convert staging from ether_addr_copy() to eth_hw_addr_set():
@@
expression dev, np;
@@
- ether_addr_copy(dev->dev_addr, np)
+ eth_hw_addr_set(dev, np)
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20211019171243.1412240-3-kuba@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 406f42fa0d ("net-next: When a bond have a massive amount
of VLANs...") introduced a rbtree for faster Ethernet address look
up. To maintain netdev->dev_addr in this tree we need to make all
the writes to it got through appropriate helpers.
Convert staging drivers from memcpy(... ETH_ADDR) to eth_hw_addr_set():
@@
expression dev, np;
@@
- memcpy(dev->dev_addr, np, ETH_ALEN)
+ eth_hw_addr_set(dev, np)
@@
- memcpy(dev->dev_addr, np, 6)
+ eth_hw_addr_set(dev, np)
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/r/20211019171243.1412240-2-kuba@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove the redundant first 'if' statement of two identical ones.
In rtw_cmd_thread() there are two identical 'if' statement, one
immediately after the other. They check whether or not the device is
removed or the driver is stopped and, if true, they break a 'while'
loop.
The only noteworthy context difference is that the second statement is
within a block labelled "_next". The code has a 'goto' to the "_next"
label so that the checking is performed each time the above directive
is encountered. Instead, the first 'if' is before the "_next" label.
One of the two must be removed and that it must be the one before the
label because "bSurpriseRemoved" as well as "bDriverStopped" may be
changed asynchronously by other code of the driver and so they should be
checked at each jump to "_next".
Tested with "ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano]".
Acked-by: Martin Kaiser <martin@kaiser.cx>
Acked-by: Phillip Potter <phil@philpotter.co.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Link: https://lore.kernel.org/r/20211018162006.5527-4-fmdefrancesco@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
rtw_enqueue_cmd() uses a semaphore to notify rtw_cmd_thread() that it
has enqueued commands. rtw_cmd_thread() "down(s)" in interruptible mode
to wait to be notified.
Use completion variables because they are better suited for the purpose.
In rtw_cmd_thread(), wait in uninterruptible mode, even if the original
code uses down_interruptible(), because the interruption of
rtw_cmd_thread() is not allowed and unwanted.
Tested with "ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano]".
Acked-by: Phillip Potter <phil@philpotter.co.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Link: https://lore.kernel.org/r/20211018162006.5527-3-fmdefrancesco@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
rtw_cmd_thread() "up(s)" a semaphore twice, first to notify callers when
its execution is started and then to notify when it is about to end.
It makes the same semaphore go "up" twice in the same thread. This
construct makes Smatch to warn of duplicate "up(s)".
This thread uses interruptible semaphores where instead completions are
more suitable. For this purpose it calls an helper (_rtw_down_sema())
that returns values that are never checked. It may lead to bugs.
To address the above-mentioned issues, use two completions variables
instead of semaphores. Use the uninterruptible versions of
wake_for_completion*() because the interruptible / killable versions are
not necessary.
Tested with "ASUSTek Computer, Inc. Realtek 8188EUS [USB-N10 Nano]".
Acked-by: Phillip Potter <phil@philpotter.co.uk>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Link: https://lore.kernel.org/r/20211018162006.5527-2-fmdefrancesco@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Replace strncpy with strlcpy to fix the following gcc warning.
drivers/staging/r8188eu/os_dep/ioctl_linux.c: In function 'rtw_wx_set_enc_ext':
drivers/staging/r8188eu/os_dep/ioctl_linux.c:1929:9: warning: 'strncpy' specified bound 16 equals destination size [-Wstringop-truncation]
1929 | strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
The destination buffer size is IEEE_CRYPT_ALG_NAME_LEN and the length
of the string to copy is always < IEEE_CRYPT_ALG_NAME_LEN. So strlcpy
will never truncate the string.
Acked-by: Phillip Potter <phil@philpotter.co.uk>
Signed-off-by: Michael Straube <straube.linux@gmail.com>
Link: https://lore.kernel.org/r/20211018221231.7837-1-straube.linux@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Nodes for 'i2s' and 'nand' have no driver present inside the linux tree.
The normal approach for a dts file to be mainlined is start with those stuff
which is already mainlined and get rid of the other stuff. If needed it will
be properly added afterwards together with the suitable device driver. Hence,
remove both nodes from the device tree include file.
Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
Link: https://lore.kernel.org/r/20211018170206.11959-1-sergio.paracuellos@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
At the moment, our command thread can be killed by user space.
[root@host ]# kill `pidof RTW_CMD_THREAD`
The driver will then stop working until the module is unloaded
and reloaded.
Don't process SIGTERM in the command thread. Other drivers that have a
command thread don't process SIGTERM either.
Acked-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Martin Kaiser <martin@kaiser.cx>
Link: https://lore.kernel.org/r/20211016181343.3686-4-martin@kaiser.cx
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
