* Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
From: Kalle Valo @ 2019-05-28 11:55 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: pkshih, davem, linux-wireless, netdev, linux-kernel, Jia-Ju Bai
In-Reply-To: <20190514123439.10524-1-baijiaju1990@gmail.com>
Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> *BUG 1:
> In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails,
> rtl_deinit_core() in the error handling code is executed.
> rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses
> rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been
> initialized. Thus a null-pointer dereference occurs.
> The reason is that rtlpriv->scan_list.list is initialized by
> INIT_LIST_HEAD() in rtl_init_core(), which has not been called.
>
> To fix this bug, rtl_deinit_core() should not be called when
> rtlpriv->cfg->ops->init_sw_vars() fails.
>
> *BUG 2:
> In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in
> this function fails, and rtlpriv->scan_list.list has not been
> initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error
> handling code of rtl_pci_probe() is executed. Finally, a null-pointer
> dereference occurs due to the same reason of the above bug.
>
> To fix this bug, the initialization of lists in rtl_init_core() are
> performed before the call to rtl_regd_init().
>
> These bugs are found by a runtime fuzzing tool named FIZZER written by
> us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Ping & Larry, is this ok to take?
--
https://patchwork.kernel.org/patch/10942971/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [RFC] rtlwifi: rtl8821ae: Use inline routines rather than macros for descriptor word 0
From: Kalle Valo @ 2019-05-28 12:08 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, pkshih
In-Reply-To: <20190506173916.16486-1-Larry.Finger@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> The driver uses complicated macros to set parts of word 0 of the TX and RX
> descriptors. These are changed into inline routines.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
>
> Kalle,
>
> Based on your comment on how much you dislike those "byte macros", I have
> converted a few of them from rtl8821ae into static inline functions.
>
> Is this what you had in mind, and do you consider these changes to
> improve the code?
[...]
> --- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.h
> +++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/trx.h
> @@ -14,25 +14,67 @@
> #define USB_HWDESC_HEADER_LEN 40
> #define CRCLENGTH 4
>
> -#define SET_TX_DESC_PKT_SIZE(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 0, 16, __val)
> -#define SET_TX_DESC_OFFSET(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 16, 8, __val)
> -#define SET_TX_DESC_BMC(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 24, 1, __val)
> -#define SET_TX_DESC_HTC(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 25, 1, __val)
> -#define SET_TX_DESC_LAST_SEG(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 26, 1, __val)
> -#define SET_TX_DESC_FIRST_SEG(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 27, 1, __val)
> -#define SET_TX_DESC_LINIP(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 28, 1, __val)
> -#define SET_TX_DESC_OWN(__pdesc, __val) \
> - SET_BITS_TO_LE_4BYTE(__pdesc, 31, 1, __val)
> -
> -#define GET_TX_DESC_OWN(__pdesc) \
> - LE_BITS_TO_4BYTE(__pdesc, 31, 1)
> +/* Set packet size (16 bits) in TX descriptor word 0 */
> +static inline void set_tx_desc_pkt_size(__le32 *__pdesc, u16 __val)
> +{
> + *__pdesc = cpu_to_le32((le32_to_cpu(*__pdesc) & ~GENMASK(0, 15)) |
> + __val);
> +}
This was not exactly what I had mind. My point was that the firmware
command or event should be handled as a complete structure, not parsed
one (or four) bytes at a time. To show what I mean here's a random
example from iwlwifi:
struct iwl_alive_resp {
u8 ucode_minor;
u8 ucode_major;
__le16 reserved1;
u8 sw_rev[8];
u8 ver_type;
u8 ver_subtype; /* not "9" for runtime alive */
__le16 reserved2;
__le32 log_event_table_ptr; /* SRAM address for event log */
__le32 error_event_table_ptr; /* SRAM address for error log */
__le32 timestamp;
__le32 is_valid;
} __packed;
This a nice, clean and robust way both use AND document an event coming
from the firmware. And the driver can pass around 'struct iwl_alive_res
*resp' pointer to other functions to make it easy to understand what
event the buffer contains. Compared to rtlwifi where there's just 'u8
buf[]' and nobody has any clue what it contains, and need to spend at
least five minutes figuring that out everytime they are looking at a
function.
But to be honest I'm not sure if it's worth trying to cleanup rtlwifi,
it really would need a full rewrite to become a clean driver. IMHO much
better to put more effort on rtl8xxxxu and rtw88, which both already are
clean drivers.
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] network: wireless: p54u: Fix race between disconnect and firmware loading
From: Kalle Valo @ 2019-05-28 12:11 UTC (permalink / raw)
To: Alan Stern
Cc: Christian Lamparter, syzbot, davem, andreyknvl, syzkaller-bugs,
Kernel development list, USB list, linux-wireless, netdev
In-Reply-To: <Pine.LNX.4.44L0.1905201042110.1498-100000@iolanthe.rowland.org>
Alan Stern <stern@rowland.harvard.edu> writes:
> The syzbot fuzzer found a bug in the p54 USB wireless driver. The
> issue involves a race between disconnect and the firmware-loader
> callback routine, and it has several aspects.
>
> One big problem is that when the firmware can't be loaded, the
> callback routine tries to unbind the driver from the USB _device_ (by
> calling device_release_driver) instead of from the USB _interface_ to
> which it is actually bound (by calling usb_driver_release_interface).
>
> The race involves access to the private data structure. The driver's
> disconnect handler waits for a completion that is signalled by the
> firmware-loader callback routine. As soon as the completion is
> signalled, you have to assume that the private data structure may have
> been deallocated by the disconnect handler -- even if the firmware was
> loaded without errors. However, the callback routine does access the
> private data several times after that point.
>
> Another problem is that, in order to ensure that the USB device
> structure hasn't been freed when the callback routine runs, the driver
> takes a reference to it. This isn't good enough any more, because now
> that the callback routine calls usb_driver_release_interface, it has
> to ensure that the interface structure hasn't been freed.
>
> Finally, the driver takes an unnecessary reference to the USB device
> structure in the probe function and drops the reference in the
> disconnect handler. This extra reference doesn't accomplish anything,
> because the USB core already guarantees that a device structure won't
> be deallocated while a driver is still bound to any of its interfaces.
>
> To fix these problems, this patch makes the following changes:
>
> Call usb_driver_release_interface() rather than
> device_release_driver().
>
> Don't signal the completion until after the important
> information has been copied out of the private data structure,
> and don't refer to the private data at all thereafter.
>
> Lock udev (the interface's parent) before unbinding the driver
> instead of locking udev->parent.
>
> During the firmware loading process, take a reference to the
> USB interface instead of the USB device.
>
> Don't take an unnecessary reference to the device during probe
> (and then don't drop it during disconnect).
>
> Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
> Reported-and-tested-by: syzbot+200d4bb11b23d929335f@syzkaller.appspotmail.com
> CC: <stable@vger.kernel.org>
>
> ---
>
>
> [as1899]
>
>
> drivers/net/wireless/intersil/p54/p54usb.c | 43 ++++++++++++-----------------
> 1 file changed, 18 insertions(+), 25 deletions(-)
The correct prefix is "p54:", but I can fix that during commit.
> Index: usb-devel/drivers/net/wireless/intersil/p54/p54usb.c
> ===================================================================
> --- usb-devel.orig/drivers/net/wireless/intersil/p54/p54usb.c
> +++ usb-devel/drivers/net/wireless/intersil/p54/p54usb.c
> @@ -33,6 +33,8 @@ MODULE_ALIAS("prism54usb");
> MODULE_FIRMWARE("isl3886usb");
> MODULE_FIRMWARE("isl3887usb");
>
> +static struct usb_driver p54u_driver;
How is it safe to use static variables from a wireless driver? For
example, what if there are two p54 usb devices on the host? How do we
avoid a race in that case?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-28 12:14 UTC (permalink / raw)
To: Kalle Valo; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190528113922.E2B1060312@smtp.codeaurora.org>
On Tue, May 28, 2019 at 11:39:22AM +0000, Kalle Valo wrote:
> Gen Zhang <blackgod016574@gmail.com> wrote:
>
> > In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
> > when this allocation fails, ENOMEM is returned. However, 'pdev_data'
> > and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
> > platform_device_alloc() returns NULL, we should also free 'pdev_data'
> > and 'glue' before wl1271_probe() ends to prevent leaking memory.
> >
> > Similarly, we shoulf free 'pdev_data' when 'glue' is NULL. And we should
> > free 'pdev_data' and 'glue' when 'glue->reg' is error and when 'ret' is
> > error.
> >
> > Further, we should free 'glue->core', 'pdev_data' and 'glue' when this
> > function normally ends to prevent leaking memory.
> >
> > Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
>
> Same questions as with similar SDIO patch:
>
> https://patchwork.kernel.org/patch/10959049/
>
> Patch set to Changes Requested.
>
> --
> https://patchwork.kernel.org/patch/10959053/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>
Thanks for your reply, Kalle. I had debate with Jon about this patch.
You could kindly refer to lkml: https://lkml.org/lkml/2019/5/23/1547.
And I don't think a practical conclusion is made there.
Further, I e-mailed Greg K-H about when should we use devm_kmalloc().
On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> devm_kmalloc() is used to allocate memory for a driver dev. Comments
> above the definition and doc
> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> imply that allocated the memory is automatically freed on driver attach,
> no matter allocation fail or not. However, I examined the code, and
> there are many sites that devm_kfree() is used to free devm_kmalloc().
> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> So I am totally confused about this issue. Can anybody give me some
> guidance? When should we use devm_kfree()?
He replied: If you "know" you need to free the memory now,
call devm_kfree(). If you want to wait for it to be cleaned up latter,
like normal, then do not call it.
So could please look in to this issue?
Thanks
Gen
^ permalink raw reply
* Re: [PATCH 1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354
From: Kalle Valo @ 2019-05-28 12:18 UTC (permalink / raw)
To: Douglas Anderson
Cc: Ulf Hansson, Adrian Hunter, Arend van Spriel, linux-rockchip,
Double Lo, briannorris, Madhan Mohan R, mka, Wright Feng,
Chi-Hsien Lin, Douglas Anderson, brcm80211-dev-list.pdl,
Franky Lin, netdev, linux-wireless, linux-kernel, Madhan Mohan R,
Hante Meuleman, Naveen Gupta, brcm80211-dev-list, YueHaibing,
David S. Miller
In-Reply-To: <20190517225420.176893-2-dianders@chromium.org>
Douglas Anderson <dianders@chromium.org> wrote:
> In commit 29f6589140a1 ("brcmfmac: disable command decode in
> sdio_aos") we disabled something called "command decode in sdio_aos"
> for a whole bunch of Broadcom SDIO WiFi parts.
>
> After that patch landed I find that my kernel log on
> rk3288-veyron-minnie and rk3288-veyron-speedy is filled with:
> brcmfmac: brcmf_sdio_bus_sleep: error while changing bus sleep state -110
>
> This seems to happen every time the Broadcom WiFi transitions out of
> sleep mode. Reverting the part of the commit that affects the WiFi on
> my boards fixes the problem for me, so that's what this patch does.
>
> Note that, in general, the justification in the original commit seemed
> a little weak. It looked like someone was testing on a SD card
> controller that would sometimes die if there were CRC errors on the
> bus. This used to happen back in early days of dw_mmc (the controller
> on my boards), but we fixed it. Disabling a feature on all boards
> just because one SD card controller is broken seems bad. ...so
> instead of just this patch possibly the right thing to do is to fully
> revert the original commit.
>
> Fixes: 29f6589140a1 ("brcmfmac: disable command decode in sdio_aos")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
I don't see patch 2 in patchwork and I assume discussion continues.
Please resend if/when I need to apply something.
2 patches set to Changes Requested.
10948785 [1/3] brcmfmac: re-enable command decode in sdio_aos for BRCM 4354
10948777 [3/3] brcmfmac: sdio: Disable auto-tuning around commands expected to fail
--
https://patchwork.kernel.org/patch/10948785/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* [PATCH] nl80211: fill all policy .type entries
From: Johannes Berg @ 2019-05-28 12:19 UTC (permalink / raw)
To: linux-wireless; +Cc: j, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
For old commands, it's fine to have .type = NLA_UNSPEC and it
behaves the same as NLA_MIN_LEN. However, for new commands with
strict validation this is no longer true, and for policy export
to userspace these are also ignored.
Fix up the remaining ones that don't have a type.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 78 ++++++++++++++++++++++++++++++------------
1 file changed, 57 insertions(+), 21 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index c391b560d986..4892f307f51d 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -304,8 +304,11 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_IFINDEX] = { .type = NLA_U32 },
[NL80211_ATTR_IFNAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ-1 },
- [NL80211_ATTR_MAC] = { .len = ETH_ALEN },
- [NL80211_ATTR_PREV_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MAC] = { .type = NLA_EXACT_LEN_WARN, .len = ETH_ALEN },
+ [NL80211_ATTR_PREV_BSSID] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_KEY] = { .type = NLA_NESTED, },
[NL80211_ATTR_KEY_DATA] = { .type = NLA_BINARY,
@@ -356,7 +359,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MESH_CONFIG] = { .type = NLA_NESTED },
[NL80211_ATTR_SUPPORT_MESH_AUTH] = { .type = NLA_FLAG },
- [NL80211_ATTR_HT_CAPABILITY] = { .len = NL80211_HT_CAPABILITY_LEN },
+ [NL80211_ATTR_HT_CAPABILITY] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = NL80211_HT_CAPABILITY_LEN
+ },
[NL80211_ATTR_MGMT_SUBTYPE] = { .type = NLA_U8 },
[NL80211_ATTR_IE] = NLA_POLICY_VALIDATE_FN(NLA_BINARY,
@@ -386,7 +392,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_WPA_VERSIONS] = { .type = NLA_U32 },
[NL80211_ATTR_PID] = { .type = NLA_U32 },
[NL80211_ATTR_4ADDR] = { .type = NLA_U8 },
- [NL80211_ATTR_PMKID] = { .len = WLAN_PMKID_LEN },
+ [NL80211_ATTR_PMKID] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = WLAN_PMKID_LEN
+ },
[NL80211_ATTR_DURATION] = { .type = NLA_U32 },
[NL80211_ATTR_COOKIE] = { .type = NLA_U64 },
[NL80211_ATTR_TX_RATES] = { .type = NLA_NESTED },
@@ -448,7 +457,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_WDEV] = { .type = NLA_U64 },
[NL80211_ATTR_USER_REG_HINT_TYPE] = { .type = NLA_U32 },
[NL80211_ATTR_AUTH_DATA] = { .type = NLA_BINARY, },
- [NL80211_ATTR_VHT_CAPABILITY] = { .len = NL80211_VHT_CAPABILITY_LEN },
+ [NL80211_ATTR_VHT_CAPABILITY] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = NL80211_VHT_CAPABILITY_LEN
+ },
[NL80211_ATTR_SCAN_FLAGS] = { .type = NLA_U32 },
[NL80211_ATTR_P2P_CTWINDOW] = NLA_POLICY_MAX(NLA_U8, 127),
[NL80211_ATTR_P2P_OPPPS] = NLA_POLICY_MAX(NLA_U8, 1),
@@ -484,7 +496,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_VENDOR_DATA] = { .type = NLA_BINARY },
[NL80211_ATTR_QOS_MAP] = { .type = NLA_BINARY,
.len = IEEE80211_QOS_MAP_LEN_MAX },
- [NL80211_ATTR_MAC_HINT] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MAC_HINT] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_WIPHY_FREQ_HINT] = { .type = NLA_U32 },
[NL80211_ATTR_TDLS_PEER_CAPABILITY] = { .type = NLA_U32 },
[NL80211_ATTR_SOCKET_OWNER] = { .type = NLA_FLAG },
@@ -495,7 +510,10 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
NLA_POLICY_MAX(NLA_U8, IEEE80211_NUM_UPS - 1),
[NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
[NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
- [NL80211_ATTR_MAC_MASK] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MAC_MASK] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_WIPHY_SELF_MANAGED_REG] = { .type = NLA_FLAG },
[NL80211_ATTR_NETNS_FD] = { .type = NLA_U32 },
[NL80211_ATTR_SCHED_SCAN_DELAY] = { .type = NLA_U32 },
@@ -507,15 +525,21 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_MU_MIMO_GROUP_DATA] = {
.len = VHT_MUMIMO_GROUPS_DATA_LEN
},
- [NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = { .len = ETH_ALEN },
+ [NL80211_ATTR_MU_MIMO_FOLLOW_MAC_ADDR] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_ATTR_NAN_MASTER_PREF] = NLA_POLICY_MIN(NLA_U8, 1),
[NL80211_ATTR_BANDS] = { .type = NLA_U32 },
[NL80211_ATTR_NAN_FUNC] = { .type = NLA_NESTED },
[NL80211_ATTR_FILS_KEK] = { .type = NLA_BINARY,
.len = FILS_MAX_KEK_LEN },
- [NL80211_ATTR_FILS_NONCES] = { .len = 2 * FILS_NONCE_LEN },
+ [NL80211_ATTR_FILS_NONCES] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = 2 * FILS_NONCE_LEN
+ },
[NL80211_ATTR_MULTICAST_TO_UNICAST_ENABLED] = { .type = NLA_FLAG, },
- [NL80211_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_ATTR_BSSID] = { .type = NLA_EXACT_LEN_WARN, .len = ETH_ALEN },
[NL80211_ATTR_SCHED_SCAN_RELATIVE_RSSI] = { .type = NLA_S8 },
[NL80211_ATTR_SCHED_SCAN_RSSI_ADJUST] = {
.len = sizeof(struct nl80211_bss_select_rssi_adjust)
@@ -528,7 +552,7 @@ const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_FILS_ERP_NEXT_SEQ_NUM] = { .type = NLA_U16 },
[NL80211_ATTR_FILS_ERP_RRK] = { .type = NLA_BINARY,
.len = FILS_ERP_MAX_RRK_LEN },
- [NL80211_ATTR_FILS_CACHE_ID] = { .len = 2 },
+ [NL80211_ATTR_FILS_CACHE_ID] = { .type = NLA_EXACT_LEN_WARN, .len = 2 },
[NL80211_ATTR_PMK] = { .type = NLA_BINARY, .len = PMK_MAX_LEN },
[NL80211_ATTR_SCHED_SCAN_MULTI] = { .type = NLA_FLAG },
[NL80211_ATTR_EXTERNAL_AUTH_SUPPORT] = { .type = NLA_FLAG },
@@ -589,10 +613,13 @@ static const struct nla_policy
nl80211_wowlan_tcp_policy[NUM_NL80211_WOWLAN_TCP] = {
[NL80211_WOWLAN_TCP_SRC_IPV4] = { .type = NLA_U32 },
[NL80211_WOWLAN_TCP_DST_IPV4] = { .type = NLA_U32 },
- [NL80211_WOWLAN_TCP_DST_MAC] = { .len = ETH_ALEN },
+ [NL80211_WOWLAN_TCP_DST_MAC] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_WOWLAN_TCP_SRC_PORT] = { .type = NLA_U16 },
[NL80211_WOWLAN_TCP_DST_PORT] = { .type = NLA_U16 },
- [NL80211_WOWLAN_TCP_DATA_PAYLOAD] = { .len = 1 },
+ [NL80211_WOWLAN_TCP_DATA_PAYLOAD] = { .type = NLA_MIN_LEN, .len = 1 },
[NL80211_WOWLAN_TCP_DATA_PAYLOAD_SEQ] = {
.len = sizeof(struct nl80211_wowlan_tcp_data_seq)
},
@@ -600,8 +627,8 @@ nl80211_wowlan_tcp_policy[NUM_NL80211_WOWLAN_TCP] = {
.len = sizeof(struct nl80211_wowlan_tcp_data_token)
},
[NL80211_WOWLAN_TCP_DATA_INTERVAL] = { .type = NLA_U32 },
- [NL80211_WOWLAN_TCP_WAKE_PAYLOAD] = { .len = 1 },
- [NL80211_WOWLAN_TCP_WAKE_MASK] = { .len = 1 },
+ [NL80211_WOWLAN_TCP_WAKE_PAYLOAD] = { .type = NLA_MIN_LEN, .len = 1 },
+ [NL80211_WOWLAN_TCP_WAKE_MASK] = { .type = NLA_MIN_LEN, .len = 1 },
};
#endif /* CONFIG_PM */
@@ -619,9 +646,12 @@ nl80211_coalesce_policy[NUM_NL80211_ATTR_COALESCE_RULE] = {
/* policy for GTK rekey offload attributes */
static const struct nla_policy
nl80211_rekey_policy[NUM_NL80211_REKEY_DATA] = {
- [NL80211_REKEY_DATA_KEK] = { .len = NL80211_KEK_LEN },
- [NL80211_REKEY_DATA_KCK] = { .len = NL80211_KCK_LEN },
- [NL80211_REKEY_DATA_REPLAY_CTR] = { .len = NL80211_REPLAY_CTR_LEN },
+ [NL80211_REKEY_DATA_KEK] = { .type = NLA_EXACT_LEN_WARN, .len = NL80211_KEK_LEN },
+ [NL80211_REKEY_DATA_KCK] = { .type = NLA_EXACT_LEN_WARN, .len = NL80211_KCK_LEN },
+ [NL80211_REKEY_DATA_REPLAY_CTR] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = NL80211_REPLAY_CTR_LEN
+ },
};
static const struct nla_policy
@@ -635,7 +665,10 @@ static const struct nla_policy
nl80211_match_policy[NL80211_SCHED_SCAN_MATCH_ATTR_MAX + 1] = {
[NL80211_SCHED_SCAN_MATCH_ATTR_SSID] = { .type = NLA_BINARY,
.len = IEEE80211_MAX_SSID_LEN },
- [NL80211_SCHED_SCAN_MATCH_ATTR_BSSID] = { .len = ETH_ALEN },
+ [NL80211_SCHED_SCAN_MATCH_ATTR_BSSID] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_SCHED_SCAN_MATCH_ATTR_RSSI] = { .type = NLA_U32 },
[NL80211_SCHED_SCAN_MATCH_PER_BAND_RSSI] =
NLA_POLICY_NESTED(nl80211_match_band_rssi_policy),
@@ -667,7 +700,10 @@ nl80211_nan_func_policy[NL80211_NAN_FUNC_ATTR_MAX + 1] = {
[NL80211_NAN_FUNC_SUBSCRIBE_ACTIVE] = { .type = NLA_FLAG },
[NL80211_NAN_FUNC_FOLLOW_UP_ID] = { .type = NLA_U8 },
[NL80211_NAN_FUNC_FOLLOW_UP_REQ_ID] = { .type = NLA_U8 },
- [NL80211_NAN_FUNC_FOLLOW_UP_DEST] = { .len = ETH_ALEN },
+ [NL80211_NAN_FUNC_FOLLOW_UP_DEST] = {
+ .type = NLA_EXACT_LEN_WARN,
+ .len = ETH_ALEN
+ },
[NL80211_NAN_FUNC_CLOSE_RANGE] = { .type = NLA_FLAG },
[NL80211_NAN_FUNC_TTL] = { .type = NLA_U32 },
[NL80211_NAN_FUNC_SERVICE_INFO] = { .type = NLA_BINARY,
@@ -4057,7 +4093,7 @@ static const struct nla_policy nl80211_txattr_policy[NL80211_TXRATE_MAX + 1] = {
.len = NL80211_MAX_SUPP_RATES },
[NL80211_TXRATE_HT] = { .type = NLA_BINARY,
.len = NL80211_MAX_SUPP_HT_RATES },
- [NL80211_TXRATE_VHT] = { .len = sizeof(struct nl80211_txrate_vht)},
+ [NL80211_TXRATE_VHT] = { .type = NLA_EXACT_LEN_WARN, .len = sizeof(struct nl80211_txrate_vht)},
[NL80211_TXRATE_GI] = { .type = NLA_U8 },
};
--
2.17.2
^ permalink raw reply related
* Re: [PATCH 1/7] brcm80211: switch common header files to using SPDX license identifier
From: Kalle Valo @ 2019-05-28 12:24 UTC (permalink / raw)
To: Arend van Spriel; +Cc: linux-wireless, Arend van Spriel
In-Reply-To: <1558008251-13692-2-git-send-email-arend.vanspriel@broadcom.com>
Arend van Spriel <arend.vanspriel@broadcom.com> wrote:
> With ISC license text in place under the LICENSES folder switch
> to using the SPDX license identifier to refer to the ISC license.
>
> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com>
> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com>
> Reviewed-by: Franky Lin <franky.lin@broadcom.com>
> Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
6 patches applied to wireless-drivers-next.git, thanks.
7e5677de8e0c brcm80211: switch common header files to using SPDX license identifier
afe06f822035 brcmutil: switch source files to using SPDX license identifier
f843863d6d10 brcmsmac: switch phy source files to using SPDX license identifier
daeccac2d5e7 brcmfmac: switch source files to using SPDX license identifier
9ff8614a3dbe brcmfmac: use separate Kconfig file for brcmfmac
885a93cf3110 brcm80211: select WANT_DEV_COREDUMP conditionally for brcmfmac
--
https://patchwork.kernel.org/patch/10946467/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH v2] brcmfmac: fix typos in code comments
From: Kalle Valo @ 2019-05-28 12:25 UTC (permalink / raw)
To: Weitao Hou
Cc: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
wright.feng, davem, houweitaoo, rafal, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel
In-Reply-To: <20190520122825.981-1-houweitaoo@gmail.com>
Weitao Hou <houweitaoo@gmail.com> wrote:
> fix lengh to length
>
> Signed-off-by: Weitao Hou <houweitaoo@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
b07e1ae2ce53 brcmfmac: fix typos in code comments
--
https://patchwork.kernel.org/patch/10950995/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] brcmfmac: use strlcpy() instead of strcpy()
From: Kalle Valo @ 2019-05-28 12:25 UTC (permalink / raw)
To: neojou
Cc: arend.vanspriel, franky.lin, hante.meuleman, chi-hsien.lin,
wright.feng, davem, rafal, hdegoedg, p.figiel, linux-wireless,
brcm80211-dev-list.pdl, brcm80211-dev-list, netdev, linux-kernel,
Neo Jou
In-Reply-To: <1558429940-8709-1-git-send-email-neojou@gmail.com>
neojou@gmail.com wrote:
> From: Neo Jou <neojou@gmail.com>
>
> The function strcpy() is inherently not safe. Though the function
> works without problems here, it would be better to use other safer
> function, e.g. strlcpy(), to replace strcpy() still.
>
> Signed-off-by: Neo Jou <neojou@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
bbfab331e3ab brcmfmac: use strlcpy() instead of strcpy()
--
https://patchwork.kernel.org/patch/10953203/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] p54: fix crash during initialization
From: Kalle Valo @ 2019-05-28 12:26 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, David S . Miller
In-Reply-To: <20190518200548.7662-1-chunkeey@gmail.com>
Christian Lamparter <chunkeey@gmail.com> wrote:
> This patch fixes a crash that got introduced when the
> mentioned patch replaced the direct list_head access
> with skb_peek_tail(). When the device is starting up,
> there are no entries in the queue, so previously to
> "Use skb_peek_tail() instead..." the target_skb would
> end up as the tail and head pointer which then could
> be used by __skb_queue_after to fill the empty queue.
>
> With skb_peek_tail() in its place will instead just
> return NULL which then causes a crash in the
> __skb_queue_after().
>
> | BUG: unable to handle kernel NULL pointer dereference at 000000
> | #PF error: [normal kernel read fault]
> | PGD 0 P4D 0
> | Oops: 0000 [#1] SMP PTI
> | CPU: 0 PID: 12 Comm: kworker/0:1 Tainted: GO 5.1.0-rc7-wt+ #218
> | Hardware name: MSI MS-7816/Z87-G43 (MS-7816), BIOS V1.11 05/09/2015
> | Workqueue: events request_firmware_work_func
> | RIP: 0010:p54_tx_pending+0x10f/0x1b0 [p54common]
> | Code: 78 06 80 78 28 00 74 6d <48> 8b 07 49 89 7c 24 08 49 89 04 24 4
> | RSP: 0018:ffffa81c81927d90 EFLAGS: 00010086
> | RAX: ffff9bbaaf131048 RBX: 0000000000020670 RCX: 0000000000020264
> | RDX: ffff9bbaa976d660 RSI: 0000000000000202 RDI: 0000000000000000
> | RBP: ffff9bbaa976d620 R08: 00000000000006c0 R09: ffff9bbaa976d660
> | R10: 0000000000000000 R11: ffffe8480dbc5900 R12: ffff9bbb45e87700
> | R13: ffff9bbaa976d648 R14: ffff9bbaa976d674 R15: ffff9bbaaf131048
> | FS: 0000000000000000(0000) GS:ffff9bbb5ec00000(0000) knlGS:00000
> | CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> | CR2: 0000000000000000 CR3: 00000003695fc003 CR4: 00000000001606f0
> | Call Trace:
> | p54_download_eeprom+0xbe/0x120 [p54common]
> | p54_read_eeprom+0x7f/0xc0 [p54common]
> | p54u_load_firmware_cb+0xe0/0x160 [p54usb]
> | request_firmware_work_func+0x42/0x80
> | process_one_work+0x1f5/0x3f0
> | worker_thread+0x28/0x3c0
>
> Cc: stable@vger.kernel.org
> Fixes: e3554197fc8f ("p54: Use skb_peek_tail() instead of direct head pointer accesses.")
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
1645ab931998 p54: fix crash during initialization
--
https://patchwork.kernel.org/patch/10949055/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] p54: Support boottime in scan results
From: Kalle Valo @ 2019-05-28 12:27 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless
In-Reply-To: <20190524213308.18575-1-chunkeey@gmail.com>
Christian Lamparter <chunkeey@gmail.com> wrote:
> This patch fixes an issue with wpa_supplicant dropping all scan
> results because their where considered to be "too old" (e.g.:
> "skip - scan result not recent enough (121056.086325 seconds too old)")
> which is very weird because this looks like that the scan results have
> been received before a scan started. This is due to the inaccuracy of
> the default timing mechanism for calculating the BSS entry age.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
c11c75ec784e p54: Support boottime in scan results
--
https://patchwork.kernel.org/patch/10960463/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/7] rt2x00: allow to specify watchdog interval
From: Kalle Valo @ 2019-05-28 12:28 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-wireless, Tomislav Požega, Daniel Golle, Felix Fietkau,
Mathias Kresin
In-Reply-To: <1556788021-6531-2-git-send-email-sgruszka@redhat.com>
Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Allow subdriver to change watchdog interval by intialize
> link->watchdog_interval value before rt2x00link_register().
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Failed to apply:
fatal: sha1 information is lacking or useless (drivers/net/wireless/ralink/rt2x00/rt2800lib.c).
error: could not build fake ancestor
Applying: rt2800: initial watchdog implementation
Patch failed at 0001 rt2800: initial watchdog implementation
The copy of the patch that failed is found in: .git/rebase-apply/patch
7 patches set to Changes Requested.
10926395 [1/7] rt2x00: allow to specify watchdog interval
10926397 [2/7] rt2800: add helpers for reading dma done index
10926401 [3/7] rt2800: initial watchdog implementation
10926403 [4/7] rt2800: add pre_reset_hw callback
10926405 [5/7] rt2800: do not nullify initialization vector data
10926407 [6/7] rt2x00: add restart hw
10926409 [7/7] rt2800: do not enable watchdog by default
--
https://patchwork.kernel.org/patch/10926395/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Kalle Valo @ 2019-05-28 12:33 UTC (permalink / raw)
To: Gen Zhang; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190528121452.GA23464@zhanggen-UX430UQ>
Gen Zhang <blackgod016574@gmail.com> writes:
> On Tue, May 28, 2019 at 11:39:22AM +0000, Kalle Valo wrote:
>> Gen Zhang <blackgod016574@gmail.com> wrote:
>>
>> > In wl1271_probe(), 'glue->core' is allocated by platform_device_alloc(),
>> > when this allocation fails, ENOMEM is returned. However, 'pdev_data'
>> > and 'glue' are allocated by devm_kzalloc() before 'glue->core'. When
>> > platform_device_alloc() returns NULL, we should also free 'pdev_data'
>> > and 'glue' before wl1271_probe() ends to prevent leaking memory.
>> >
>> > Similarly, we shoulf free 'pdev_data' when 'glue' is NULL. And we should
>> > free 'pdev_data' and 'glue' when 'glue->reg' is error and when 'ret' is
>> > error.
>> >
>> > Further, we should free 'glue->core', 'pdev_data' and 'glue' when this
>> > function normally ends to prevent leaking memory.
>> >
>> > Signed-off-by: Gen Zhang <blackgod016574@gmail.com>
>>
>> Same questions as with similar SDIO patch:
>>
>> https://patchwork.kernel.org/patch/10959049/
>>
>> Patch set to Changes Requested.
>>
>> --
>> https://patchwork.kernel.org/patch/10959053/
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
>>
> Thanks for your reply, Kalle. I had debate with Jon about this patch.
> You could kindly refer to lkml: https://lkml.org/lkml/2019/5/23/1547.
> And I don't think a practical conclusion is made there.
Yeah, I don't see how that thread proves that these patches are correct.
> Further, I e-mailed Greg K-H about when should we use devm_kmalloc().
>
> On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
>> devm_kmalloc() is used to allocate memory for a driver dev. Comments
>> above the definition and doc
>> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
>> imply that allocated the memory is automatically freed on driver attach,
>> no matter allocation fail or not. However, I examined the code, and
>> there are many sites that devm_kfree() is used to free devm_kmalloc().
>> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
>> So I am totally confused about this issue. Can anybody give me some
>> guidance? When should we use devm_kfree()?
> He replied: If you "know" you need to free the memory now,
> call devm_kfree(). If you want to wait for it to be cleaned up latter,
> like normal, then do not call it.
>
> So could please look in to this issue?
Sorry, no time to investigate this in detail. If you think the patches
are correct you can resend them and get someone familiar with the driver
to provide Reviewed-by, then I will apply them.
--
Kalle Valo
^ permalink raw reply
* Re: [wireless-regdb] [PATCH v3] wireless-regdb: Update regulatory rules for South Korea
From: Seth Forshee @ 2019-05-28 12:32 UTC (permalink / raw)
To: Peter Oh; +Cc: wireless-regdb@lists.infradead.org,
linux-wireless@vger.kernel.org
In-Reply-To: <1557948501-12579-1-git-send-email-peter.oh@bowerswilkins.com>
On Wed, May 15, 2019 at 07:28:29PM +0000, Peter Oh wrote:
> From: Peter Oh <peter.oh@bowerswilkins.com>
>
> Update power limit as documented in:
> http://www.law.go.kr/%ED%96%89%EC%A0%95%EA%B7%9C%EC%B9%99/
> %EC%8B%A0%EA%B3%A0%ED%95%98%EC%A7%80%EC%95%84%EB%8B%88%ED
> %95%98%EA%B3%A0%EA%B0%9C%EC%84%A4%ED%95%A0%EC%88%98%EC%9E
> %88%EB%8A%94%EB%AC%B4%EC%84%A0%EA%B5%AD%EC%9A%A9%EB%AC%B4
> %EC%84%A0%EA%B8%B0%EA%B8%B0/(2018-89,20181227)
> which revised on December 27, 2018.
>
> Signed-off-by: Peter Oh <peter.oh@bowerswilkins.com>
Applied, thanks!
^ permalink raw reply
* Re: [PATCH 1/4] rtlwifi: 8192de: Reduce indentation and fix coding style
From: Kalle Valo @ 2019-05-28 12:34 UTC (permalink / raw)
To: pkshih; +Cc: Larry.Finger, linux-wireless, colin.king
In-Reply-To: <20190510080333.3789-2-pkshih@realtek.com>
<pkshih@realtek.com> wrote:
> From: Ping-Ke Shih <pkshih@realtek.com>
>
> This commit doesn't change logic at all. Since indentation is lower, the
> wrapped statements can be put in single line that will become more
> readable.
>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
4 patches applied to wireless-drivers-next.git, thanks.
3bde4ed3da38 rtlwifi: 8192de: Reduce indentation and fix coding style
8a7db8b66b40 rtlwifi: 8192de: make tables to be 'static const'
0e7d38132bf8 rtlwifi: 8192de: Fix used uninitialized variables in power tracking
80429a86e3d9 rtlwifi: 8192de: use le32 to access cckswing tables
--
https://patchwork.kernel.org/patch/10938337/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/4] rtlwifi: rtl8821ae: Remove unused GET_XXX and SET_XXX descriptor macros
From: Kalle Valo @ 2019-05-28 12:36 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, pkshih, Larry Finger
In-Reply-To: <20190520172359.9993-2-Larry.Finger@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> wrote:
> As the first step in converting from macros that get/set information
> in the RX and TX descriptors, unused macros are being removed.
>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
4 patches applied to wireless-drivers-next.git, thanks.
7ffe556230b8 rtlwifi: rtl8821ae: Remove unused GET_XXX and SET_XXX descriptor macros
f5678bfe1cdc rtlwifi: rtl8821ae: Replace local bit manipulation macros
bd421dab7515 rtlwifi: rtl8821ae: Convert macros that set descriptor
f7fbb03f2169 rtlwifi: rtl8821ae: Convert inline routines to little-endian words
--
https://patchwork.kernel.org/patch/10951971/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: rtl8821ae: Remove set but not used variables 'cur_txokcnt' and 'b_last_is_cur_rdl_state'
From: Kalle Valo @ 2019-05-28 12:37 UTC (permalink / raw)
To: YueHaibing
Cc: pkshih, davem, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190525144332.17268-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c: In function rtl8821ae_dm_check_rssi_monitor:
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c:658:6: warning: variable cur_txokcnt set but not used [-Wunused-but-set-variable]
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c: In function rtl8821ae_dm_check_edca_turbo:
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/dm.c:2657:7: warning: variable b_last_is_cur_rdl_state set but not used [-Wunused-but-set-variable]
>
> They are never used so can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
3e42a66dfd15 rtlwifi: rtl8821ae: Remove set but not used variables 'cur_txokcnt' and 'b_last_is_cur_rdl_state'
--
https://patchwork.kernel.org/patch/10960841/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: Remove set but not used variable 'len' and 'asso_type_v2'
From: Kalle Valo @ 2019-05-28 12:37 UTC (permalink / raw)
To: YueHaibing
Cc: pkshih, davem, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190525144634.10696-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c: In function rtl_btc_btmpinfo_notify:
> drivers/net/wireless/realtek/rtlwifi/btcoexist/rtl_btc.c:319:17: warning: variable len set but not used [-Wunused-but-set-variable]
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c: In function exhalbtc_connect_notify:
> drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:1581:16: warning: variable asso_type_v2 set but not used [-Wunused-but-set-variable]
>
> 'len' is never used since commit 6aad6075ccd5 ("rtlwifi:
> Add BT_MP_INFO to c2h handler.") so can be removed.
>
> 'asso_type_v2' is not used since introduction in
> commit 0843e98a3b9a ("rtlwifi: btcoex: add assoc
> type v2 to connection notify")
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
d477a4856aec rtlwifi: btcoex: Remove set but not used variable 'len' and 'asso_type_v2'
--
https://patchwork.kernel.org/patch/10960843/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] rtlwifi: btcoex: remove unused function exhalbtc_stack_operation_notify
From: Kalle Valo @ 2019-05-28 12:38 UTC (permalink / raw)
To: YueHaibing
Cc: pkshih, davem, linux-kernel, netdev, linux-wireless, YueHaibing
In-Reply-To: <20190525144844.16976-1-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> wrote:
> There is no callers in tree, so can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Patch applied to wireless-drivers-next.git, thanks.
dfbe36197dbc rtlwifi: btcoex: remove unused function exhalbtc_stack_operation_notify
--
https://patchwork.kernel.org/patch/10960845/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] wlcore: spi: Fix a memory leaking bug in wl1271_probe()
From: Gen Zhang @ 2019-05-28 12:38 UTC (permalink / raw)
To: Kalle Valo; +Cc: davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <87tvde4v3u.fsf@kamboji.qca.qualcomm.com>
On Tue, May 28, 2019 at 03:33:09PM +0300, Kalle Valo wrote:
> Yeah, I don't see how that thread proves that these patches are correct.
>
Sure, I didn't mean that we came to an agreement that these patches are
correct.
> > Further, I e-mailed Greg K-H about when should we use devm_kmalloc().
> >
> > On Tue, May 28, 2019 at 08:32:57AM +0800, Gen Zhang wrote:
> >> devm_kmalloc() is used to allocate memory for a driver dev. Comments
> >> above the definition and doc
> >> (https://www.kernel.org/doc/Documentation/driver-model/devres.txt) all
> >> imply that allocated the memory is automatically freed on driver attach,
> >> no matter allocation fail or not. However, I examined the code, and
> >> there are many sites that devm_kfree() is used to free devm_kmalloc().
> >> e.g. hisi_sas_debugfs_init() in drivers/scsi/hisi_sas/hisi_sas_main.c.
> >> So I am totally confused about this issue. Can anybody give me some
> >> guidance? When should we use devm_kfree()?
> > He replied: If you "know" you need to free the memory now,
> > call devm_kfree(). If you want to wait for it to be cleaned up latter,
> > like normal, then do not call it.
> >
> > So could please look in to this issue?
>
> Sorry, no time to investigate this in detail. If you think the patches
> are correct you can resend them and get someone familiar with the driver
> to provide Reviewed-by, then I will apply them.
>
> --
> Kalle Valo
Ok, thanks for your time. I will follow your suggestions.
Thanks
Gen
^ permalink raw reply
* Re: [PATCH] libertas: fix spelling mistake "Donwloading" -> "Downloading"
From: Kalle Valo @ 2019-05-28 12:43 UTC (permalink / raw)
To: Colin King
Cc: David S . Miller, libertas-dev, linux-wireless, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <20190514211406.6353-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is are two spelling mistakes in lbtf_deb_usb2 messages, fix these.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> Reviewed-by: Mukesh Ojha <mojha@codeaurora.org>
Patch applied to wireless-drivers-next.git, thanks.
aeffda6b10f8 libertas: fix spelling mistake "Donwloading" -> "Downloading"
--
https://patchwork.kernel.org/patch/10943765/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] b43: Avoid possible double calls to b43_one_core_detach()
From: Kalle Valo @ 2019-05-28 12:43 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: davem, colin.king, yuehaibing, linux-wireless, b43-dev, netdev,
linux-kernel, Jia-Ju Bai
In-Reply-To: <20190504091000.18665-1-baijiaju1990@gmail.com>
Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> In b43_request_firmware(), when ieee80211_register_hw() fails,
> b43_one_core_detach() is called. In b43_bcma_remove() and
> b43_ssb_remove(), b43_one_core_detach() is called again. In this case,
> null-pointer dereferences and double-free problems can occur when
> the driver is removed.
>
> To fix this bug, the call to b43_one_core_detach() in
> b43_request_firmware() is deleted.
>
> This bug is found by a runtime fuzzing tool named FIZZER written by us.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Patch applied to wireless-drivers-next.git, thanks.
ec2e93cf1910 b43: Avoid possible double calls to b43_one_core_detach()
--
https://patchwork.kernel.org/patch/10929623/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath10k: remove mmc_hw_reset while hif power down
From: Ulf Hansson @ 2019-05-28 12:45 UTC (permalink / raw)
To: Kalle Valo, Wen Gong, Grant Grundler
Cc: Wen Gong, linux-wireless@vger.kernel.org,
ath10k@lists.infradead.org
In-Reply-To: <87pnour4jg.fsf@codeaurora.org>
On Tue, 7 May 2019 at 11:35, Kalle Valo <kvalo@codeaurora.org> wrote:
>
> + Ulf to give comments from SDIO point of view
Apologize for the delay, it's been a busy period.
>
> Wen Gong <wgong@qti.qualcomm.com> writes:
>
> >> -----Original Message-----
> >> From: ath10k <ath10k-bounces@lists.infradead.org> On Behalf Of Grant
> >> Grundler
> >> Sent: Saturday, May 4, 2019 2:01 AM
> >> To: Wen Gong <wgong@codeaurora.org>
> >> Cc: linux-wireless@vger.kernel.org; ath10k@lists.infradead.org
> >> Subject: [EXT] Re: [PATCH] ath10k: remove mmc_hw_reset while hif power
> >> down
> >>
> >> [repeating comments I made in the gerrit review for Chrome OS :
> >> https://chromium-
> >> review.googlesource.com/c/chromiumos/third_party/kernel/+/1585667
> >> ]
> >>
> >> On Sat, Apr 27, 2019 at 7:17 PM Wen Gong <wgong@codeaurora.org> wrote:
> >> >
> >> > For sdio 3.0 chip, the clock will drop from 200M Hz to 50M Hz after load
> >> > ath10k driver, it is because mmc_hw_reset will reset the sdio's power,
> >> > then mmc will consider it as sdio 2.0 and drop the clock.
> >>
> >> Wen,
> >> 5468e784c0600551ca03263f5255a375c05f88e7 commit message gives
> >> reasons
> >> for adding the mmc_hw_reset() call. The commit message for removing
> >> gives different reason for removal. Both are good but second one is
> >> incomplete.
> >>
> >> The commit message for removal should ALSO explain why adding this
> >> call wasn't necessary in the first place OR move the call to a
> >> different code path.
> >>
> >> > Remove mmc_hw_reset will avoid the drop of clock.
> >>
> >> This commit message makes it clear the original patch introduced a new
> >> problem. But the original patch fixed a different problem and that
> >> this proposed change seems likely to re-introduce and the commit
> >> message should explain why that isn't true (or how the original was
> >> fixed differently)
> >
> > The mmc_hw_reset's effect depends on the hardware layout/configure
> > software's behavior, recently it will effect the clock of sdio for the
> > platform I used. And it will still work well without mmc_hw_reset for
> > the platform I Used currently. If sdio cannot work on other platform,
> > I think it can add flag in ath10k_hw_params_list for the platform to
> > call the mmc_hw_reset depends on the flag.
>
> I don't see how you can use ath10k_hw_params_list to separate SDIO
> controller functionality, I assume that's the real reason for difference
> of functionality? Maybe this is a bug on the SDIO controller?
Ideally mmc_hw_reset() should not make the SDIO card to become
re-initialized differently than it was originally.
However, as that is the case here, it sounds like that there is a bug
somewhere. Perhaps the boot-loader have done some pre-initialization,
which isn't covered in the second case, or maybe a bug in the mmc host
driver.
>
> Ulf, what do you think? Any suggestions? Full discussion here:
>
> https://patchwork.kernel.org/patch/10920563/
>
> --
> Kalle Valo
In the end, it seems like this needs a more detailed debug study, to
figure out what exactly happens during the re-initialization of the
SDIO card, rather than just papering over the problem by removing the
call to mmc_hw_reset() in the SDIO func driver. Hope this helps.
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
From: Larry Finger @ 2019-05-28 13:00 UTC (permalink / raw)
To: Kalle Valo, Jia-Ju Bai
Cc: pkshih, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <20190528115555.301E760F3C@smtp.codeaurora.org>
On 5/28/19 6:55 AM, Kalle Valo wrote:
> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>
>> *BUG 1:
>> In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails,
>> rtl_deinit_core() in the error handling code is executed.
>> rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses
>> rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been
>> initialized. Thus a null-pointer dereference occurs.
>> The reason is that rtlpriv->scan_list.list is initialized by
>> INIT_LIST_HEAD() in rtl_init_core(), which has not been called.
>>
>> To fix this bug, rtl_deinit_core() should not be called when
>> rtlpriv->cfg->ops->init_sw_vars() fails.
>>
>> *BUG 2:
>> In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in
>> this function fails, and rtlpriv->scan_list.list has not been
>> initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error
>> handling code of rtl_pci_probe() is executed. Finally, a null-pointer
>> dereference occurs due to the same reason of the above bug.
>>
>> To fix this bug, the initialization of lists in rtl_init_core() are
>> performed before the call to rtl_regd_init().
>>
>> These bugs are found by a runtime fuzzing tool named FIZZER written by
>> us.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>
> Ping & Larry, is this ok to take?
>
Kalle,
Not at the moment. In reviewing the code, I was unable to see how this situation
could develop, and his backtrace did not mention any rtlwifi code. For that
reason, I asked him to add printk stat4ements to show the last part of rtl_pci
that executed correctly. In
https://marc.info/?l=linux-wireless&m=155788322631134&w=2, he promised to do
that, but I have not seen the result.
Larry
^ permalink raw reply
* Re: [PATCH] rtlwifi: Fix null-pointer dereferences in error handling code of rtl_pci_probe()
From: Kalle Valo @ 2019-05-28 13:09 UTC (permalink / raw)
To: Larry Finger
Cc: Jia-Ju Bai, pkshih, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <2658b691-b992-b773-c6cf-85801adc479f@lwfinger.net>
Larry Finger <Larry.Finger@lwfinger.net> writes:
> On 5/28/19 6:55 AM, Kalle Valo wrote:
>> Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
>>
>>> *BUG 1:
>>> In rtl_pci_probe(), when rtlpriv->cfg->ops->init_sw_vars() fails,
>>> rtl_deinit_core() in the error handling code is executed.
>>> rtl_deinit_core() calls rtl_free_entries_from_scan_list(), which uses
>>> rtlpriv->scan_list.list in list_for_each_entry_safe(), but it has been
>>> initialized. Thus a null-pointer dereference occurs.
>>> The reason is that rtlpriv->scan_list.list is initialized by
>>> INIT_LIST_HEAD() in rtl_init_core(), which has not been called.
>>>
>>> To fix this bug, rtl_deinit_core() should not be called when
>>> rtlpriv->cfg->ops->init_sw_vars() fails.
>>>
>>> *BUG 2:
>>> In rtl_pci_probe(), rtl_init_core() can fail when rtl_regd_init() in
>>> this function fails, and rtlpriv->scan_list.list has not been
>>> initialized by INIT_LIST_HEAD(). Then, rtl_deinit_core() in the error
>>> handling code of rtl_pci_probe() is executed. Finally, a null-pointer
>>> dereference occurs due to the same reason of the above bug.
>>>
>>> To fix this bug, the initialization of lists in rtl_init_core() are
>>> performed before the call to rtl_regd_init().
>>>
>>> These bugs are found by a runtime fuzzing tool named FIZZER written by
>>> us.
>>>
>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>
>> Ping & Larry, is this ok to take?
>>
>
> Kalle,
>
> Not at the moment. In reviewing the code, I was unable to see how this
> situation could develop, and his backtrace did not mention any rtlwifi
> code. For that reason, I asked him to add printk stat4ements to show
> the last part of rtl_pci that executed correctly. In
> https://marc.info/?l=linux-wireless&m=155788322631134&w=2, he promised
> to do that, but I have not seen the result.
Ok, thanks. I'll then drop this, please resubmit once everything is
understood and the patch is ready to be applied.
--
Kalle Valo
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox