* Re: [PATCH 5.1] mt76x02: avoid status_list.lock and sta->rate_ctrl_lock dependency
From: Kalle Valo @ 2019-04-12 18:33 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: linux-wireless, Felix Fietkau, Lorenzo Bianconi
In-Reply-To: <20190405114256.6416-1-sgruszka@redhat.com>
Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> Move ieee80211_tx_status_ext() outside of status_list lock section
> in order to avoid locking dependency and possible deadlock reposed by
> LOCKDEP in below warning.
>
> Also do mt76_tx_status_lock() just before it's needed.
>
> [ 440.224832] WARNING: possible circular locking dependency detected
> [ 440.224833] 5.1.0-rc2+ #22 Not tainted
> [ 440.224834] ------------------------------------------------------
> [ 440.224835] kworker/u16:28/2362 is trying to acquire lock:
> [ 440.224836] 0000000089b8cacf (&(&q->lock)->rlock#2){+.-.}, at: mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.224842]
> but task is already holding lock:
> [ 440.224842] 000000002cfedc59 (&(&sta->lock)->rlock){+.-.}, at: ieee80211_stop_tx_ba_cb+0x32/0x1f0 [mac80211]
> [ 440.224863]
> which lock already depends on the new lock.
>
> [ 440.224863]
> the existing dependency chain (in reverse order) is:
> [ 440.224864]
> -> #3 (&(&sta->lock)->rlock){+.-.}:
> [ 440.224869] _raw_spin_lock_bh+0x34/0x40
> [ 440.224880] ieee80211_start_tx_ba_session+0xe4/0x3d0 [mac80211]
> [ 440.224894] minstrel_ht_get_rate+0x45c/0x510 [mac80211]
> [ 440.224906] rate_control_get_rate+0xc1/0x140 [mac80211]
> [ 440.224918] ieee80211_tx_h_rate_ctrl+0x195/0x3c0 [mac80211]
> [ 440.224930] ieee80211_xmit_fast+0x26d/0xa50 [mac80211]
> [ 440.224942] __ieee80211_subif_start_xmit+0xfc/0x310 [mac80211]
> [ 440.224954] ieee80211_subif_start_xmit+0x38/0x390 [mac80211]
> [ 440.224956] dev_hard_start_xmit+0xb8/0x300
> [ 440.224957] __dev_queue_xmit+0x7d4/0xbb0
> [ 440.224968] ip6_finish_output2+0x246/0x860 [ipv6]
> [ 440.224978] mld_sendpack+0x1bd/0x360 [ipv6]
> [ 440.224987] mld_ifc_timer_expire+0x1a4/0x2f0 [ipv6]
> [ 440.224989] call_timer_fn+0x89/0x2a0
> [ 440.224990] run_timer_softirq+0x1bd/0x4d0
> [ 440.224992] __do_softirq+0xdb/0x47c
> [ 440.224994] irq_exit+0xfa/0x100
> [ 440.224996] smp_apic_timer_interrupt+0x9a/0x220
> [ 440.224997] apic_timer_interrupt+0xf/0x20
> [ 440.224999] cpuidle_enter_state+0xc1/0x470
> [ 440.225000] do_idle+0x21a/0x260
> [ 440.225001] cpu_startup_entry+0x19/0x20
> [ 440.225004] start_secondary+0x135/0x170
> [ 440.225006] secondary_startup_64+0xa4/0xb0
> [ 440.225007]
> -> #2 (&(&sta->rate_ctrl_lock)->rlock){+.-.}:
> [ 440.225009] _raw_spin_lock_bh+0x34/0x40
> [ 440.225022] rate_control_tx_status+0x4f/0xb0 [mac80211]
> [ 440.225031] ieee80211_tx_status_ext+0x142/0x1a0 [mac80211]
> [ 440.225035] mt76x02_send_tx_status+0x2e4/0x340 [mt76x02_lib]
> [ 440.225037] mt76x02_tx_status_data+0x31/0x40 [mt76x02_lib]
> [ 440.225040] mt76u_tx_status_data+0x51/0xa0 [mt76_usb]
> [ 440.225042] process_one_work+0x237/0x5d0
> [ 440.225043] worker_thread+0x3c/0x390
> [ 440.225045] kthread+0x11d/0x140
> [ 440.225046] ret_from_fork+0x3a/0x50
> [ 440.225047]
> -> #1 (&(&list->lock)->rlock#8){+.-.}:
> [ 440.225049] _raw_spin_lock_bh+0x34/0x40
> [ 440.225052] mt76_tx_status_skb_add+0x51/0x100 [mt76]
> [ 440.225054] mt76x02u_tx_prepare_skb+0xbd/0x116 [mt76x02_usb]
> [ 440.225056] mt76u_tx_queue_skb+0x5f/0x180 [mt76_usb]
> [ 440.225058] mt76_tx+0x93/0x190 [mt76]
> [ 440.225070] ieee80211_tx_frags+0x148/0x210 [mac80211]
> [ 440.225081] __ieee80211_tx+0x75/0x1b0 [mac80211]
> [ 440.225092] ieee80211_tx+0xde/0x110 [mac80211]
> [ 440.225105] __ieee80211_tx_skb_tid_band+0x72/0x90 [mac80211]
> [ 440.225122] ieee80211_send_auth+0x1f3/0x360 [mac80211]
> [ 440.225141] ieee80211_auth.cold.40+0x6c/0x100 [mac80211]
> [ 440.225156] ieee80211_mgd_auth.cold.50+0x132/0x15f [mac80211]
> [ 440.225171] cfg80211_mlme_auth+0x149/0x360 [cfg80211]
> [ 440.225181] nl80211_authenticate+0x273/0x2e0 [cfg80211]
> [ 440.225183] genl_family_rcv_msg+0x196/0x3a0
> [ 440.225184] genl_rcv_msg+0x47/0x8e
> [ 440.225185] netlink_rcv_skb+0x3a/0xf0
> [ 440.225187] genl_rcv+0x24/0x40
> [ 440.225188] netlink_unicast+0x16d/0x210
> [ 440.225189] netlink_sendmsg+0x204/0x3b0
> [ 440.225191] sock_sendmsg+0x36/0x40
> [ 440.225193] ___sys_sendmsg+0x259/0x2b0
> [ 440.225194] __sys_sendmsg+0x47/0x80
> [ 440.225196] do_syscall_64+0x60/0x1f0
> [ 440.225197] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 440.225198]
> -> #0 (&(&q->lock)->rlock#2){+.-.}:
> [ 440.225200] lock_acquire+0xb9/0x1a0
> [ 440.225202] _raw_spin_lock_bh+0x34/0x40
> [ 440.225204] mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225215] ieee80211_agg_start_txq+0xe8/0x2b0 [mac80211]
> [ 440.225225] ieee80211_stop_tx_ba_cb+0xb8/0x1f0 [mac80211]
> [ 440.225235] ieee80211_ba_session_work+0x1c1/0x2f0 [mac80211]
> [ 440.225236] process_one_work+0x237/0x5d0
> [ 440.225237] worker_thread+0x3c/0x390
> [ 440.225239] kthread+0x11d/0x140
> [ 440.225240] ret_from_fork+0x3a/0x50
> [ 440.225240]
> other info that might help us debug this:
>
> [ 440.225241] Chain exists of:
> &(&q->lock)->rlock#2 --> &(&sta->rate_ctrl_lock)->rlock --> &(&sta->lock)->rlock
>
> [ 440.225243] Possible unsafe locking scenario:
>
> [ 440.225244] CPU0 CPU1
> [ 440.225244] ---- ----
> [ 440.225245] lock(&(&sta->lock)->rlock);
> [ 440.225245] lock(&(&sta->rate_ctrl_lock)->rlock);
> [ 440.225246] lock(&(&sta->lock)->rlock);
> [ 440.225247] lock(&(&q->lock)->rlock#2);
> [ 440.225248]
> *** DEADLOCK ***
>
> [ 440.225249] 5 locks held by kworker/u16:28/2362:
> [ 440.225250] #0: 0000000048fcd291 ((wq_completion)phy0){+.+.}, at: process_one_work+0x1b5/0x5d0
> [ 440.225252] #1: 00000000f1c6828f ((work_completion)(&sta->ampdu_mlme.work)){+.+.}, at: process_one_work+0x1b5/0x5d0
> [ 440.225254] #2: 00000000433d2b2c (&sta->ampdu_mlme.mtx){+.+.}, at: ieee80211_ba_session_work+0x5c/0x2f0 [mac80211]
> [ 440.225265] #3: 000000002cfedc59 (&(&sta->lock)->rlock){+.-.}, at: ieee80211_stop_tx_ba_cb+0x32/0x1f0 [mac80211]
> [ 440.225276] #4: 000000009d7b9a44 (rcu_read_lock){....}, at: ieee80211_agg_start_txq+0x33/0x2b0 [mac80211]
> [ 440.225286]
> stack backtrace:
> [ 440.225288] CPU: 2 PID: 2362 Comm: kworker/u16:28 Not tainted 5.1.0-rc2+ #22
> [ 440.225289] Hardware name: LENOVO 20KGS23S0P/20KGS23S0P, BIOS N23ET55W (1.30 ) 08/31/2018
> [ 440.225300] Workqueue: phy0 ieee80211_ba_session_work [mac80211]
> [ 440.225301] Call Trace:
> [ 440.225304] dump_stack+0x85/0xc0
> [ 440.225306] print_circular_bug.isra.38.cold.58+0x15c/0x195
> [ 440.225307] check_prev_add.constprop.48+0x5f0/0xc00
> [ 440.225309] ? check_prev_add.constprop.48+0x39d/0xc00
> [ 440.225311] ? __lock_acquire+0x41d/0x1100
> [ 440.225312] __lock_acquire+0xd98/0x1100
> [ 440.225313] ? __lock_acquire+0x41d/0x1100
> [ 440.225315] lock_acquire+0xb9/0x1a0
> [ 440.225317] ? mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225319] _raw_spin_lock_bh+0x34/0x40
> [ 440.225321] ? mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225323] mt76_wake_tx_queue+0x4c/0xb0 [mt76]
> [ 440.225334] ieee80211_agg_start_txq+0xe8/0x2b0 [mac80211]
> [ 440.225344] ieee80211_stop_tx_ba_cb+0xb8/0x1f0 [mac80211]
> [ 440.225354] ieee80211_ba_session_work+0x1c1/0x2f0 [mac80211]
> [ 440.225356] process_one_work+0x237/0x5d0
> [ 440.225358] worker_thread+0x3c/0x390
> [ 440.225359] ? wq_calc_node_cpumask+0x70/0x70
> [ 440.225360] kthread+0x11d/0x140
> [ 440.225362] ? kthread_create_on_node+0x40/0x40
> [ 440.225363] ret_from_fork+0x3a/0x50
>
> Cc: stable@vger.kernel.org
> Fixes: 88046b2c9f6d ("mt76: add support for reporting tx status with skb")
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> Acked-by: Felix Fietkau <nbd@nbd.name>
Patch applied to wireless-drivers.git, thanks.
bafdf85dfa59 mt76x02: avoid status_list.lock and sta->rate_ctrl_lock dependency
--
https://patchwork.kernel.org/patch/10887243/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 5.1] rt2x00: do not increment sequence number while re-transmitting
From: Kalle Valo @ 2019-04-12 18:31 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linux-wireless, Tomislav Požega, Daniel Golle, Felix Fietkau,
Mathias Kresin, Vijayakumar Durai, stable, Stanislaw Gruszka
In-Reply-To: <1553680997-19511-1-git-send-email-sgruszka@redhat.com>
Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> From: Vijayakumar Durai <vijayakumar.durai1@vivint.com>
>
> Currently rt2x00 devices retransmit the management frames with
> incremented sequence number if hardware is assigning the sequence.
>
> This is HW bug fixed already for non-QOS data frames, but it should
> be fixed for management frames except beacon.
>
> Without fix retransmitted frames have wrong SN:
>
> AlphaNet_e8:fb:36 Vivotek_52:31:51 Authentication, SN=1648, FN=0, Flags=........C Frame is not being retransmitted 1648 1
> AlphaNet_e8:fb:36 Vivotek_52:31:51 Authentication, SN=1649, FN=0, Flags=....R...C Frame is being retransmitted 1649 1
> AlphaNet_e8:fb:36 Vivotek_52:31:51 Authentication, SN=1650, FN=0, Flags=....R...C Frame is being retransmitted 1650 1
>
> With the fix SN stays correctly the same:
>
> 88:6a:e3:e8:f9:a2 8c:f5:a3:88:76:87 Authentication, SN=1450, FN=0, Flags=........C
> 88:6a:e3:e8:f9:a2 8c:f5:a3:88:76:87 Authentication, SN=1450, FN=0, Flags=....R...C
> 88:6a:e3:e8:f9:a2 8c:f5:a3:88:76:87 Authentication, SN=1450, FN=0, Flags=....R...C
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Vijayakumar Durai <vijayakumar.durai1@vivint.com>
> [sgruszka: simplify code, change comments and changelog]
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Patch applied to wireless-drivers.git, thanks.
746ba11f1706 rt2x00: do not increment sequence number while re-transmitting
--
https://patchwork.kernel.org/patch/10873171/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 5.1 1/3] mt76: mt7603: add missing initialization for dev->ps_lock
From: Kalle Valo @ 2019-04-12 18:29 UTC (permalink / raw)
To: Felix Fietkau; +Cc: linux-wireless
In-Reply-To: <20190326083421.57998-1-nbd@nbd.name>
Felix Fietkau <nbd@nbd.name> wrote:
> Fixes lockdep complaint and a potential race condition
>
> Signed-off-by: Felix Fietkau <nbd@nbd.name>
3 patches applied to wireless-drivers.git, thanks.
2170e2157d7c mt76: mt7603: add missing initialization for dev->ps_lock
aa3cb24be18b mt76: mt7603: fix sequence number assignment
9dc27bcbe78c mt76: mt7603: send BAR after powersave wakeup
--
https://patchwork.kernel.org/patch/10870631/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCHv3 2/2] mac80211: Implement API to configure station specific rssi threshold
From: kbuild test robot @ 2019-04-12 17:52 UTC (permalink / raw)
To: Tamizh chelvam; +Cc: kbuild-all, johannes, linux-wireless, Tamizh chelvam
In-Reply-To: <1554831488-27065-3-git-send-email-tamizhr@codeaurora.org>
Hi Tamizh,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on mac80211-next/master]
[also build test WARNING on next-20190412]
[cannot apply to v5.1-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Tamizh-chelvam/cfg80211-Add-support-to-configure-station-specific-RSSI-threshold-for-AP-mode/20190412-225220
base: https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
sparse warnings: (new ones prefixed by >>)
>> net/mac80211/cfg.c:3947:17: sparse: incompatible types in comparison expression (different address spaces)
--
>> net/mac80211/rx.c:1735:13: sparse: context imbalance in 'ieee80211_sta_rx_signal_thold_check' - wrong count at exit
vim +3947 net/mac80211/cfg.c
3940
3941 void sta_mon_rssi_config_free(struct sta_info *sta)
3942 {
3943 struct sta_mon_rssi_config *old_rssi_config;
3944
3945 if (sta->rssi_config) {
3946 old_rssi_config = sta->rssi_config;
> 3947 RCU_INIT_POINTER(sta->rssi_config, NULL);
3948 kfree_rcu(old_rssi_config, rcu_head);
3949 }
3950 }
3951
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: gsmtap design/extensions?
From: Johannes Berg @ 2019-04-12 17:15 UTC (permalink / raw)
To: Harald Welte
Cc: Vadim Yanitskiy, OpenBSC Mailing List, Sean Tranchetti, radiotap,
Dan Williams, netdev, linux-wireless, Aleksander Morgado,
Subash Abhinov Kasiviswanathan, Bjørn Mork
In-Reply-To: <20190410234555.GO25552@nataraja>
On Thu, 2019-04-11 at 01:45 +0200, Harald Welte wrote:
> Hi Johannes,
>
> On Wed, Apr 10, 2019 at 09:23:13AM +0200, Johannes Berg wrote:
> > > but unfortunately, nobody has invested time into this (yet?).
> >
> > 2012!
>
> Well, Osmocom is a very small community, with probably somewhere less than
> 25 active developers over the last few years (less than 15 full-time),
> with an *incredibly* large scope: Implement virtually any protocol
> layer of any protocol stack on any of the 3GPP interfaces and all their
> related network elements for 2G/3G as well as even other technologies
> like TETRA, GMR-1, ...
>
> And all that in a field of technology that has less free software than
> the Operating Systems world had in the mid-1990ies. It really feels a
> bit like the Linux community 20 years ago.
>
> So resources are always *extremely* tight, and given those limited
> resources, I'm actually very happy with the results by now, having
> automatied CI, build verifications, unit tests, functional test suites,
> end-to-end testing, and all the code we implemented on git.osmocom.org :)
:-)
Sure, I get it. Just a bit surprised I guess.
> While current GSMTAPv2 is ugly, it works rather solid for all known
> existing use cases, so there was no urgency to introduce a new version
> of it.
OK.
> > Not sure I get this, but I also don't really care all that much.
>
> Well, with all respect, GSMTAP was created for a variety of use cases,
> see my other lengthy mail. It's fine if you don't care, but unless you
> could explain your use cases with a few paragraphs, neither you nor us
> are able to determine if there is common functionality and if it makes
> sense to use GSMTAP or not :)
Agree. Sorry about that. No disrespect was intended, but I'm still not
sure I understand the need for UDP encapsulation *as part of the
protocol*. I guess saying "GSMTAP can optionally be encapsulated in UDP
with the well-known port xyz" would be something else, and it'd make
more sense to me than saying it has to be.
> So far I have not seen any explanation about what kind of data you want
> to encapsulate at all.
Right, see my other mail(s) from today as well. Basically the IP frame
that we're actually sending, and then attaching to that the "session" or
"mux id" that it's being sent on. Sorry, I probably also don't know the
right term - those show up in the drivers now.
> > just a pretty strange design if the kernel were to output this, I'm not
> > even sure how I'd do that properly. I don't want to be generating UDP
> > packets there...
>
> There are well-established APIs for having sockets in the kernel and for
> generating + receiving UDP packets from it. NFS has been doing this for
> decades, as do various kernel-side tunneling helpers including the GTP
> kernel module.
>
> I'm not saying it's the right approach for your problem, I'm just saying
> kernel-side code can for sure use UDP sockets.
Of course it *can*. But I don't think it makes *sense*. The key feature
here isn't communicating with somebody else (unlike NFS, GTP, GENEVE and
whatever other protocol they have). In fact, you shouldn't really care
about the communication part per se at all, I'd think.
> Sure, that works. But the real question is, to me: Are there common
> GSMTAP payload types that both the existing GSMTAP users carry, as well
> as what you would want to carry? If yes, then it makes sense to think
> about a common encapsulation like GSMTAP. If the payload types differ,
> then it seems rather like there are two distinct use cases that
> wouldn't benefit from standardizing on one format.
Agree, and I don't really know.
Maybe I should start differently. Do you have an example GSMTAP capture
file that I could look at in wireshark? Yes, I see you've pointed out
how I can get all the software running, but if you have a file already
that's almost certainly faster :-)
And then the question I'd want answer is this: If there's an IP frame
that I send to the modem from the application using a regular UDP or TCP
socket, what would the corresponding GSMTAP capture look like? Surely it
includes the IP frame in some way?!
If the answer to that question is yes, then I think there is some
overlap, because you can always imagine the modem receiving an IP frame
and telling you more about how it was encapsulated over the air, no?
Mind, most if not all modems probably don't actually do that today, but
I wonder how much of that is because of lack of infrastructure to do it,
vs. it just not being necessary - since I've been told that the modems
do in fact often output tracing that contains information about this.
Just not directly combined with the IP frame.
johannes
^ permalink raw reply
* Re: [PATCH] mt76: usb: fix possible memory leak during suspend/resume
From: Lorenzo Bianconi @ 2019-04-12 16:27 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190412153509.GB3156@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 2259 bytes --]
> > On Fri, Apr 12, 2019 at 02:27:16PM +0200, Lorenzo Bianconi wrote:
> > > Disable mt76u_tx_tasklet at the end of mt76u_stop_queues in order to
> > > properly deallocate all pending skbs during suspend/resume phase
> >
> > On suspend/resume tx skb's are processed after tasklet_enable()
> > in resume callback. There is issue with device removal though
> > (during suspend or otherwise).
>
> Hi Stanislaw,
>
> I guess the right moment to deallocate the skbs is during suspend since resume
> can happen in very far future
>
> >
> > > Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > drivers/net/wireless/mediatek/mt76/usb.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> > > index a3acc070063a..575207133775 100644
> > > --- a/drivers/net/wireless/mediatek/mt76/usb.c
> > > +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> > > @@ -842,10 +842,10 @@ static void mt76u_stop_tx(struct mt76_dev *dev)
> > > void mt76u_stop_queues(struct mt76_dev *dev)
> > > {
> > > tasklet_disable(&dev->usb.rx_tasklet);
> > > - tasklet_disable(&dev->usb.tx_tasklet);
> > > -
> > > mt76u_stop_rx(dev);
> > > +
> > > mt76u_stop_tx(dev);
> > > + tasklet_disable(&dev->usb.tx_tasklet);
> >
> > If tasklet is scheduled and we disable it and never enable, we end up
> > with infinite loop in tasklet_action_common(). This patch make the
> > problem less reproducible since tasklet_disable() is moved after
> > usb_kill_urb() -> tasklet_schedule(), but it is still possible.
>
> I can see the point here. Maybe we can just run tasklet_kill instead of
> tasklet_disable here (at least for tx one)
I think it is enough even for rx side since usb_kill_urb() will wait the end of
mt76u_complete_rx() and tasklet_kill() will wait for the completion of
mt76u_rx_tasklet(). We will need to remove tasklet_enable in resume routines.
Regards,
Lorenzo
>
> Regards,
> Lorenzo
>
> >
> > I comprehansive have fix for that, but giving it more testing.
> > Please drop this patch.
> >
> > Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] mt76: usb: fix possible memory leak during suspend/resume
From: Lorenzo Bianconi @ 2019-04-12 15:35 UTC (permalink / raw)
To: Stanislaw Gruszka; +Cc: Lorenzo Bianconi, nbd, linux-wireless
In-Reply-To: <20190412145442.GA2539@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]
> On Fri, Apr 12, 2019 at 02:27:16PM +0200, Lorenzo Bianconi wrote:
> > Disable mt76u_tx_tasklet at the end of mt76u_stop_queues in order to
> > properly deallocate all pending skbs during suspend/resume phase
>
> On suspend/resume tx skb's are processed after tasklet_enable()
> in resume callback. There is issue with device removal though
> (during suspend or otherwise).
Hi Stanislaw,
I guess the right moment to deallocate the skbs is during suspend since resume
can happen in very far future
>
> > Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> > drivers/net/wireless/mediatek/mt76/usb.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> > index a3acc070063a..575207133775 100644
> > --- a/drivers/net/wireless/mediatek/mt76/usb.c
> > +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> > @@ -842,10 +842,10 @@ static void mt76u_stop_tx(struct mt76_dev *dev)
> > void mt76u_stop_queues(struct mt76_dev *dev)
> > {
> > tasklet_disable(&dev->usb.rx_tasklet);
> > - tasklet_disable(&dev->usb.tx_tasklet);
> > -
> > mt76u_stop_rx(dev);
> > +
> > mt76u_stop_tx(dev);
> > + tasklet_disable(&dev->usb.tx_tasklet);
>
> If tasklet is scheduled and we disable it and never enable, we end up
> with infinite loop in tasklet_action_common(). This patch make the
> problem less reproducible since tasklet_disable() is moved after
> usb_kill_urb() -> tasklet_schedule(), but it is still possible.
I can see the point here. Maybe we can just run tasklet_kill instead of
tasklet_disable here (at least for tx one)
Regards,
Lorenzo
>
> I comprehansive have fix for that, but giving it more testing.
> Please drop this patch.
>
> Stanislaw
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: NL80211_SCAN_FLAG_RANDOM_ADDR ?
From: Denis Kenzior @ 2019-04-12 15:00 UTC (permalink / raw)
To: Sergey Matyukevich; +Cc: linux-wireless@vger.kernel.org
In-Reply-To: <20190412092623.46ygdnec3wx26zvd@bars>
Hi Sergey,
On 04/12/2019 04:26 AM, Sergey Matyukevich wrote:
>> I've been poking around at how this flag is used and I noticed this
>> check in net/wireless/nl80211.c:
>>
>> nl80211_check_scan_flags()
>>
>> if (*flags & NL80211_SCAN_FLAG_RANDOM_ADDR) {
>> int err;
>>
>> if (!(wiphy->features & randomness_flag) ||
>> (wdev && wdev->current_bss))
>> return -EOPNOTSUPP;
>>
>>
>> The above disallows the use of RANDOM_ADDR for scans while connected.
>> The nl80211.h uapi header seems to concur:
>>
>> "@NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR: This device/driver supports
>> using a random MAC address during scan (if the device is unassociated);"
>>
>> However, if I create a P2P Device (in addition to the default STA
>> device), the kernel happily lets me scan on the wdev while the STA
>> interface is connected.
>>
>> sudo iw phy0 interface add p2p type __p2pdev
>> sudo iw wdev 0x2 p2p start
>> sudo iw wdev 0x2 scan randomize
>>
>> So the immediate question I have is, should the RANDOM_ADDR flag indeed
>> be limited to unassociated STA interfaces? It would seem the hardware
>> is capable randomizing even when connected? Please educate me :)
>
> Hello Denis,
>
> IIUC, this feature could be introduced to support Android Compatibility
> Definition Document (CDD). Those documents are available at the
> following page: https://source.android.com/compatibility/cdd
Thanks for the reference. It looks like a 'At a minimum you should/must
do this' type of document. It doesn't look like it precludes the use of
randomization when connected?
>
> For instance, in the latest CDD randomized scan requirements are described
> in the section 7.4.2. It looks like current high level nl80211 API follows
> those recommendations. Probably it has been implemented with STA use-case
> in mind, that is why you can use that flag for P2P connection. But, as
> Ben pointed out, actual application of this flag may depend on
> implementation in firwmare and hardware.
>
Sure, understood. But this is exactly the point of my question. Is the
check at the global level correct? Or should it be relaxed in case
there is hardware out there that can randomize probe requests while
connected? From my test it would seem this is possible?
Or put another way, besides hardware limitations, are there reasons why
you would not want to randomize probe request address when connected?
Regards,
-Denis
^ permalink raw reply
* Re: [PATCH] mt76: usb: fix possible memory leak during suspend/resume
From: Stanislaw Gruszka @ 2019-04-12 14:54 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: nbd, linux-wireless, lorenzo.bianconi
In-Reply-To: <bdd1046beaee6104b1fcd25b1069f25db8f2fb20.1555071870.git.lorenzo@kernel.org>
On Fri, Apr 12, 2019 at 02:27:16PM +0200, Lorenzo Bianconi wrote:
> Disable mt76u_tx_tasklet at the end of mt76u_stop_queues in order to
> properly deallocate all pending skbs during suspend/resume phase
On suspend/resume tx skb's are processed after tasklet_enable()
in resume callback. There is issue with device removal though
(during suspend or otherwise).
> Fixes: b40b15e1521f ("mt76: add usb support to mt76 layer")
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> drivers/net/wireless/mediatek/mt76/usb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
> index a3acc070063a..575207133775 100644
> --- a/drivers/net/wireless/mediatek/mt76/usb.c
> +++ b/drivers/net/wireless/mediatek/mt76/usb.c
> @@ -842,10 +842,10 @@ static void mt76u_stop_tx(struct mt76_dev *dev)
> void mt76u_stop_queues(struct mt76_dev *dev)
> {
> tasklet_disable(&dev->usb.rx_tasklet);
> - tasklet_disable(&dev->usb.tx_tasklet);
> -
> mt76u_stop_rx(dev);
> +
> mt76u_stop_tx(dev);
> + tasklet_disable(&dev->usb.tx_tasklet);
If tasklet is scheduled and we disable it and never enable, we end up
with infinite loop in tasklet_action_common(). This patch make the
problem less reproducible since tasklet_disable() is moved after
usb_kill_urb() -> tasklet_schedule(), but it is still possible.
I comprehansive have fix for that, but giving it more testing.
Please drop this patch.
Stanislaw
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Tvrtko Ursulin @ 2019-04-12 14:47 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: linux-fbdev, Stanislaw Gruszka, Petr Mladek, David Airlie,
dri-devel, devel, linux-scsi, Jassi Brar, ath10k, intel-gfx,
Jose Abreu, Tom Lendacky, James E.J. Bottomley, linux-fsdevel,
Steven Rostedt, Kalle Valo, Karsten Keil, Martin K. Petersen,
Greg Kroah-Hartman, linux-wireless, linux-kernel,
Sergey Senozhatsky, netdev, Enric Balletbo i Serra, Andrew Morton,
David S. Miller, Alexander Viro
In-Reply-To: <20190410031720.11067-4-alastair@au1.ibm.com>
On 10/04/2019 04:17, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> In order to support additional features in hex_dump_to_buffer, replace
> the ascii bool parameter with flags.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
> drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++--
> drivers/mailbox/mailbox-test.c | 2 +-
> drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +-
> drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +-
> drivers/net/wireless/ath/ath10k/debug.c | 3 ++-
> drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +-
> drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++-
> drivers/scsi/scsi_logging.c | 8 +++-----
> drivers/staging/fbtft/fbtft-core.c | 2 +-
> fs/seq_file.c | 3 ++-
> include/linux/printk.h | 2 +-
> lib/hexdump.c | 15 ++++++++-------
> lib/test_hexdump.c | 5 +++--
> 14 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 49fa43ff02ba..fb133e729f9a 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
> WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> rowsize, sizeof(u32),
> line, sizeof(line),
> - false) >= sizeof(line));
> + 0) >= sizeof(line));
> drm_printf(m, "[%04zx] %s\n", pos, line);
>
> prev = buf + pos;
i915 code here actually does something I think is more interesting than
HEXDUMP_SUPPRESS_0X**. It replaces any N repeating lines with a single star
marker. For example:
00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
*
00000040 00000001 00000000 00000018 00000002 00000001 00000000 00000018 00000000
00000060 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000003
00000080 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
*
000000c0 00000002 00000000 00000000 00000000 00000000 00000000 00000000 00000000
000000e0 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
*
If or when you end up with this feature in your series you can therefore
replace the whole implementation of hexdump in the above file.
Regards,
Tvrtko
^ permalink raw reply
* INFO: trying to register non-static key in del_timer_sync (2)
From: syzbot @ 2019-04-12 14:26 UTC (permalink / raw)
To: amitkarwar, andreyknvl, davem, gbhat, huxinming820, kvalo,
linux-kernel, linux-usb, linux-wireless, netdev, nishants,
syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 9a33b369 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan/tree/usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=14793fa7200000
kernel config: https://syzkaller.appspot.com/x/.config?x=23e37f59d94ddd15
dashboard link: https://syzkaller.appspot.com/bug?extid=dc4127f950da51639216
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=16f8c22d200000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=16eeadbb200000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+dc4127f950da51639216@syzkaller.appspotmail.com
usb 1-1: string descriptor 0 read error: -71
usb 1-1: USB disconnect, device number 2
usb 1-1: Direct firmware load for mrvl/usb8801_uapsta.bin failed with error
-2
usb 1-1: Failed to get firmware mrvl/usb8801_uapsta.bin
usb 1-1: info: _mwifiex_fw_dpc: unregister device
INFO: trying to register non-static key.
the code is fine but needs lockdep annotation.
turning off the locking correctness validator.
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: events request_firmware_work_func
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xe8/0x16e lib/dump_stack.c:113
assign_lock_key kernel/locking/lockdep.c:786 [inline]
register_lock_class+0x11b8/0x1250 kernel/locking/lockdep.c:1095
__lock_acquire+0xfb/0x37c0 kernel/locking/lockdep.c:3582
lock_acquire+0x10d/0x2f0 kernel/locking/lockdep.c:4211
del_timer_sync+0x4c/0x150 kernel/time/timer.c:1282
mwifiex_usb_cleanup_tx_aggr
drivers/net/wireless/marvell/mwifiex/usb.c:1358 [inline]
mwifiex_unregister_dev+0x41b/0x690
drivers/net/wireless/marvell/mwifiex/usb.c:1370
_mwifiex_fw_dpc+0x711/0xdd0 drivers/net/wireless/marvell/mwifiex/main.c:651
request_firmware_work_func+0x12d/0x249
drivers/base/firmware_loader/main.c:785
process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
kthread+0x313/0x420 kernel/kthread.c:253
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
------------[ cut here ]------------
ODEBUG: assert_init not available (active state 0) object type: timer_list
hint: (null)
WARNING: CPU: 0 PID: 12 at lib/debugobjects.c:325
debug_print_object+0x162/0x250 lib/debugobjects.c:325
Kernel panic - not syncing: panic_on_warn set ...
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: events request_firmware_work_func
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xe8/0x16e lib/dump_stack.c:113
panic+0x29d/0x5f2 kernel/panic.c:214
__warn.cold+0x20/0x48 kernel/panic.c:571
report_bug+0x262/0x2a0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:179 [inline]
fixup_bug arch/x86/kernel/traps.c:174 [inline]
do_error_trap+0x130/0x1f0 arch/x86/kernel/traps.c:272
do_invalid_op+0x37/0x40 arch/x86/kernel/traps.c:291
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:debug_print_object+0x162/0x250 lib/debugobjects.c:325
Code: dd e0 a1 b3 8e 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 bf 00 00 00 48
8b 14 dd e0 a1 b3 8e 48 c7 c7 60 96 b3 8e e8 8e 93 d2 fd <0f> 0b 83 05 e9
d6 59 10 01 48 83 c4 20 5b 5d 41 5c 41 5d c3 48 89
RSP: 0018:ffff8880a84b78d8 EFLAGS: 00010286
RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff815b1e42 RDI: ffffed1015096f0d
RBP: 0000000000000001 R08: ffff8880a849b100 R09: fffffbfff22f95ed
R10: fffffbfff22f95ec R11: ffffffff917caf63 R12: ffffffff917e7780
R13: ffffffff8161ec90 R14: 1ffff11015096f28 R15: ffff88809fc893f8
debug_object_assert_init lib/debugobjects.c:694 [inline]
debug_object_assert_init+0x23d/0x2f0 lib/debugobjects.c:665
debug_timer_assert_init kernel/time/timer.c:723 [inline]
debug_assert_init kernel/time/timer.c:775 [inline]
try_to_del_timer_sync+0x72/0x110 kernel/time/timer.c:1222
del_timer_sync+0x112/0x150 kernel/time/timer.c:1292
mwifiex_usb_cleanup_tx_aggr
drivers/net/wireless/marvell/mwifiex/usb.c:1358 [inline]
mwifiex_unregister_dev+0x41b/0x690
drivers/net/wireless/marvell/mwifiex/usb.c:1370
_mwifiex_fw_dpc+0x711/0xdd0 drivers/net/wireless/marvell/mwifiex/main.c:651
request_firmware_work_func+0x12d/0x249
drivers/base/firmware_loader/main.c:785
process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
kthread+0x313/0x420 kernel/kthread.c:253
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Kernel Offset: disabled
Rebooting in 86400 seconds..
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* general protection fault in ath6kl_usb_alloc_urb_from_pipe
From: syzbot @ 2019-04-12 14:26 UTC (permalink / raw)
To: andreyknvl, davem, kvalo, linux-kernel, linux-usb, linux-wireless,
netdev, syzkaller-bugs
Hello,
syzbot found the following crash on:
HEAD commit: 9a33b369 usb-fuzzer: main usb gadget fuzzer driver
git tree: https://github.com/google/kasan/tree/usb-fuzzer
console output: https://syzkaller.appspot.com/x/log.txt?x=12405cd3200000
kernel config: https://syzkaller.appspot.com/x/.config?x=23e37f59d94ddd15
dashboard link: https://syzkaller.appspot.com/bug?extid=ead4037ec793e025e66f
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11acc1af200000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1344acd3200000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+ead4037ec793e025e66f@syzkaller.appspotmail.com
usb 1-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
usb 1-1: config 0 descriptor??
usb 1-1: string descriptor 0 read error: -71
kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN PTI
CPU: 0 PID: 12 Comm: kworker/0:1 Not tainted 5.1.0-rc4-319354-g9a33b36 #3
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
RIP: 0010:__lock_acquire+0xadc/0x37c0 kernel/locking/lockdep.c:3573
Code: 00 0f 85 c1 1d 00 00 48 81 c4 10 01 00 00 5b 5d 41 5c 41 5d 41 5e 41
5f c3 48 b8 00 00 00 00 00 fc ff df 4c 89 ea 48 c1 ea 03 <80> 3c 02 00 0f
85 35 1e 00 00 49 81 7d 00 40 39 01 96 0f 84 e8 f5
RSP: 0018:ffff8880a84b6f78 EFLAGS: 00010006
RAX: dffffc0000000000 RBX: ffff8880a849c980 RCX: 0000000000000000
RDX: 0000000000000003 RSI: 0000000000000000 RDI: 0000000000000001
RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000000 R11: ffff8880a849c980 R12: 0000000000000000
R13: 0000000000000018 R14: 0000000000000001 R15: 0000000000000001
FS: 0000000000000000(0000) GS:ffff8880ad000000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004c6768 CR3: 000000009b83e000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
lock_acquire+0x10d/0x2f0 kernel/locking/lockdep.c:4211
__raw_spin_lock_irqsave include/linux/spinlock_api_smp.h:110 [inline]
_raw_spin_lock_irqsave+0x44/0x60 kernel/locking/spinlock.c:152
ath6kl_usb_alloc_urb_from_pipe+0x49/0x2b0
drivers/net/wireless/ath/ath6kl/usb.c:135
ath6kl_usb_post_recv_transfers.constprop.0+0x233/0x400
drivers/net/wireless/ath/ath6kl/usb.c:410
ath6kl_usb_start_recv_pipes drivers/net/wireless/ath/ath6kl/usb.c:484
[inline]
hif_start drivers/net/wireless/ath/ath6kl/usb.c:682 [inline]
ath6kl_usb_power_on+0x8d/0x120 drivers/net/wireless/ath/ath6kl/usb.c:1041
ath6kl_hif_power_on drivers/net/wireless/ath/ath6kl/hif-ops.h:136 [inline]
ath6kl_core_init drivers/net/wireless/ath/ath6kl/core.c:97 [inline]
ath6kl_core_init+0x1b8/0x1060 drivers/net/wireless/ath/ath6kl/core.c:66
ath6kl_usb_probe+0xc7f/0x1180 drivers/net/wireless/ath/ath6kl/usb.c:1147
usb_probe_interface+0x31d/0x820 drivers/usb/core/driver.c:361
really_probe+0x2da/0xb10 drivers/base/dd.c:509
driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
__device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
__device_attach+0x223/0x3a0 drivers/base/dd.c:844
bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
device_add+0xad2/0x16e0 drivers/base/core.c:2106
usb_set_configuration+0xdf7/0x1740 drivers/usb/core/message.c:2021
generic_probe+0xa2/0xda drivers/usb/core/generic.c:210
usb_probe_device+0xc0/0x150 drivers/usb/core/driver.c:266
really_probe+0x2da/0xb10 drivers/base/dd.c:509
driver_probe_device+0x21d/0x350 drivers/base/dd.c:671
__device_attach_driver+0x1d8/0x290 drivers/base/dd.c:778
bus_for_each_drv+0x163/0x1e0 drivers/base/bus.c:454
__device_attach+0x223/0x3a0 drivers/base/dd.c:844
bus_probe_device+0x1f1/0x2a0 drivers/base/bus.c:514
device_add+0xad2/0x16e0 drivers/base/core.c:2106
usb_new_device.cold+0x537/0xccf drivers/usb/core/hub.c:2534
hub_port_connect drivers/usb/core/hub.c:5089 [inline]
hub_port_connect_change drivers/usb/core/hub.c:5204 [inline]
port_event drivers/usb/core/hub.c:5350 [inline]
hub_event+0x138e/0x3b00 drivers/usb/core/hub.c:5432
process_one_work+0x90f/0x1580 kernel/workqueue.c:2269
worker_thread+0x9b/0xe20 kernel/workqueue.c:2415
kthread+0x313/0x420 kernel/kthread.c:253
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Modules linked in:
---[ end trace 4efca44223c9b0b0 ]---
RIP: 0010:__lock_acquire+0xadc/0x37c0 kernel/locking/lockdep.c:3573
Code: 00 0f 85 c1 1d 00 00 48 81 c4 10 01 00 00 5b 5d 41 5c 41 5d 41 5e 41
5f c3 48 b8 00 00 00 00 00 fc ff df 4c 89 ea 48 c1 ea 03 <80> 3c 02 00 0f
85 35 1e 00 00 49 81 7d 00 40 39 01 96 0f 84 e8 f5
RSP: 0018:ffff8880a84b6f78 EFLAGS: 00010006
RAX: dffffc0000000000 RBX: ffff8880a849c980 RCX: 0000000000000000
RDX: 0000000000000003 RSI: 0000000000000000 RDI: 0000000000000001
RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000001
R10: 0000000000000000 R11: ffff8880a849c980 R12: 0000000000000000
R13: 0000000000000018 R14: 0000000000000001 R15: 0000000000000001
FS: 0000000000000000(0000) GS:ffff8880ad000000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00000000004c6768 CR3: 000000009b83e000 CR4: 00000000001406f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH 3/4] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
From: Petr Mladek @ 2019-04-12 14:12 UTC (permalink / raw)
To: Alastair D'Silva
Cc: alastair, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David Airlie, Daniel Vetter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Sergey Senozhatsky, Steven Rostedt, Andrew Morton,
intel-gfx, dri-devel, linux-kernel, netdev, ath10k,
linux-wireless, linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <20190410031720.11067-4-alastair@au1.ibm.com>
On Wed 2019-04-10 13:17:19, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> In order to support additional features in hex_dump_to_buffer, replace
> the ascii bool parameter with flags.
>
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
> drivers/isdn/hardware/mISDN/mISDNisar.c | 6 ++++--
> drivers/mailbox/mailbox-test.c | 2 +-
> drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +-
> drivers/net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +-
> drivers/net/wireless/ath/ath10k/debug.c | 3 ++-
> drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +-
> drivers/platform/chrome/wilco_ec/debugfs.c | 3 ++-
> drivers/scsi/scsi_logging.c | 8 +++-----
> drivers/staging/fbtft/fbtft-core.c | 2 +-
> fs/seq_file.c | 3 ++-
> include/linux/printk.h | 2 +-
> lib/hexdump.c | 15 ++++++++-------
> lib/test_hexdump.c | 5 +++--
> 14 files changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 49fa43ff02ba..fb133e729f9a 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1318,7 +1318,7 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
> WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
> rowsize, sizeof(u32),
> line, sizeof(line),
> - false) >= sizeof(line));
> + 0) >= sizeof(line));
It might be more clear when we define:
#define HEXDUMP_BINARY 0
> drm_printf(m, "[%04zx] %s\n", pos, line);
>
> prev = buf + pos;
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index c014e5573665..82975853c400 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -493,7 +493,7 @@ enum {
>
> extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
> int groupsize, char *linebuf, size_t linebuflen,
> - bool ascii);
> + u64 flags);
I wonder how fancy hex_dump could be. IMHO, u32 should be enough.
The last famous words ;-)
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 2/4] lib/hexdump.c: Optionally suppress lines of filler bytes
From: Petr Mladek @ 2019-04-12 14:03 UTC (permalink / raw)
To: Alastair D'Silva
Cc: alastair, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David Airlie, Daniel Vetter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Sergey Senozhatsky, Steven Rostedt, Andrew Morton,
intel-gfx, dri-devel, linux-kernel, netdev, ath10k,
linux-wireless, linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <20190410031720.11067-3-alastair@au1.ibm.com>
On Wed 2019-04-10 13:17:18, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> Some buffers may only be partially filled with useful data, while the rest
> is padded (typically with 0x00 or 0xff).
>
> This patch introduces flags which allow lines of padding bytes to be
> suppressed, making the output easier to interpret: HEXDUMP_SUPPRESS_0X00,
> HEXDUMP_SUPPRESS_0XFF
>
> The first and last lines are not suppressed by default, so the function
> always outputs something. This behaviour can be further controlled with
> the HEXDUMP_SUPPRESS_FIRST & HEXDUMP_SUPPRESS_LAST flags.
>
> An inline wrapper function is provided for backwards compatibility with
> existing code, which maintains the original behaviour.
>
> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index b8a164814744..2f3bafb55a44 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> +void print_hex_dump_ext(const char *level, const char *prefix_str,
> + int prefix_type, int rowsize, int groupsize,
> + const void *buf, size_t len, u64 flags)
> {
> const u8 *ptr = buf;
> - int i, linelen, remaining = len;
> + int i, remaining = len;
> unsigned char linebuf[64 * 3 + 2 + 64 + 1];
> + bool first_line = true;
>
> if (rowsize != 16 && rowsize != 32 && rowsize != 64)
> rowsize = 16;
>
> for (i = 0; i < len; i += rowsize) {
> - linelen = min(remaining, rowsize);
> + bool skip = false;
> + int linelen = min(remaining, rowsize);
> +
> remaining -= rowsize;
>
> + if (flags & HEXDUMP_SUPPRESS_0X00)
> + skip = buf_is_all(ptr + i, linelen, 0x00);
> +
> + if (!skip && (flags & HEXDUMP_SUPPRESS_0XFF))
> + skip = buf_is_all(ptr + i, linelen, 0xff);
> +
> + if (first_line && !(flags & HEXDUMP_SUPPRESS_FIRST))
> + skip = false;
> +
> + if (remaining <= 0 && !(flags & HEXDUMP_SUPPRESS_LAST))
> + skip = false;
> +
> + if (skip)
> + continue;
IMHO, quietly skipping lines could cause a lot of confusion,
espcially when the address is not printed.
I wonder how it would look like when we print something like:
--- skipped XX lines full of 0x00 ---
Then we might even remove the SUPPRESS_FIRST, SUPPRESS_LAST
and the ambiguous QUIET flags.
> +
> + first_line = false;
This should be above the if (skip).
> +
> hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
> - linebuf, sizeof(linebuf), ascii);
> + linebuf, sizeof(linebuf),
> + flags & HEXDUMP_ASCII);
>
> switch (prefix_type) {
> case DUMP_PREFIX_ADDRESS:
> @@ -272,7 +316,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
> }
> }
> }
> -EXPORT_SYMBOL(print_hex_dump);
> +EXPORT_SYMBOL(print_hex_dump_ext);
We should still export even the original function that
is still used everywhere.
Best Regards,
Petr
^ permalink raw reply
* Re: [PATCH 1/4] lib/hexdump.c: Allow 64 bytes per line
From: Petr Mladek @ 2019-04-12 13:48 UTC (permalink / raw)
To: Alastair D'Silva
Cc: alastair, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
David Airlie, Daniel Vetter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Sergey Senozhatsky, Steven Rostedt, Andrew Morton,
intel-gfx, dri-devel, linux-kernel, netdev, ath10k,
linux-wireless, linux-scsi, linux-fbdev, devel, linux-fsdevel
In-Reply-To: <20190410031720.11067-2-alastair@au1.ibm.com>
On Wed 2019-04-10 13:17:17, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
>
> With modern high resolution screens, we can display more data, which makes
> life a bit easier when debugging.
I have quite some doubts about this feature.
We are talking about more than 256 characters per-line. I wonder
if such a long line is really easier to read for a human.
I am not expert but there is a reason why the standard is 80
characters per-line. I guess that anything above 100 characters
is questionable. https://en.wikipedia.org/wiki/Line_length
somehow confirms that.
Second, if we take 8 pixels per-character. Then we need
2048 to show the 256 characters. It is more than HD.
IMHO, there is still huge number of people that even do
not have HD display, especially on a notebook.
I am sorry but I am strongly against having this hardcoded
anywhere. And I doubt that there will be enough users
to complicate the code and make it configurable.
Best Regards,
Petr
^ permalink raw reply
* [PATCH 6/6] ath10k: fix use-after-free on SDIO data frames
From: Kalle Valo @ 2019-04-12 13:29 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
In-Reply-To: <155507569645.32018.15231567732563770250.stgit@potku.adurom.net>
With SDIO there's a use after free after a data frame is transfered, call stack
below. This happens because ath10k_htt_tx_hl() directly transmits the skb
provided by mac80211 using ath10k_htc_send(), all other HTT functions use
separate skb created with ath10k_htc_alloc_skb() to transmit the HTC packet.
After the packet is transmitted mac80211 frees the skb in ieee80211_tx_status()
but HTT layer expects that it still owns the skb, and frees it in
ath10k_htt_htc_tx_complete().
To fix this take a reference of skb before sending it to HTC layer to make sure
we still own the skb.
Tested on QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
ath10k_htt_tx_hl() is only used by SDIO and USB so other busses (PCI, AHB and
SNOC) should be unaffected.
call stack of use-after-free:
dump_backtrace+0x0/0x2d8
show_stack+0x20/0x2c
__dump_stack+0x20/0x28
dump_stack+0xc8/0xec
print_address_description+0x74/0x240
kasan_report+0x258/0x274
__asan_report_load4_noabort+0x20/0x28
skb_pull+0xbc/0x114
ath10k_htc_notify_tx_completion+0x190/0x2a4 [ath10k_core]
ath10k_sdio_write_async_work+0x1e4/0x2c4 [ath10k_sdio]
process_one_work+0x3d8/0x8b0
worker_thread+0x518/0x7e0
kthread+0x260/0x278
ret_from_fork+0x10/0x18
Allocated by one task:
kasan_kmalloc+0xa0/0x13c
kasan_slab_alloc+0x14/0x1c
kmem_cache_alloc+0x144/0x208
__alloc_skb+0xec/0x394
alloc_skb_with_frags+0x8c/0x374
sock_alloc_send_pskb+0x520/0x5d4
sock_alloc_send_skb+0x40/0x50
__ip_append_data+0xf5c/0x1858
ip_make_skb+0x194/0x1d4
udp_sendmsg+0xf24/0x1ab8
inet_sendmsg+0x1b0/0x2e0
sock_sendmsg+0x88/0xa0
__sys_sendto+0x220/0x3a8
__arm64_sys_sendto+0x78/0x80
el0_svc_common+0x120/0x1e0
el0_svc_compat_handler+0x64/0x80
el0_svc_compat+0x8/0x18
Freed by another task:
__kasan_slab_free+0x120/0x1d4
kasan_slab_free+0x10/0x1c
kmem_cache_free+0x74/0x504
kfree_skbmem+0x88/0xc8
__kfree_skb+0x24/0x2c
consume_skb+0x114/0x18c
__ieee80211_tx_status+0xb7c/0xf60 [mac80211]
ieee80211_tx_status+0x224/0x270 [mac80211]
ath10k_txrx_tx_unref+0x564/0x950 [ath10k_core]
ath10k_htt_t2h_msg_handler+0x178c/0x2a38 [ath10k_core]
ath10k_htt_htc_t2h_msg_handler+0x20/0x30 [ath10k_core]
ath10k_sdio_irq_handler+0xcc0/0x1654 [ath10k_sdio]
process_sdio_pending_irqs+0xec/0x358
sdio_run_irqs+0x68/0xe4
sdio_irq_work+0x1c/0x28
process_one_work+0x3d8/0x8b0
worker_thread+0x518/0x7e0
kthread+0x260/0x278
ret_from_fork+0x10/0x18
Reported-by: Wen Gong <wgong@codeaurora.org>
Tested-by: Wen Gong <wgong@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/htt_tx.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 88f880b601d8..78f07b7a3b21 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -1302,6 +1302,13 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
msdu_id = res;
}
+ /* As msdu is freed by mac80211 (in ieee80211_tx_status()) and by
+ * ath10k (in ath10k_htt_htc_tx_complete()) we have to increase
+ * reference by one to avoid a use-after-free case and a double
+ * free.
+ */
+ skb_get(msdu);
+
skb_push(msdu, sizeof(*cmd_hdr));
skb_push(msdu, sizeof(*tx_desc));
cmd_hdr = (struct htt_cmd_hdr *)msdu->data;
^ permalink raw reply related
* [PATCH 5/6] ath10k: htt: support MSDU ids with SDIO
From: Kalle Valo @ 2019-04-12 13:28 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
In-Reply-To: <155507569645.32018.15231567732563770250.stgit@potku.adurom.net>
From: Alagu Sankar <alagusankar@silex-india.com>
Transmit completion for SDIO is similar to PCIe, modify the high
latency path to allow SDIO modules to use the msdu id.
kvalo: the original patch from Alagu enabled this only for SDIO but I'm not
sure should we also enable this with USB. I'll use bus params to enable this
for so that it's easy to enable also for USB later.
Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
Co-developed-by: Wen Gong <wgong@codeaurora.org>
Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
Signed-off-by: Wen Gong <wgong@codeaurora.org>.
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/htt_tx.c | 13 ++++++++++++-
drivers/net/wireless/ath/ath10k/sdio.c | 2 ++
3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
index e08a17b01e03..f22cb3f49034 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -919,6 +919,7 @@ struct ath10k_bus_params {
u32 chip_id;
enum ath10k_dev_type dev_type;
bool link_can_suspend;
+ bool hl_msdu_ids;
};
struct ath10k {
diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index d8e9cc0bb772..88f880b601d8 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -1244,6 +1244,7 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
u8 tid = ath10k_htt_tx_get_tid(msdu, is_eth);
u8 flags0 = 0;
u16 flags1 = 0;
+ u16 msdu_id = 0;
data_len = msdu->len;
@@ -1291,6 +1292,16 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
}
}
+ if (ar->bus_param.hl_msdu_ids) {
+ flags1 |= HTT_DATA_TX_DESC_FLAGS1_POSTPONED;
+ res = ath10k_htt_tx_alloc_msdu_id(htt, msdu);
+ if (res < 0) {
+ ath10k_err(ar, "msdu_id allocation failed %d\n", res);
+ goto out;
+ }
+ msdu_id = res;
+ }
+
skb_push(msdu, sizeof(*cmd_hdr));
skb_push(msdu, sizeof(*tx_desc));
cmd_hdr = (struct htt_cmd_hdr *)msdu->data;
@@ -1300,7 +1311,7 @@ static int ath10k_htt_tx_hl(struct ath10k_htt *htt, enum ath10k_hw_txrx_mode txm
tx_desc->flags0 = flags0;
tx_desc->flags1 = __cpu_to_le16(flags1);
tx_desc->len = __cpu_to_le16(data_len);
- tx_desc->id = 0;
+ tx_desc->id = __cpu_to_le16(msdu_id);
tx_desc->frags_paddr = 0; /* always zero */
/* Initialize peer_id to INVALID_PEER because this is NOT
* Reinjection path
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index bbfdc4d7cf5f..d5073fac9509 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -2045,6 +2045,8 @@ static int ath10k_sdio_probe(struct sdio_func *func,
bus_params.dev_type = ATH10K_DEV_TYPE_HL;
/* TODO: don't know yet how to get chip_id with SDIO */
bus_params.chip_id = 0;
+ bus_params.hl_msdu_ids = true;
+
ret = ath10k_core_register(ar, &bus_params);
if (ret) {
ath10k_err(ar, "failed to register driver core: %d\n", ret);
^ permalink raw reply related
* [PATCH 4/6] ath10k: initialise struct ath10k_bus params to zero
From: Kalle Valo @ 2019-04-12 13:28 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
In-Reply-To: <155507569645.32018.15231567732563770250.stgit@potku.adurom.net>
This way we don't need to set every variable and give them to default, which is
zero. This is also safer in case we forgot to initalise a new field in some of
the bus modules.
Compile tested only.
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/ahb.c | 2 +-
drivers/net/wireless/ath/ath10k/pci.c | 2 +-
drivers/net/wireless/ath/ath10k/sdio.c | 2 +-
drivers/net/wireless/ath/ath10k/snoc.c | 2 +-
drivers/net/wireless/ath/ath10k/usb.c | 2 +-
5 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ahb.c b/drivers/net/wireless/ath/ath10k/ahb.c
index 0bf726c55736..f80854180e21 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -740,7 +740,7 @@ static int ath10k_ahb_probe(struct platform_device *pdev)
enum ath10k_hw_rev hw_rev;
size_t size;
int ret;
- struct ath10k_bus_params bus_params;
+ struct ath10k_bus_params bus_params = {};
of_id = of_match_device(ath10k_ahb_of_match, &pdev->dev);
if (!of_id) {
diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 271f92c24d44..13c24cd977d2 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -3478,7 +3478,7 @@ static int ath10k_pci_probe(struct pci_dev *pdev,
struct ath10k *ar;
struct ath10k_pci *ar_pci;
enum ath10k_hw_rev hw_rev;
- struct ath10k_bus_params bus_params;
+ struct ath10k_bus_params bus_params = {};
bool pci_ps;
int (*pci_soft_reset)(struct ath10k *ar);
int (*pci_hard_reset)(struct ath10k *ar);
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index d573d24624c0..bbfdc4d7cf5f 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -1954,7 +1954,7 @@ static int ath10k_sdio_probe(struct sdio_func *func,
struct ath10k *ar;
enum ath10k_hw_rev hw_rev;
u32 dev_id_base;
- struct ath10k_bus_params bus_params;
+ struct ath10k_bus_params bus_params = {};
int ret, i;
/* Assumption: All SDIO based chipsets (so far) are QCA6174 based.
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 873cb4ce419b..845914f2eaf8 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -1249,7 +1249,7 @@ static int ath10k_snoc_resource_init(struct ath10k *ar)
int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type)
{
struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
- struct ath10k_bus_params bus_params;
+ struct ath10k_bus_params bus_params = {};
int ret;
if (test_bit(ATH10K_SNOC_FLAG_UNREGISTERING, &ar_snoc->flags))
diff --git a/drivers/net/wireless/ath/ath10k/usb.c b/drivers/net/wireless/ath/ath10k/usb.c
index 970cf69ac35f..2fb0fb40e86d 100644
--- a/drivers/net/wireless/ath/ath10k/usb.c
+++ b/drivers/net/wireless/ath/ath10k/usb.c
@@ -973,7 +973,7 @@ static int ath10k_usb_probe(struct usb_interface *interface,
struct usb_device *dev = interface_to_usbdev(interface);
int ret, vendor_id, product_id;
enum ath10k_hw_rev hw_rev;
- struct ath10k_bus_params bus_params;
+ struct ath10k_bus_params bus_params = {};
/* Assumption: All USB based chipsets (so far) are QCA9377 based.
* If there will be newer chipsets that does not use the hw reg
^ permalink raw reply related
* [PATCH 3/6] ath10k: htt: don't use txdone_fifo with SDIO
From: Kalle Valo @ 2019-04-12 13:28 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
In-Reply-To: <155507569645.32018.15231567732563770250.stgit@potku.adurom.net>
From: Alagu Sankar <alagusankar@silex-india.com>
HTT High Latency (ATH10K_DEV_TYPE_HL) does not use txdone_fifo at all, we don't
even initialise it by skipping ath10k_htt_tx_alloc_buf() in
ath10k_htt_tx_start(). Because of this using QCA6174 SDIO
ath10k_htt_rx_tx_compl_ind() will crash when it accesses unitialised
txdone_fifo. So skip txdone_fifo when using High Latency mode.
Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
Co-developed-by: Wen Gong <wgong@codeaurora.org>
Signed-off-by: Alagu Sankar <alagusankar@silex-india.com>
Signed-off-by: Wen Gong <wgong@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a20ea270d519..8d13d0c3e85e 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2277,7 +2277,9 @@ static void ath10k_htt_rx_tx_compl_ind(struct ath10k *ar,
* Note that with only one concurrent reader and one concurrent
* writer, you don't need extra locking to use these macro.
*/
- if (!kfifo_put(&htt->txdone_fifo, tx_done)) {
+ if (ar->bus_param.dev_type == ATH10K_DEV_TYPE_HL) {
+ ath10k_txrx_tx_unref(htt, &tx_done);
+ } else if (!kfifo_put(&htt->txdone_fifo, tx_done)) {
ath10k_warn(ar, "txdone fifo overrun, msdu_id %d status %d\n",
tx_done.msdu_id, tx_done.status);
ath10k_txrx_tx_unref(htt, &tx_done);
^ permalink raw reply related
* [PATCH 2/6] ath10k: don't disable interrupts in ath10k_sdio_remove()
From: Kalle Valo @ 2019-04-12 13:28 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
In-Reply-To: <155507569645.32018.15231567732563770250.stgit@potku.adurom.net>
From: Wen Gong <wgong@codeaurora.org>
Disabling interrupts this early meant WMI communication was not working anymore
when the SDIO device was removed. But we call ath10k_core_unregister() that
will eventually call ath10k_sdio_hif_stop(), which disables the interrupts. So
there's actually no need to disable interrupts here.
Also remove cancel_work_sync() as it's also called in ath10k_sdio_hif_stop().
Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
Signed-off-by: Wen Gong <wgong@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/sdio.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
index fae56c67766f..d573d24624c0 100644
--- a/drivers/net/wireless/ath/ath10k/sdio.c
+++ b/drivers/net/wireless/ath/ath10k/sdio.c
@@ -2073,8 +2073,6 @@ static void ath10k_sdio_remove(struct sdio_func *func)
"sdio removed func %d vendor 0x%x device 0x%x\n",
func->num, func->vendor, func->device);
- (void)ath10k_sdio_hif_disable_intrs(ar);
- cancel_work_sync(&ar_sdio->wr_async_work);
ath10k_core_unregister(ar);
ath10k_core_destroy(ar);
}
^ permalink raw reply related
* [PATCH 1/6] ath10k: sdio: workaround firmware UART pin configuration bug
From: Kalle Valo @ 2019-04-12 13:28 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
From: Wen Gong <wgong@codeaurora.org>
On QCA6174 SDIO devices the SDIO interrupt will fail if UART is
disabled from ath10k. SDIO firmware enables UART printouts by
default. If ath10k will try to enable UART again the firmware
will configure it's GPIO line incorrectly and SDIO interrupts
won't work anymore. The workaround is to set UART pin again (19
for QCA6174 SDIO) if uart_print is 0.
Tested with QCA6174 SDIO with firmware WLAN.RMH.4.4.1-00007-QCARMSWP-1.
Signed-off-by: Wen Gong <wgong@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
drivers/net/wireless/ath/ath10k/core.c | 10 +++++++++-
drivers/net/wireless/ath/ath10k/hw.h | 5 +++++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 835b8de92d55..cfd7bb29a1ec 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2065,8 +2065,16 @@ static int ath10k_init_uart(struct ath10k *ar)
return ret;
}
- if (!uart_print)
+ if (!uart_print && ar->hw_params.uart_pin_workaround) {
+ ret = ath10k_bmi_write32(ar, hi_dbg_uart_txpin,
+ ar->hw_params.uart_pin);
+ if (ret) {
+ ath10k_warn(ar, "failed to set UART TX pin: %d", ret);
+ return ret;
+ }
+
return 0;
+ }
ret = ath10k_bmi_write32(ar, hi_dbg_uart_txpin, ar->hw_params.uart_pin);
if (ret) {
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 71314999aa24..a92fce1bb6f0 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -606,6 +606,11 @@ struct ath10k_hw_params {
/* target supporting fw download via diag ce */
bool fw_diag_ce_download;
+
+ /* need to set uart pin if disable uart print, workaround for a
+ * firmware bug
+ */
+ bool uart_pin_workaround;
};
struct htt_rx_desc;
^ permalink raw reply related
* [PATCH] mt76: fix tx power issues
From: Felix Fietkau @ 2019-04-12 13:20 UTC (permalink / raw)
To: linux-wireless
- tx power is stored in the channels after ieee80211_register_hw, so
chan->orig_mpwr needs to be updated as well
- for non-TSSI devices, mt76x2e needs to use a different target power value
from the EEPROM
- fix a rounding error in a few places (need to round up, not down)
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
drivers/net/wireless/mediatek/mt76/mac80211.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt7603/init.c | 1 +
drivers/net/wireless/mediatek/mt76/mt76x0/init.c | 1 +
drivers/net/wireless/mediatek/mt76/mt76x2/init.c | 12 +++---------
drivers/net/wireless/mediatek/mt76/mt76x2/phy.c | 6 +++---
5 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c
index f0d418b751ad..4b63d061c2a0 100644
--- a/drivers/net/wireless/mediatek/mt76/mac80211.c
+++ b/drivers/net/wireless/mediatek/mt76/mac80211.c
@@ -745,7 +745,7 @@ int mt76_get_txpower(struct ieee80211_hw *hw, struct ieee80211_vif *vif,
struct mt76_dev *dev = hw->priv;
int n_chains = hweight8(dev->antenna_mask);
- *dbm = dev->txpower_cur / 2;
+ *dbm = DIV_ROUND_UP(dev->txpower_cur, 2);
/* convert from per-chain power to combined
* output on 2x2 devices
diff --git a/drivers/net/wireless/mediatek/mt76/mt7603/init.c b/drivers/net/wireless/mediatek/mt76/mt7603/init.c
index a84b239887ca..569c61b9b5ae 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7603/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7603/init.c
@@ -488,6 +488,7 @@ mt7603_init_txpower(struct mt7603_dev *dev,
for (i = 0; i < sband->n_channels; i++) {
chan = &sband->channels[i];
chan->max_power = target_power;
+ chan->orig_mpwr = target_power;
}
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x0/init.c b/drivers/net/wireless/mediatek/mt76/mt76x0/init.c
index e5f4ce3b595b..57e46d57b449 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x0/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x0/init.c
@@ -280,6 +280,7 @@ mt76x0_init_txpower(struct mt76x02_dev *dev,
mt76x0_get_power_info(dev, chan, &tp);
chan->max_power = (mt76x02_get_max_rate_power(&t) + tp) / 2;
+ chan->orig_mpwr = chan->max_power;
}
}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/init.c b/drivers/net/wireless/mediatek/mt76/mt76x2/init.c
index a30ef2c5a9db..c6078e90ca43 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/init.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/init.c
@@ -165,27 +165,21 @@ void mt76x2_init_txpower(struct mt76x02_dev *dev,
struct ieee80211_channel *chan;
struct mt76x2_tx_power_info txp;
struct mt76_rate_power t = {};
- int target_power;
int i;
for (i = 0; i < sband->n_channels; i++) {
chan = &sband->channels[i];
mt76x2_get_power_info(dev, &txp, chan);
-
- target_power = max_t(int, (txp.chain[0].target_power +
- txp.chain[0].delta),
- (txp.chain[1].target_power +
- txp.chain[1].delta));
-
mt76x2_get_rate_power(dev, &t, chan);
chan->max_power = mt76x02_get_max_rate_power(&t) +
- target_power;
- chan->max_power /= 2;
+ txp.target_power;
+ chan->max_power = DIV_ROUND_UP(chan->max_power, 2);
/* convert to combined output power on 2x2 devices */
chan->max_power += 3;
+ chan->orig_mpwr = chan->max_power;
}
}
EXPORT_SYMBOL_GPL(mt76x2_init_txpower);
diff --git a/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c b/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c
index 769a9b972044..cdedf95ca4f5 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c
+++ b/drivers/net/wireless/mediatek/mt76/mt76x2/phy.c
@@ -161,12 +161,12 @@ void mt76x2_phy_set_txpower(struct mt76x02_dev *dev)
delta = txp.delta_bw80;
mt76x2_get_rate_power(dev, &t, chan);
- mt76x02_add_rate_power_offset(&t, txp.chain[0].target_power);
+ mt76x02_add_rate_power_offset(&t, txp.target_power + delta);
mt76x02_limit_rate_power(&t, dev->mt76.txpower_conf);
dev->mt76.txpower_cur = mt76x02_get_max_rate_power(&t);
base_power = mt76x2_get_min_rate_power(&t);
- delta += base_power - txp.chain[0].target_power;
+ delta = base_power - txp.target_power;
txp_0 = txp.chain[0].target_power + txp.chain[0].delta + delta;
txp_1 = txp.chain[1].target_power + txp.chain[1].delta + delta;
@@ -182,7 +182,7 @@ void mt76x2_phy_set_txpower(struct mt76x02_dev *dev)
}
mt76x02_add_rate_power_offset(&t, -base_power);
- dev->target_power = txp.chain[0].target_power;
+ dev->target_power = txp.target_power;
dev->target_power_delta[0] = txp_0 - txp.chain[0].target_power;
dev->target_power_delta[1] = txp_1 - txp.chain[0].target_power;
dev->mt76.rate_power = t;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH 6/6] ath10k: sdio: replace skb_trim with explicit set of skb->len
From: Kalle Valo @ 2019-04-12 13:17 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: linux-wireless, ath10k
In-Reply-To: <20190409190851.4557-7-erik.stromdahl@gmail.com>
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> This patch fixes a bug with padding of the skb data buffer.
> Since skb_trim can only be used to reduce the skb len, it is useless when
> we pad (increase the length of) the skb. Instead we must set skb->len
> directly.
>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
> ---
> drivers/net/wireless/ath/ath10k/sdio.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/sdio.c b/drivers/net/wireless/ath/ath10k/sdio.c
> index 3eb241cb8a25..989e3f563f3d 100644
> --- a/drivers/net/wireless/ath/ath10k/sdio.c
> +++ b/drivers/net/wireless/ath/ath10k/sdio.c
> @@ -1496,7 +1496,12 @@ static int ath10k_sdio_hif_tx_sg(struct ath10k *ar, u8 pipe_id,
> skb = items[i].transfer_context;
> padded_len = ath10k_sdio_calc_txrx_padded_len(ar_sdio,
> skb->len);
> - skb_trim(skb, padded_len);
> + /* FIXME: unsure if just extending the skb len is the right
> + * thing to do since we might read outside the skb->data
> + * buffer. But we really don't want to realloc the skb just to
> + * pad the length.
> + */
> + skb->len = padded_len;
Good catch! But I don't think you can modify skb->len directly like
that. There is skb_pad() but that doesn't change skb->len, so that most
likely needs more changes. So maybe skb_put() is the safest here?
--
Kalle Valo
^ permalink raw reply
* Re: [PATCH 3/6] ath10k: sdio: read RX packets in bundles
From: Kalle Valo @ 2019-04-12 13:08 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: linux-wireless, ath10k, Alagu Sankar
In-Reply-To: <20190409190851.4557-4-erik.stromdahl@gmail.com>
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> From: Alagu Sankar <alagusankar@silex-india.com>
>
> The existing implementation of initiating multiple sdio transfers for
> receive bundling is slowing down the receive speed.
>
> Instead of having one sdio transfer for each packet in the bundle, we
> read all packets in one sdio transfer.
>
> This results in significant performance improvement on some targets.
Do you have any numbers? Before and after would be nice to know.
> +static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
> {
> struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> + struct ath10k_sdio_rx_data *pkt = &ar_sdio->rx_pkts[0];
> struct sk_buff *skb = pkt->skb;
> int ret;
>
> - ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
> - skb->data, pkt->alloc_len);
> - pkt->status = ret;
> - if (!ret)
> + ret = ath10k_sdio_read(ar, ar_sdio->mbox_info.htc_addr,
> + skb->data, pkt->alloc_len);
> + if (ret) {
> + ar_sdio->n_rx_pkts = 0;
> + ath10k_sdio_mbox_free_rx_pkt(pkt);
> + } else {
> + pkt->status = ret;
> skb_put(skb, pkt->act_len);
> + }
With this you can avoid the else branch:
if (ret) {
ar_sdio->n_rx_pkts = 0;
ath10k_sdio_mbox_free_rx_pkt(pkt);
return ret;
}
> -static int ath10k_sdio_mbox_rx_fetch(struct ath10k *ar)
> +static int ath10k_sdio_mbox_rx_fetch_bundle(struct ath10k *ar)
> {
> struct ath10k_sdio *ar_sdio = ath10k_sdio_priv(ar);
> + struct ath10k_sdio_rx_data *pkt;
> int ret, i;
> + u32 pkt_offset = 0, pkt_bundle_len = 0;
> +
> + for (i = 0; i < ar_sdio->n_rx_pkts; i++)
> + pkt_bundle_len += ar_sdio->rx_pkts[i].alloc_len;
> +
> + if (pkt_bundle_len > ATH10K_SDIO_READ_BUF_SIZE) {
> + ret = -ENOMEM;
> + ath10k_err(ar, "bundle size (%d) exceeding limit %d\n",
> + pkt_bundle_len, ATH10K_SDIO_READ_BUF_SIZE);
As this is a recoverable case please use ath10k_warn(). And would
-ENOSPC be more descriptive error?
> + goto err;
> + }
> +
> + ret = ath10k_sdio_readsb(ar, ar_sdio->mbox_info.htc_addr,
> + ar_sdio->sdio_read_buf, pkt_bundle_len);
> + if (ret)
> + goto err;
>
> for (i = 0; i < ar_sdio->n_rx_pkts; i++) {
> - ret = ath10k_sdio_mbox_rx_packet(ar,
> - &ar_sdio->rx_pkts[i]);
> - if (ret)
> - goto err;
> + struct sk_buff *skb = ar_sdio->rx_pkts[i].skb;
> +
> + pkt = &ar_sdio->rx_pkts[i];
> + memcpy(skb->data, ar_sdio->sdio_read_buf + pkt_offset,
> + pkt->alloc_len);
> + pkt->status = 0;
> + skb_put(skb, pkt->act_len);
Shouldn't you call first skb_put() and then memcpy()?
--
Kalle Valo
^ permalink raw reply
* [PATCH] nl80211: add iftype mask when sending iftype_data
From: John Crispin @ 2019-04-12 12:54 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, John Crispin
Add the iftype mask to the blob passed to userland. This allows hostapd to
find the entry applicable for constructing the HE beacons of an AP.
Signed-off-by: John Crispin <john@phrozen.org>
---
include/uapi/linux/nl80211.h | 3 +++
net/wireless/nl80211.c | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index 84fa62bc2c7d..da6fb7624a11 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3330,6 +3330,8 @@ enum nl80211_mpath_info {
* capabilities IE
* @NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE: HE PPE thresholds information as
* defined in HE capabilities IE
+ * @NL80211_BAND_IFTYPE_ATTR_TYPE_MASK: The IFTYPEs that this element is
+ * applicable for
* @NL80211_BAND_IFTYPE_ATTR_MAX: highest band HE capability attribute currently
* defined
* @__NL80211_BAND_IFTYPE_ATTR_AFTER_LAST: internal use
@@ -3342,6 +3344,7 @@ enum nl80211_band_iftype_attr {
NL80211_BAND_IFTYPE_ATTR_HE_CAP_PHY,
NL80211_BAND_IFTYPE_ATTR_HE_CAP_MCS_SET,
NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE,
+ NL80211_BAND_IFTYPE_ATTR_TYPE_MASK,
/* keep last */
__NL80211_BAND_IFTYPE_ATTR_AFTER_LAST,
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 25a9e3b5c154..87d1127e8cc1 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1421,7 +1421,9 @@ nl80211_send_iftype_data(struct sk_buff *msg,
sizeof(he_cap->he_mcs_nss_supp),
&he_cap->he_mcs_nss_supp) ||
nla_put(msg, NL80211_BAND_IFTYPE_ATTR_HE_CAP_PPE,
- sizeof(he_cap->ppe_thres), he_cap->ppe_thres))
+ sizeof(he_cap->ppe_thres), he_cap->ppe_thres) ||
+ nla_put_u16(msg, NL80211_BAND_IFTYPE_ATTR_TYPE_MASK,
+ iftdata->types_mask))
return -ENOBUFS;
}
--
2.11.0
^ permalink raw reply related
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