There was strange error handling logic in case of fw load failure. For
some reason fw loader callback was doing clean up stuff when fw is not
available. I don't see any reason behind doing this. Since this driver
doesn't have EEPROM firmware let's just disconnect it in case of fw load
failure. Doing clean up stuff in 2 different place which can run
concurently is not good idea and syzbot found 2 bugs related to this
strange approach.
So, in this pacth I deleted all clean up code from fw callback and made
a call to device_release_driver() under device_lock(parent) in case of fw
load failure. This approach is more generic and it defend driver from UAF
bugs, since all clean up code is moved to one place.
Fixes: e02a3b9458 ("staging: rtl8712: fix memory leak in rtl871x_load_fw_cb")
Fixes: 8c213fa591 ("staging: r8712u: Use asynchronous firmware loading")
Cc: stable <stable@vger.kernel.org>
Reported-and-tested-by: syzbot+5872a520e0ce0a7c7230@syzkaller.appspotmail.com
Reported-and-tested-by: syzbot+cc699626e48a6ebaf295@syzkaller.appspotmail.com
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/d49ecc56e97c4df181d7bd4d240b031f315eacc3.1626895918.git.paskripkin@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch is preparation for following patch for error handling
refactoring.
flush_scheduled_work() takes (wq_completion)events lock and
it can lead to deadlock when r871xu_dev_remove() is called from workqueue.
To avoid deadlock sutiation we can change flush_scheduled_work() call to
flush_work() call for all possibly scheduled works in this driver,
since next patch adds device_release_driver() in case of fw load failure.
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Cc: stable <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/6e028b4c457eeb7156c76c6ea3cdb3cb0207c7e1.1626895918.git.paskripkin@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
There is a leak in rtl8712 driver.
The problem was in non-freed adapter data if
firmware load failed.
This leak can be reproduced with this code:
https://syzkaller.appspot.com/text?tag=ReproC&x=16612f02d00000,
Autoload must fail (to not hit memory leak reported by syzkaller)
There are 2 possible ways how rtl871x_load_fw_cb() and
r871xu_dev_remove() can be called (in case of fw load error).
1st case:
r871xu_dev_remove() then rtl871x_load_fw_cb()
In this case r871xu_dev_remove() will wait for
completion and then will jump to the end, because
rtl871x_load_fw_cb() set intfdata to NULL:
if (pnetdev) {
struct _adapter *padapter = netdev_priv(pnetdev);
/* never exit with a firmware callback pending */
wait_for_completion(&padapter->rtl8712_fw_ready);
pnetdev = usb_get_intfdata(pusb_intf);
usb_set_intfdata(pusb_intf, NULL);
if (!pnetdev)
goto firmware_load_fail;
... clean up code here ...
}
2nd case:
rtl871x_load_fw_cb() then r871xu_dev_remove()
In this case pnetdev (from code snippet above) will
be zero (because rtl871x_load_fw_cb() set it to NULL)
And clean up code won't be executed again.
So, in all cases we need to free adapted data in rtl871x_load_fw_cb(),
because disconnect function cannot take care of it. And there won't be
any race conditions, because complete() call happens after setting
intfdata to NULL.
In previous patch I moved out free_netdev() from r8712_free_drv_sw()
and that's why now it's possible to free adapter data and then call
complete.
Fixes: 8c213fa591 ("staging: r8712u: Use asynchronous firmware loading")
Signed-off-by: Pavel Skripkin <paskripkin@gmail.com>
Link: https://lore.kernel.org/r/81e68fe0194499cc2e7692d35bc4dcf167827d8f.1623620630.git.paskripkin@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We recently moved "bssid" off the stack, and allocated it with kmalloc()
instead. Unfortunately, this one line was overlooked so it will copy
random data into the &tgt_network->network instead of the data we want.
Fixes: 0b18e5fe60 ("staging: rtl8712: rtl871x_mp_ioctl: Move a large data struct onto the heap")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Link: https://lore.kernel.org/r/YJ6IrfkbdaTHgpEv@mwanda
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Commit 6e2baa44c6 ("staging: rtl8712: remove enum WIFI_FRAME_SUBTYPE")
was wrong because:
WIFI_DATA_NULL != IEEE80211_STYPE_NULLFUNC
WIFI_DATA_CFACK != IEEE80211_STYPE_DATA_CFACK
WIFI_DATA_CFPOLL != IEEE80211_STYPE_DATA_CFPOLL
WIFI_DATA_CFACKPOLL != IEEE80211_STYPE_DATA_CFACKPOLL
the WIFI_DATA_xxx definitions include WIFI_DATA_TYPE, which is 'BIT(3)'.
Restore the previous behavior by adding the missing
'IEEE80211_FTYPE_DATA |' (0x0008, that is to say BIT(3)) when these values
are used.
Hopefully, the wrong commit was small enough and hand review is possible.
Fixes: 6e2baa44c6 ("staging: rtl8712: remove enum WIFI_FRAME_SUBTYPE")
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/44aebfa3c5ce8f45ae05369c73e9ff77c6d271f9.1619939806.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Use existing list iterators and helper functions.
The following coccinelle script was used to convert the code.
@@
identifier v1, v2, v3, v4;
symbol next;
expression e;
iterator name list_for_each;
statement S;
@@
<+...
(
- e = v1->next;
|
- e = get_next(v1);
)
... when != e
- while ( \( v1 != e \| e != v1 \) )
+ list_for_each (e, v1)
{
...
- v2 = container_of(e, struct v3, v4);
+ v2 = list_entry(e, struct v3, v4);
?- if (!v2) S
...
(
- e = e->next;
|
- e = get_next(e);
)
... when != e
}
...+>
Compile tested only.
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20210428173523.149958-1-linux@roeck-us.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fixes the following W=1 kernel build warning(s):
drivers/staging/rtl8712/rtl871x_mp_ioctl.h:256:34: warning: ‘oid_rtl_seg_81_85’ defined but not used [-Wunused-const-variable=]
drivers/staging/rtl8712/rtl871x_mp_ioctl.h:249:34: warning: ‘oid_rtl_seg_81_80_80’ defined but not used [-Wunused-const-variable=]
drivers/staging/rtl8712/rtl871x_mp_ioctl.h:240:34: warning: ‘oid_rtl_seg_81_80_40’ defined but not used [-Wunused-const-variable=]
drivers/staging/rtl8712/rtl871x_mp_ioctl.h:205:34: warning: ‘oid_rtl_seg_81_80_20’ defined but not used [-Wunused-const-variable=]
drivers/staging/rtl8712/rtl871x_mp_ioctl.h:138:34: warning: ‘oid_rtl_seg_81_80_00’ defined but not used [-Wunused-const-variable=]
Cc: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Florian Schilhabel <florian.c.schilhabel@googlemail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: WLAN FAE <wlanfae@realtek.com>
Cc: linux-staging@lists.linux.dev
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Link: https://lore.kernel.org/r/20210414181129.1628598-13-lee.jones@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Return NULL from r8712_find_network() if no matched wlan_network
was found. Code with a bug:
while (plist != phead) {
pnetwork = container_of(plist, struct wlan_network, list);
plist = plist->next;
if (!memcmp(addr, pnetwork->network.MacAddress, ETH_ALEN))
break;
}
spin_unlock_irqrestore(&scanned_queue->lock, irqL);
return pnetwork;
In this code last processed pnetwork returned if list end was reached
and no pnetwork matched test condition.
Signed-off-by: Sergei Krainov <sergei.krainov.lkd@gmail.com>
Link: https://lore.kernel.org/r/20210409124611.GA3981@test-VirtualBox
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Some constants defined in wifi.h are already defined in <linux/ieee80211.h>
with some other (but similar) names.
Be consistent and use the ones from <linux/ieee80211.h>.
The conversions made are:
_SSID_IE_ --> WLAN_EID_SSID
_SUPPORTEDRATES_IE_ --> WLAN_EID_SUPP_RATES
_DSSET_IE_ --> WLAN_EID_DS_PARAMS
_IBSS_PARA_IE_ --> WLAN_EID_IBSS_PARAMS
_ERPINFO_IE_ --> WLAN_EID_ERP_INFO
_EXT_SUPPORTEDRATES_IE_ --> WLAN_EID_EXT_SUPP_RATES
_HT_CAPABILITY_IE_ --> WLAN_EID_HT_CAPABILITY
_HT_EXTRA_INFO_IE_ --> WLAN_EID_HT_OPERATION (not used)
_HT_ADD_INFO_IE_ --> WLAN_EID_HT_OPERATION
_VENDOR_SPECIFIC_IE_ --> WLAN_EID_VENDOR_SPECIFIC
_RESERVED47_ --> (not used)
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/fe35fb45323adc3a30f31b7280cec7700fd325d8.1617741313.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
