* Re: [PATCH 2/2] Fix a NULL-ptr-deref bug in ath10k_usb_alloc_urb_from_pipe
From: Guenter Roeck @ 2019-08-31 21:31 UTC (permalink / raw)
To: Hui Peng
Cc: kvalo, davem, Mathias Payer, ath10k, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20190804003101.11541-1-benquike@gmail.com>
Hi,
On Sat, Aug 03, 2019 at 08:31:01PM -0400, Hui Peng wrote:
> The `ar_usb` field of `ath10k_usb_pipe_usb_pipe` objects
> are initialized to point to the containing `ath10k_usb` object
> according to endpoint descriptors read from the device side, as shown
> below in `ath10k_usb_setup_pipe_resources`:
>
> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
> endpoint = &iface_desc->endpoint[i].desc;
>
> // get the address from endpoint descriptor
> pipe_num = ath10k_usb_get_logical_pipe_num(ar_usb,
> endpoint->bEndpointAddress,
> &urbcount);
> ......
> // select the pipe object
> pipe = &ar_usb->pipes[pipe_num];
>
> // initialize the ar_usb field
> pipe->ar_usb = ar_usb;
> }
>
> The driver assumes that the addresses reported in endpoint
> descriptors from device side to be complete. If a device is
> malicious and does not report complete addresses, it may trigger
> NULL-ptr-deref `ath10k_usb_alloc_urb_from_pipe` and
> `ath10k_usb_free_urb_to_pipe`.
>
> This patch fixes the bug by preventing potential NULL-ptr-deref.
>
> Signed-off-by: Hui Peng <benquike@gmail.com>
> Reported-by: Hui Peng <benquike@gmail.com>
> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
This patch fixes CVE-2019-15099, which has CVSS scores of 7.5 (CVSS 3.0)
and 7.8 (CVSS 2.0). Yet, I don't find it in the upstream kernel or in Linux
next.
Is the patch going to be applied to the upstream kernel anytime soon ? If
not, is there reason to believe that its severity may not be as high as the
CVSS score indicates ?
Thanks,
Guenter
> ---
> drivers/net/wireless/ath/ath10k/usb.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/usb.c b/drivers/net/wireless/ath/ath10k/usb.c
> index e1420f67f776..14d86627b47f 100644
> --- a/drivers/net/wireless/ath/ath10k/usb.c
> +++ b/drivers/net/wireless/ath/ath10k/usb.c
> @@ -38,6 +38,10 @@ ath10k_usb_alloc_urb_from_pipe(struct ath10k_usb_pipe *pipe)
> struct ath10k_urb_context *urb_context = NULL;
> unsigned long flags;
>
> + /* bail if this pipe is not initialized */
> + if (!pipe->ar_usb)
> + return NULL;
> +
> spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags);
> if (!list_empty(&pipe->urb_list_head)) {
> urb_context = list_first_entry(&pipe->urb_list_head,
> @@ -55,6 +59,10 @@ static void ath10k_usb_free_urb_to_pipe(struct ath10k_usb_pipe *pipe,
> {
> unsigned long flags;
>
> + /* bail if this pipe is not initialized */
> + if (!pipe->ar_usb)
> + return NULL;
> +
> spin_lock_irqsave(&pipe->ar_usb->cs_lock, flags);
>
> pipe->urb_cnt++;
> --
> 2.22.0
>
^ permalink raw reply
* [PATCH] iwlwifi: mvm: Move static keyword to the front of declarations
From: Krzysztof Wilczynski @ 2019-08-31 22:01 UTC (permalink / raw)
To: Kalle Valo
Cc: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, David S. Miller, Sara Sharon,
Shaul Triebitz, Liad Kaufman, linux-wireless, netdev,
linux-kernel
Move the static keyword to the front of declarations of
he_if_types_ext_capa_sta and he_iftypes_ext_capa, and
resolve the following compiler warnings that can be seen
when building with warnings enabled (W=1):
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:427:1: warning:
‘static’ is not at beginning of declaration [-Wold-style-declaration]
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c:434:1: warning:
‘static’ is not at beginning of declaration [-Wold-style-declaration]
Signed-off-by: Krzysztof Wilczynski <kw@linux.com>
---
Related: https://lore.kernel.org/r/20190827233017.GK9987@google.com
drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
index d6499763f0dd..937a843fed56 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/mac80211.c
@@ -424,14 +424,14 @@ int iwl_mvm_init_fw_regd(struct iwl_mvm *mvm)
return ret;
}
-const static u8 he_if_types_ext_capa_sta[] = {
+static const u8 he_if_types_ext_capa_sta[] = {
[0] = WLAN_EXT_CAPA1_EXT_CHANNEL_SWITCHING,
[2] = WLAN_EXT_CAPA3_MULTI_BSSID_SUPPORT,
[7] = WLAN_EXT_CAPA8_OPMODE_NOTIF,
[9] = WLAN_EXT_CAPA10_TWT_REQUESTER_SUPPORT,
};
-const static struct wiphy_iftype_ext_capab he_iftypes_ext_capa[] = {
+static const struct wiphy_iftype_ext_capab he_iftypes_ext_capa[] = {
{
.iftype = NL80211_IFTYPE_STATION,
.extended_capabilities = he_if_types_ext_capa_sta,
--
2.22.1
^ permalink raw reply related
* Re: KASAN: use-after-free Read in rxrpc_put_peer
From: syzbot @ 2019-08-31 23:28 UTC (permalink / raw)
To: davem, dhowells, linux-afs, linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <21753.1567081045@warthog.procyon.org.uk>
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger
crash:
Reported-and-tested-by:
syzbot+b9be979c55f2bea8ed30@syzkaller.appspotmail.com
Tested on:
commit: 48b9e92a rxrpc: Fix lack of conn cleanup when local endpoi..
git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git
kernel config: https://syzkaller.appspot.com/x/.config?x=230542aa16bfc4b0
dashboard link: https://syzkaller.appspot.com/bug?extid=b9be979c55f2bea8ed30
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
Note: testing is done by a robot and is best-effort only.
^ permalink raw reply
* Re: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: Sasha Levin @ 2019-09-01 0:07 UTC (permalink / raw)
To: Matthieu Baerts
Cc: Tim Froidcoeur, aprout, cpaasch, davem, edumazet, gregkh,
jonathan.lemon, jtl, linux-kernel, mkubecek, ncardwell, stable,
ycheng, netdev
In-Reply-To: <35cd974b-2b31-4f86-d53d-8e9516d4077e@tessares.net>
On Sat, Aug 31, 2019 at 03:14:35PM +0200, Matthieu Baerts wrote:
>Hi Sasha,
>
>Thank you for your reply!
>
>On 31/08/2019 14:20, Sasha Levin wrote:
>> On Sat, Aug 24, 2019 at 08:03:51AM +0200, Tim Froidcoeur wrote:
>>> Commit 8c3088f895a0 ("tcp: be more careful in tcp_fragment()")
>>> triggers following stack trace:
>>>
>>> [25244.848046] kernel BUG at ./include/linux/skbuff.h:1406!
>>> [25244.859335] RIP: 0010:skb_queue_prev+0x9/0xc
>>> [25244.888167] Call Trace:
>>> [25244.889182] <IRQ>
>>> [25244.890001] tcp_fragment+0x9c/0x2cf
>>> [25244.891295] tcp_write_xmit+0x68f/0x988
>>> [25244.892732] __tcp_push_pending_frames+0x3b/0xa0
>>> [25244.894347] tcp_data_snd_check+0x2a/0xc8
>>> [25244.895775] tcp_rcv_established+0x2a8/0x30d
>>> [25244.897282] tcp_v4_do_rcv+0xb2/0x158
>>> [25244.898666] tcp_v4_rcv+0x692/0x956
>>> [25244.899959] ip_local_deliver_finish+0xeb/0x169
>>> [25244.901547] __netif_receive_skb_core+0x51c/0x582
>>> [25244.903193] ? inet_gro_receive+0x239/0x247
>>> [25244.904756] netif_receive_skb_internal+0xab/0xc6
>>> [25244.906395] napi_gro_receive+0x8a/0xc0
>>> [25244.907760] receive_buf+0x9a1/0x9cd
>>> [25244.909160] ? load_balance+0x17a/0x7b7
>>> [25244.910536] ? vring_unmap_one+0x18/0x61
>>> [25244.911932] ? detach_buf+0x60/0xfa
>>> [25244.913234] virtnet_poll+0x128/0x1e1
>>> [25244.914607] net_rx_action+0x12a/0x2b1
>>> [25244.915953] __do_softirq+0x11c/0x26b
>>> [25244.917269] ? handle_irq_event+0x44/0x56
>>> [25244.918695] irq_exit+0x61/0xa0
>>> [25244.919947] do_IRQ+0x9d/0xbb
>>> [25244.921065] common_interrupt+0x85/0x85
>>> [25244.922479] </IRQ>
>>>
>>> tcp_rtx_queue_tail() (called by tcp_fragment()) can call
>>> tcp_write_queue_prev() on the first packet in the queue, which will
>>> trigger
>>> the BUG in tcp_write_queue_prev(), because there is no previous packet.
>>>
>>> This happens when the retransmit queue is empty, for example in case of a
>>> zero window.
>>>
>>> Patch is needed for 4.4, 4.9 and 4.14 stable branches.
>>
>> There needs to be a better explanation of why it's not needed
>> upstream...
>
>Commit 8c3088f895a0 ("tcp: be more careful in tcp_fragment()") was not a
>simple cherry-pick of the original one from master (b617158dc096)
>because there is a specific TCP rtx queue only since v4.15. For more
>details, please see the commit message of b617158dc096 ("tcp: be more
>careful in tcp_fragment()").
>
>The BUG() is hit due to the specific code added to versions older than
>v4.15. The comment in skb_queue_prev() (include/linux/skbuff.h:1406),
>just before the BUG_ON() somehow suggests to add a check before using
>it, what Tim did.
>
>In master, this code path causing the issue will not be taken because
>the implementation of tcp_rtx_queue_tail() is different:
>
> tcp_fragment() → tcp_rtx_queue_tail() → tcp_write_queue_prev() →
>skb_queue_prev() → BUG_ON()
>
>Because this patch is specific to versions older than the two last
>stable ones but still linked to the network architecture, who can review
>and approve it? :)
Thanks for the explanation. I've changed the commit message to include
this explanation and queued it for 4.4, 4.9 and 4.14.
--
Thanks,
Sasha
^ permalink raw reply
* Re: [PATCH v2 net 0/3] Fix issues in tc-taprio and tc-cbs
From: David Miller @ 2019-09-01 1:46 UTC (permalink / raw)
To: olteanv
Cc: xiyou.wangcong, jiri, vinicius.gomes, vedang.patel,
leandro.maciel.dorileo, netdev
In-Reply-To: <20190830010723.32096-1-olteanv@gmail.com>
From: Vladimir Oltean <olteanv@gmail.com>
Date: Fri, 30 Aug 2019 04:07:20 +0300
> This series fixes one panic and one WARN_ON found in the tc-taprio
> qdisc, while trying to apply it:
>
> - On an interface which is not multi-queue
> - On an interface which has no carrier
>
> The tc-cbs was also visually found to suffer of the same issue as
> tc-taprio, and the fix was only compile-tested in that case.
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 net] net: Properly update v4 routes with v6 nexthop
From: David Ahern @ 2019-09-01 2:21 UTC (permalink / raw)
To: Donald Sharp, netdev, David Miller, sworley
In-Reply-To: <20190831122254.29928-1-sharpd@cumulusnetworks.com>
On 8/31/19 6:22 AM, Donald Sharp wrote:
> When creating a v4 route that uses a v6 nexthop from a nexthop group.
> Allow the kernel to properly send the nexthop as v6 via the RTA_VIA
> attribute.
>
Dave: I am on PTO with little time through Monday. I will review/test on
Tuesday.
^ permalink raw reply
* Re: [PATCH v2 0/2] net: dsa: microchip: add KSZ8563 support
From: David Miller @ 2019-09-01 6:36 UTC (permalink / raw)
To: razvan.stefanescu
Cc: woojung.huh, UNGLinuxDriver, andrew, vivien.didelot, f.fainelli,
netdev, linux-kernel
In-Reply-To: <20190830075202.20740-1-razvan.stefanescu@microchip.com>
From: Razvan Stefanescu <razvan.stefanescu@microchip.com>
Date: Fri, 30 Aug 2019 10:52:00 +0300
> This patchset adds compatibility string for the KSZ8563 switch.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 0/3] net: tls: add socket diag
From: David Miller @ 2019-09-01 6:44 UTC (permalink / raw)
To: dcaratti
Cc: borisp, jakub.kicinski, eric.dumazet, aviadye, davejwatson,
john.fastabend, matthieu.baerts, netdev
In-Reply-To: <cover.1567158431.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Fri, 30 Aug 2019 12:25:46 +0200
> The current kernel does not provide any diagnostic tool, except
> getsockopt(TCP_ULP), to know more about TCP sockets that have an upper
> layer protocol (ULP) on top of them. This series extends the set of
> information exported by INET_DIAG_INFO, to include data that are
> specific to the ULP (and that might be meaningful for debug/testing
> purposes).
...
Series applied, thanks Davide.
^ permalink raw reply
* Re: [PATCH net-next 0/3] dpaa2-eth: Add new statistics counters
From: David Miller @ 2019-09-01 6:42 UTC (permalink / raw)
To: jakub.kicinski; +Cc: ruxandra.radulescu, netdev, ioana.ciornei
In-Reply-To: <20190830231219.2363758a@cakuba.netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 30 Aug 2019 23:12:19 -0700
> On Fri, 30 Aug 2019 13:20:40 +0300, Ioana Radulescu wrote:
>> Recent firmware versions offer access to more DPNI statistics
>> counters. Add the relevant ones to ethtool interface stats.
>>
>> Also we can now make use of a new counter for in flight egress frames
>> to avoid sleeping an arbitrary amount of time in the ndo_stop routine.
>
> A little messy there in the comment of patch 2, and IMHO if you're
> expecting particular errors to be ignored it's better to write:
>
> if (err == -EOPNOTSUPP)
> /* still fine*/;
> else if (err)
> /* real err */
>
> than assume any error is for unsupported and add a extra comment
> explaining that things may be not supported.
Ioana, please address this feedback and respin your patchset.
Thank you.
^ permalink raw reply
* Re: [PATCH] net: bcmgenet: use ethtool_op_get_ts_info()
From: David Miller @ 2019-09-01 6:51 UTC (permalink / raw)
To: rmc032; +Cc: opendmb, f.fainelli, bcm-kernel-feedback-list, netdev,
linux-kernel
In-Reply-To: <20190830184955.GA27521@pop-os.localdomain>
From: "Ryan M. Collins" <rmc032@bucknell.edu>
Date: Fri, 30 Aug 2019 14:49:55 -0400
> This change enables the use of SW timestamping on the Raspberry Pi 4.
>
> bcmgenet's transmit function bcmgenet_xmit() implements software
> timestamping. However the SOF_TIMESTAMPING_TX_SOFTWARE capability was
> missing and only SOF_TIMESTAMPING_RX_SOFTWARE was announced. By using
> ethtool_ops bcmgenet_ethtool_ops() as get_ts_info(), the
> SOF_TIMESTAMPING_TX_SOFTWARE capability is announced.
>
> Similar to commit a8f5cb9e7991 ("smsc95xx: use ethtool_op_get_ts_info()")
>
> Signed-off-by: Ryan M. Collins <rmc032@bucknell.edu>
Applied.
^ permalink raw reply
* Re: [PATCH] enetc: Add missing call to 'pci_free_irq_vectors()' in probe and remove functions
From: David Miller @ 2019-09-01 6:53 UTC (permalink / raw)
To: christophe.jaillet
Cc: yangbo.lu, claudiu.manoil, netdev, linux-kernel, kernel-janitors
In-Reply-To: <20190830202312.21287-1-christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Fri, 30 Aug 2019 22:23:12 +0200
> Call to 'pci_free_irq_vectors()' are missing both in the error handling
> path of the probe function, and in the remove function.
> Add them.
>
> Fixes: 19971f5ea0ab ("enetc: add PTP clock driver")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH v2 net-next] net/ncsi: add response handlers for PLDM over NC-SI
From: David Miller @ 2019-09-01 6:54 UTC (permalink / raw)
To: benwei; +Cc: sam, netdev, linux-kernel, openbmc
In-Reply-To: <CH2PR15MB3686A26D4BB64D8959B519B6A3BD0@CH2PR15MB3686.namprd15.prod.outlook.com>
From: Ben Wei <benwei@fb.com>
Date: Fri, 30 Aug 2019 20:50:51 +0000
> This patch adds handlers for PLDM over NC-SI command response.
>
> This enables NC-SI driver recognizes the packet type so the responses
> don't get dropped as unknown packet type.
>
> PLDM over NC-SI are not handled in kernel driver for now, but can be
> passed back to user space via Netlink for further handling.
>
> Signed-off-by: Ben Wei <benwei@fb.com>
> ---
> Changes in v2
> - fix function definition spacing
Applied.
^ permalink raw reply
* Re: [PATCH v3 1/2] net: core: Notify on changes to dev->promiscuity.
From: Jiri Pirko @ 2019-09-01 6:54 UTC (permalink / raw)
To: Andrew Lunn
Cc: Ido Schimmel, David Miller, horatiu.vultur, alexandre.belloni,
UNGLinuxDriver, allan.nielsen, ivecera, f.fainelli, netdev,
linux-kernel
In-Reply-To: <20190831193556.GB2647@lunn.ch>
Sat, Aug 31, 2019 at 09:35:56PM CEST, andrew@lunn.ch wrote:
>> Also, what happens when I'm running these application without putting
>> the interface in promisc mode? On an offloaded interface I would not be
>> able to even capture packets addressed to my interface's MAC address.
>
>Sorry for rejoining the discussion late. I've been travelling and i'm
>now 3/4 of the way to Lisbon.
>
>That statement i don't get. If the frame has the MAC address of the
>interface, it has to be delivered to the CPU. And so pcap will see it
1) you cannot go to slowpath for all such packets (route)
2) you might be interested to examine packets with other dest mac too.
>when running on the interface. I can pretty much guarantee every DSA
>driver does that.
>
>But to address the bigger picture. My understanding is that we want to
>model offloading as a mechanism to accelerate what Linux can already
>do. The user should not have to care about these accelerators. The
>interface should work like a normal Linux interface. I can put an IP
>address on it and ping a peer. I can run a dhcp client and get an IP
>address from a dhcp server. I can add the interface to a bridge, and
>packets will get bridged. I as a user should not need to care if this
>is done in software, or accelerated by offloading it. I can add a
>route, and if the accelerate knows about L3, it can accelerate that as
>well. If not, the kernel will route it.
>
>So if i run wireshark on an interface, i expect the interface will be
>put into promisc mode and i see all packets ingressing the interface.
Again, you are merging 2 things together:
1) rx filter - this is needed for bridge, ovs, others (tc)
this is promisc setting
2) cpu trap - here one may be interested in:
a) only packets ASIC traps to CPU by default (ARPs, STP, BGP, etc)
b) all packets ingressing the port (note that those are only those
passed by rx filter)
Clearly 1) and 2) need separate knobs. In 2), there are valid usecases
for both a) and b). Only the user is the one who can tell which is he
interested in. This can't happen automagically.
Can we just have a knob for 2)?
>What the accelerator needs to do to achieve this, i as a user don't
>care.
>
>I can follow the argument that i won't necessarily see every
>packet. But that is always true. For many embedded systems, the CPU is
>too slow to receive at line rate, even when we are talking about 1G
>links. Packets do get dropped. And i hope tcpdump users understand
>that.
>
>For me, having tcpdump use tc trap is just wrong. It breaks the model
>that the user should not care about the accelerator. If anything, i
>think the driver needs to translate cBPF which pcap passes to the
>kernel to whatever internal format the accelerator can process. That
>is just another example of using hardware acceleration.
>
> Andrew
^ permalink raw reply
* Re: [PATCH net-next 0/2] Minor cleanup in devlink
From: David Miller @ 2019-09-01 6:46 UTC (permalink / raw)
To: jakub.kicinski; +Cc: parav, netdev, jiri
In-Reply-To: <20190830231436.34f221b2@cakuba.netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Fri, 30 Aug 2019 23:14:36 -0700
> On Fri, 30 Aug 2019 05:39:43 -0500, Parav Pandit wrote:
>> Two minor cleanup in devlink.
>>
>> Patch-1 Explicitly defines devlink port index as unsigned int
>> Patch-2 Uses switch-case to handle different port flavours attributes
>
> Always nice to see one's comment addressed, even if it takes a while :)
>
> Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Series applied.
^ permalink raw reply
* Re: [PATCH net] tc-testing: don't hardcode 'ip' in nsPlugin.py
From: David Miller @ 2019-09-01 6:47 UTC (permalink / raw)
To: dcaratti
Cc: liuhangbin, mrv, vladbu, lucasb, nicolas.dichtel, netdev,
marcelo.leitner
In-Reply-To: <8ade839e21c5231d2d6b8690b39587f802642306.1567180765.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Fri, 30 Aug 2019 18:51:47 +0200
> the following tdc test fails on Fedora:
>
> # ./tdc.py -e 2638
> -- ns/SubPlugin.__init__
> Test 2638: Add matchall and try to get it
> -----> prepare stage *** Could not execute: "$TC qdisc add dev $DEV1 clsact"
> -----> prepare stage *** Error message: "/bin/sh: ip: command not found"
> returncode 127; expected [0]
> -----> prepare stage *** Aborting test run.
>
> Let nsPlugin.py use the 'IP' variable introduced with commit 92c1a19e2fb9
> ("tc-tests: added path to ip command in tdc"), so that the path to 'ip' is
> correctly resolved to the value we have in tdc_config.py.
>
> # ./tdc.py -e 2638
> -- ns/SubPlugin.__init__
> Test 2638: Add matchall and try to get it
> All test results:
> 1..1
> ok 1 2638 - Add matchall and try to get it
>
> Fixes: 489ce2f42514 ("tc-testing: Restore original behaviour for namespaces in tdc")
> Reported-by: Hangbin Liu <liuhangbin@gmail.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied.
^ permalink raw reply
* Re: [PATCH v4 1/5] vsock/virtio: limit the memory used per-socket
From: Michael S. Tsirkin @ 2019-09-01 6:56 UTC (permalink / raw)
To: Stefano Garzarella
Cc: Stefan Hajnoczi, netdev, linux-kernel, David S. Miller,
virtualization, Jason Wang, kvm
In-Reply-To: <20190830094059.c7qo5cxrp2nkrncd@steredhat>
On Fri, Aug 30, 2019 at 11:40:59AM +0200, Stefano Garzarella wrote:
> On Mon, Jul 29, 2019 at 10:04:29AM -0400, Michael S. Tsirkin wrote:
> > On Wed, Jul 17, 2019 at 01:30:26PM +0200, Stefano Garzarella wrote:
> > > Since virtio-vsock was introduced, the buffers filled by the host
> > > and pushed to the guest using the vring, are directly queued in
> > > a per-socket list. These buffers are preallocated by the guest
> > > with a fixed size (4 KB).
> > >
> > > The maximum amount of memory used by each socket should be
> > > controlled by the credit mechanism.
> > > The default credit available per-socket is 256 KB, but if we use
> > > only 1 byte per packet, the guest can queue up to 262144 of 4 KB
> > > buffers, using up to 1 GB of memory per-socket. In addition, the
> > > guest will continue to fill the vring with new 4 KB free buffers
> > > to avoid starvation of other sockets.
> > >
> > > This patch mitigates this issue copying the payload of small
> > > packets (< 128 bytes) into the buffer of last packet queued, in
> > > order to avoid wasting memory.
> > >
> > > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> >
> > This is good enough for net-next, but for net I think we
> > should figure out how to address the issue completely.
> > Can we make the accounting precise? What happens to
> > performance if we do?
> >
>
> Since I'm back from holidays, I'm restarting this thread to figure out
> how to address the issue completely.
>
> I did a better analysis of the credit mechanism that we implemented in
> virtio-vsock to get a clearer view and I'd share it with you:
>
> This issue affect only the "host->guest" path. In this case, when the
> host wants to send a packet to the guest, it uses a "free" buffer
> allocated by the guest (4KB).
> The "free" buffers available for the host are shared between all
> sockets, instead, the credit mechanism is per-socket, I think to
> avoid the starvation of others sockets.
> The guests re-fill the "free" queue when the available buffers are
> less than half.
>
> Each peer have these variables in the per-socket state:
> /* local vars */
> buf_alloc /* max bytes usable by this socket
> [exposed to the other peer] */
> fwd_cnt /* increased when RX packet is consumed by the
> user space [exposed to the other peer] */
> tx_cnt /* increased when TX packet is sent to the other peer */
>
> /* remote vars */
> peer_buf_alloc /* peer's buf_alloc */
> peer_fwd_cnt /* peer's fwd_cnt */
>
> When a peer sends a packet, it increases the 'tx_cnt'; when the
> receiver consumes the packet (copy it to the user-space buffer), it
> increases the 'fwd_cnt'.
> Note: increments are made considering the payload length and not the
> buffer length.
>
> The value of 'buf_alloc' and 'fwd_cnt' are sent to the other peer in
> all packet headers or with an explicit CREDIT_UPDATE packet.
>
> The local 'buf_alloc' value can be modified by the user space using
> setsockopt() with optname=SO_VM_SOCKETS_BUFFER_SIZE.
>
> Before to send a packet, the peer checks the space available:
> credit_available = peer_buf_alloc - (tx_cnt - peer_fwd_cnt)
> and it will send up to credit_available bytes to the other peer.
>
> Possible solutions considering Michael's advice:
> 1. Use the buffer length instead of the payload length when we increment
> the counters:
> - This approach will account precisely the memory used per socket.
> - This requires changes in both guest and host.
> - It is not compatible with old drivers, so a feature should be negotiated.
> 2. Decrease the advertised 'buf_alloc' taking count of bytes queued in
> the socket queue but not used. (e.g. 256 byte used on 4K available in
> the buffer)
> - pkt->hdr.buf_alloc = buf_alloc - bytes_not_used.
> - This should be compatible also with old drivers.
>
> Maybe the second is less invasive, but will it be too tricky?
> Any other advice or suggestions?
>
> Thanks in advance,
> Stefano
OK let me try to clarify. The idea is this:
Let's say we queue a buffer of 4K, and we copy if len < 128 bytes. This
means that in the worst case (128 byte packets), each byte of credit in
the socket uses up 4K/128 = 16 bytes of kernel memory. In fact we need
to also account for the virtio_vsock_pkt since I think it's kept around
until userspace consumes it.
Thus given X buf alloc allowed in the socket, we should publish X/16
credits to the other side. This will ensure the other side does not send
more than X/16 bytes for a given socket and thus we won't need to
allocate more than X bytes to hold the data.
We can play with the copy break value to tweak this.
^ permalink raw reply
* Re: [PATCH net 7/7] rxrpc: Use skb_unshare() rather than skb_cow_data()
From: David Howells @ 2019-09-01 7:11 UTC (permalink / raw)
To: Hillf Danton; +Cc: dhowells, netdev, linux-afs, linux-kernel
In-Reply-To: <20190901065603.432-1-hdanton@sina.com>
Hillf Danton <hdanton@sina.com> wrote:
> > + /* Unshare the packet so that it can be modified for in-place
> > + * decryption.
> > + */
> > + if (sp->hdr.securityIndex != 0) {
> > + struct sk_buff *nskb = skb_unshare(skb, GFP_ATOMIC);
> > + if (!nskb) {
> > + rxrpc_eaten_skb(skb, rxrpc_skb_unshared_nomem);
> > + goto out;
> > + }
> > +
> > + if (nskb != skb) {
> > + rxrpc_eaten_skb(skb, rxrpc_skb_received);
> > + rxrpc_new_skb(skb, rxrpc_skb_unshared);
> > + skb = nskb;
> > + sp = rxrpc_skb(skb);
> > + }
> > + }
>
> Unsharing skb makes it perilous to take a peep at it afterwards.
Ah, good point. rxrpc_new_skb() should be after the assignment.
David
^ permalink raw reply
* Re: [PATCH net] rxrpc: Fix lack of conn cleanup when local endpoint is cleaned up [ver #2]
From: David Howells @ 2019-09-01 7:14 UTC (permalink / raw)
To: Hillf Danton; +Cc: dhowells, netdev, marc.dionne, linux-afs, linux-kernel
In-Reply-To: <20190901020519.2392-1-hdanton@sina.com>
Hillf Danton <hdanton@sina.com> wrote:
> > It's certainly possible that that can happen. The reaper is per
> > network-namespace.
> >
> > conn->params.local holds a ref on the local endpoint.
> >
> Then local endpoint can not become dead without connection reaper
> running first, because of the ref held by connection. When it is
> dead, however, there is no need to run reaper directly (rather than
> through a workqueue).
The reaper is per-net_ns, not per-local. There may be more than one local
endpoint in a net_ns and they share the list of service connections.
David
^ permalink raw reply
* [PATCHv2 0/1] Fix deadlock problem and make performance better
From: Zhu Yanjun @ 2019-09-01 7:26 UTC (permalink / raw)
To: eric.dumazet, davem, netdev
When running with about 1Gbit/ses for very long time, running ifconfig
and netstat causes dead lock. These symptoms are similar to the
commit 5f6b4e14cada ("net: dsa: User per-cpu 64-bit statistics"). After
replacing network devices statistics with per-cpu 64-bit statistics,
the dead locks disappear even after very long time running with 1Gbit/sec.
Based on Eric's advice, "If the loops are ever restarted, the
storage->fields will have been modified multiple times.".
A similar change in the commit 5f6b4e14cada ("net: dsa: User per-cpu
64-bit statistics") is borrowed to fix the above problem.
Zhu Yanjun (1):
forcedeth: use per cpu to collect xmit/recv statistics
drivers/net/ethernet/nvidia/forcedeth.c | 143 ++++++++++++++++++++++----------
1 file changed, 99 insertions(+), 44 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCHv2 1/1] forcedeth: use per cpu to collect xmit/recv statistics
From: Zhu Yanjun @ 2019-09-01 7:26 UTC (permalink / raw)
To: eric.dumazet, davem, netdev
In-Reply-To: <1567322773-5183-1-git-send-email-yanjun.zhu@oracle.com>
When testing with a background iperf pushing 1Gbit/sec traffic and running
both ifconfig and netstat to collect statistics, some deadlocks occurred.
Ifconfig and netstat will call nv_get_stats64 to get software xmit/recv
statistics. In the commit f5d827aece36 ("forcedeth: implement
ndo_get_stats64() API"), the normal tx/rx variables is to collect tx/rx
statistics. The fix is to replace normal tx/rx variables with per
cpu 64-bit variable to collect xmit/recv statistics. The per cpu variable
will avoid deadlocks and provide fast efficient statistics updates.
In nv_probe, the per cpu variable is initialized. In nv_remove, this
per cpu variable is freed.
In xmit/recv process, this per cpu variable will be updated.
In nv_get_stats64, this per cpu variable on each cpu is added up. Then
the driver can get xmit/recv packets statistics.
A test runs for several days with this commit, the deadlocks disappear
and the performance is better.
Tested:
- iperf SMP x86_64 ->
Client connecting to 1.1.1.108, TCP port 5001
TCP window size: 85.0 KByte (default)
------------------------------------------------------------
[ 3] local 1.1.1.105 port 38888 connected with 1.1.1.108 port 5001
[ ID] Interval Transfer Bandwidth
[ 3] 0.0-10.0 sec 1.10 GBytes 943 Mbits/sec
ifconfig results:
enp0s9 Link encap:Ethernet HWaddr 00:21:28:6f:de:0f
inet addr:1.1.1.105 Bcast:0.0.0.0 Mask:255.255.255.0
UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
RX packets:5774764531 errors:0 dropped:0 overruns:0 frame:0
TX packets:633534193 errors:0 dropped:0 overruns:0 carrier:0
collisions:0 txqueuelen:1000
RX bytes:7646159340904 (7.6 TB) TX bytes:11425340407722 (11.4 TB)
netstat results:
Kernel Interface table
Iface MTU Met RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
...
enp0s9 1500 0 5774764531 0 0 0 633534193 0 0 0 BMRU
...
Fixes: f5d827aece36 ("forcedeth: implement ndo_get_stats64() API")
CC: Joe Jin <joe.jin@oracle.com>
CC: JUNXIAO_BI <junxiao.bi@oracle.com>
Reported-and-tested-by: Nan san <nan.1986san@gmail.com>
Signed-off-by: Zhu Yanjun <yanjun.zhu@oracle.com>
---
V1->V2: Following Eric's advice fix the problem "If the loops are ever
restarted, the storage->fields will have been modified multiple
times."
---
drivers/net/ethernet/nvidia/forcedeth.c | 143 ++++++++++++++++++++++----------
1 file changed, 99 insertions(+), 44 deletions(-)
diff --git a/drivers/net/ethernet/nvidia/forcedeth.c b/drivers/net/ethernet/nvidia/forcedeth.c
index b327b29..07dd017 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -713,6 +713,21 @@ struct nv_skb_map {
struct nv_skb_map *next_tx_ctx;
};
+struct nv_txrx_stats {
+ u64 stat_rx_packets;
+ u64 stat_rx_bytes; /* not always available in HW */
+ u64 stat_rx_missed_errors;
+ u64 stat_rx_dropped;
+ u64 stat_tx_packets; /* not always available in HW */
+ u64 stat_tx_bytes;
+ u64 stat_tx_dropped;
+};
+
+#define nv_txrx_stats_inc(member) \
+ __this_cpu_inc(np->txrx_stats->member)
+#define nv_txrx_stats_add(member, count) \
+ __this_cpu_add(np->txrx_stats->member, (count))
+
/*
* SMP locking:
* All hardware access under netdev_priv(dev)->lock, except the performance
@@ -797,10 +812,7 @@ struct fe_priv {
/* RX software stats */
struct u64_stats_sync swstats_rx_syncp;
- u64 stat_rx_packets;
- u64 stat_rx_bytes; /* not always available in HW */
- u64 stat_rx_missed_errors;
- u64 stat_rx_dropped;
+ struct nv_txrx_stats __percpu *txrx_stats;
/* media detection workaround.
* Locking: Within irq hander or disable_irq+spin_lock(&np->lock);
@@ -826,9 +838,6 @@ struct fe_priv {
/* TX software stats */
struct u64_stats_sync swstats_tx_syncp;
- u64 stat_tx_packets; /* not always available in HW */
- u64 stat_tx_bytes;
- u64 stat_tx_dropped;
/* msi/msi-x fields */
u32 msi_flags;
@@ -1721,6 +1730,39 @@ static void nv_update_stats(struct net_device *dev)
}
}
+static inline void nv_get_stats(int cpu, struct fe_priv *np,
+ struct rtnl_link_stats64 *storage)
+{
+ struct nv_txrx_stats *src = per_cpu_ptr(np->txrx_stats, cpu);
+ unsigned int syncp_start;
+ u64 rx_packets, rx_bytes, rx_dropped, rx_missed_errors;
+ u64 tx_packets, tx_bytes, tx_dropped;
+
+ do {
+ syncp_start = u64_stats_fetch_begin_irq(&np->swstats_rx_syncp);
+ rx_packets = src->stat_rx_packets;
+ rx_bytes = src->stat_rx_bytes;
+ rx_dropped = src->stat_rx_dropped;
+ rx_missed_errors = src->stat_rx_missed_errors;
+ } while (u64_stats_fetch_retry_irq(&np->swstats_rx_syncp, syncp_start));
+
+ storage->rx_packets += rx_packets;
+ storage->rx_bytes += rx_bytes;
+ storage->rx_dropped += rx_dropped;
+ storage->rx_missed_errors += rx_missed_errors;
+
+ do {
+ syncp_start = u64_stats_fetch_begin_irq(&np->swstats_tx_syncp);
+ tx_packets = src->stat_tx_packets;
+ tx_bytes = src->stat_tx_bytes;
+ tx_dropped = src->stat_tx_dropped;
+ } while (u64_stats_fetch_retry_irq(&np->swstats_tx_syncp, syncp_start));
+
+ storage->tx_packets += tx_packets;
+ storage->tx_bytes += tx_bytes;
+ storage->tx_dropped += tx_dropped;
+}
+
/*
* nv_get_stats64: dev->ndo_get_stats64 function
* Get latest stats value from the nic.
@@ -1733,7 +1775,7 @@ nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
__releases(&netdev_priv(dev)->hwstats_lock)
{
struct fe_priv *np = netdev_priv(dev);
- unsigned int syncp_start;
+ int cpu;
/*
* Note: because HW stats are not always available and for
@@ -1746,20 +1788,8 @@ nv_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *storage)
*/
/* software stats */
- do {
- syncp_start = u64_stats_fetch_begin_irq(&np->swstats_rx_syncp);
- storage->rx_packets = np->stat_rx_packets;
- storage->rx_bytes = np->stat_rx_bytes;
- storage->rx_dropped = np->stat_rx_dropped;
- storage->rx_missed_errors = np->stat_rx_missed_errors;
- } while (u64_stats_fetch_retry_irq(&np->swstats_rx_syncp, syncp_start));
-
- do {
- syncp_start = u64_stats_fetch_begin_irq(&np->swstats_tx_syncp);
- storage->tx_packets = np->stat_tx_packets;
- storage->tx_bytes = np->stat_tx_bytes;
- storage->tx_dropped = np->stat_tx_dropped;
- } while (u64_stats_fetch_retry_irq(&np->swstats_tx_syncp, syncp_start));
+ for_each_online_cpu(cpu)
+ nv_get_stats(cpu, np, storage);
/* If the nic supports hw counters then retrieve latest values */
if (np->driver_data & DEV_HAS_STATISTICS_V123) {
@@ -1827,7 +1857,7 @@ static int nv_alloc_rx(struct net_device *dev)
} else {
packet_dropped:
u64_stats_update_begin(&np->swstats_rx_syncp);
- np->stat_rx_dropped++;
+ nv_txrx_stats_inc(stat_rx_dropped);
u64_stats_update_end(&np->swstats_rx_syncp);
return 1;
}
@@ -1869,7 +1899,7 @@ static int nv_alloc_rx_optimized(struct net_device *dev)
} else {
packet_dropped:
u64_stats_update_begin(&np->swstats_rx_syncp);
- np->stat_rx_dropped++;
+ nv_txrx_stats_inc(stat_rx_dropped);
u64_stats_update_end(&np->swstats_rx_syncp);
return 1;
}
@@ -2013,7 +2043,7 @@ static void nv_drain_tx(struct net_device *dev)
}
if (nv_release_txskb(np, &np->tx_skb[i])) {
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_dropped++;
+ nv_txrx_stats_inc(stat_tx_dropped);
u64_stats_update_end(&np->swstats_tx_syncp);
}
np->tx_skb[i].dma = 0;
@@ -2227,7 +2257,7 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
/* on DMA mapping error - drop the packet */
dev_kfree_skb_any(skb);
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_dropped++;
+ nv_txrx_stats_inc(stat_tx_dropped);
u64_stats_update_end(&np->swstats_tx_syncp);
return NETDEV_TX_OK;
}
@@ -2273,7 +2303,7 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, struct net_device *dev)
dev_kfree_skb_any(skb);
np->put_tx_ctx = start_tx_ctx;
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_dropped++;
+ nv_txrx_stats_inc(stat_tx_dropped);
u64_stats_update_end(&np->swstats_tx_syncp);
return NETDEV_TX_OK;
}
@@ -2384,7 +2414,7 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
/* on DMA mapping error - drop the packet */
dev_kfree_skb_any(skb);
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_dropped++;
+ nv_txrx_stats_inc(stat_tx_dropped);
u64_stats_update_end(&np->swstats_tx_syncp);
return NETDEV_TX_OK;
}
@@ -2431,7 +2461,7 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff *skb,
dev_kfree_skb_any(skb);
np->put_tx_ctx = start_tx_ctx;
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_dropped++;
+ nv_txrx_stats_inc(stat_tx_dropped);
u64_stats_update_end(&np->swstats_tx_syncp);
return NETDEV_TX_OK;
}
@@ -2560,9 +2590,12 @@ static int nv_tx_done(struct net_device *dev, int limit)
&& !(flags & NV_TX_RETRYCOUNT_MASK))
nv_legacybackoff_reseed(dev);
} else {
+ unsigned int len;
+
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_packets++;
- np->stat_tx_bytes += np->get_tx_ctx->skb->len;
+ nv_txrx_stats_inc(stat_tx_packets);
+ len = np->get_tx_ctx->skb->len;
+ nv_txrx_stats_add(stat_tx_bytes, len);
u64_stats_update_end(&np->swstats_tx_syncp);
}
bytes_compl += np->get_tx_ctx->skb->len;
@@ -2577,9 +2610,12 @@ static int nv_tx_done(struct net_device *dev, int limit)
&& !(flags & NV_TX2_RETRYCOUNT_MASK))
nv_legacybackoff_reseed(dev);
} else {
+ unsigned int len;
+
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_packets++;
- np->stat_tx_bytes += np->get_tx_ctx->skb->len;
+ nv_txrx_stats_inc(stat_tx_packets);
+ len = np->get_tx_ctx->skb->len;
+ nv_txrx_stats_add(stat_tx_bytes, len);
u64_stats_update_end(&np->swstats_tx_syncp);
}
bytes_compl += np->get_tx_ctx->skb->len;
@@ -2627,9 +2663,12 @@ static int nv_tx_done_optimized(struct net_device *dev, int limit)
nv_legacybackoff_reseed(dev);
}
} else {
+ unsigned int len;
+
u64_stats_update_begin(&np->swstats_tx_syncp);
- np->stat_tx_packets++;
- np->stat_tx_bytes += np->get_tx_ctx->skb->len;
+ nv_txrx_stats_inc(stat_tx_packets);
+ len = np->get_tx_ctx->skb->len;
+ nv_txrx_stats_add(stat_tx_bytes, len);
u64_stats_update_end(&np->swstats_tx_syncp);
}
@@ -2806,6 +2845,15 @@ static int nv_getlen(struct net_device *dev, void *packet, int datalen)
}
}
+static inline void rx_missing_handler(u32 flags, struct fe_priv *np)
+{
+ if (flags & NV_RX_MISSEDFRAME) {
+ u64_stats_update_begin(&np->swstats_rx_syncp);
+ nv_txrx_stats_inc(stat_rx_missed_errors);
+ u64_stats_update_end(&np->swstats_rx_syncp);
+ }
+}
+
static int nv_rx_process(struct net_device *dev, int limit)
{
struct fe_priv *np = netdev_priv(dev);
@@ -2848,11 +2896,7 @@ static int nv_rx_process(struct net_device *dev, int limit)
}
/* the rest are hard errors */
else {
- if (flags & NV_RX_MISSEDFRAME) {
- u64_stats_update_begin(&np->swstats_rx_syncp);
- np->stat_rx_missed_errors++;
- u64_stats_update_end(&np->swstats_rx_syncp);
- }
+ rx_missing_handler(flags, np);
dev_kfree_skb(skb);
goto next_pkt;
}
@@ -2896,8 +2940,8 @@ static int nv_rx_process(struct net_device *dev, int limit)
skb->protocol = eth_type_trans(skb, dev);
napi_gro_receive(&np->napi, skb);
u64_stats_update_begin(&np->swstats_rx_syncp);
- np->stat_rx_packets++;
- np->stat_rx_bytes += len;
+ nv_txrx_stats_inc(stat_rx_packets);
+ nv_txrx_stats_add(stat_rx_bytes, len);
u64_stats_update_end(&np->swstats_rx_syncp);
next_pkt:
if (unlikely(np->get_rx.orig++ == np->last_rx.orig))
@@ -2982,8 +3026,8 @@ static int nv_rx_process_optimized(struct net_device *dev, int limit)
}
napi_gro_receive(&np->napi, skb);
u64_stats_update_begin(&np->swstats_rx_syncp);
- np->stat_rx_packets++;
- np->stat_rx_bytes += len;
+ nv_txrx_stats_inc(stat_rx_packets);
+ nv_txrx_stats_add(stat_rx_bytes, len);
u64_stats_update_end(&np->swstats_rx_syncp);
} else {
dev_kfree_skb(skb);
@@ -5651,6 +5695,12 @@ static int nv_probe(struct pci_dev *pci_dev, const struct pci_device_id *id)
SET_NETDEV_DEV(dev, &pci_dev->dev);
u64_stats_init(&np->swstats_rx_syncp);
u64_stats_init(&np->swstats_tx_syncp);
+ np->txrx_stats = alloc_percpu(struct nv_txrx_stats);
+ if (!np->txrx_stats) {
+ pr_err("np->txrx_stats, alloc memory error.\n");
+ err = -ENOMEM;
+ goto out_alloc_percpu;
+ }
timer_setup(&np->oom_kick, nv_do_rx_refill, 0);
timer_setup(&np->nic_poll, nv_do_nic_poll, 0);
@@ -6060,6 +6110,8 @@ static int nv_probe(struct pci_dev *pci_dev, const struct pci_device_id *id)
out_disable:
pci_disable_device(pci_dev);
out_free:
+ free_percpu(np->txrx_stats);
+out_alloc_percpu:
free_netdev(dev);
out:
return err;
@@ -6105,6 +6157,9 @@ static void nv_restore_mac_addr(struct pci_dev *pci_dev)
static void nv_remove(struct pci_dev *pci_dev)
{
struct net_device *dev = pci_get_drvdata(pci_dev);
+ struct fe_priv *np = netdev_priv(dev);
+
+ free_percpu(np->txrx_stats);
unregister_netdev(dev);
--
2.7.4
^ permalink raw reply related
* Re: [patch net-next v2] mlx5: Add missing init_net check in FIB notifier
From: Roi Dayan @ 2019-09-01 7:38 UTC (permalink / raw)
To: Jiri Pirko, netdev@vger.kernel.org
Cc: davem@davemloft.net, Saeed Mahameed, leon@kernel.org, mlxsw
In-Reply-To: <20190830082530.8958-1-jiri@resnulli.us>
On 2019-08-30 11:25 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Take only FIB events that are happening in init_net into account. No other
> namespaces are supported.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> v1->v2:
> - no change, just cced maintainers (fat finger made me avoid them in v1)
> ---
> drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
> index e69766393990..5d20d615663e 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/lag_mp.c
> @@ -248,6 +248,9 @@ static int mlx5_lag_fib_event(struct notifier_block *nb,
> struct net_device *fib_dev;
> struct fib_info *fi;
>
> + if (!net_eq(info->net, &init_net))
> + return NOTIFY_DONE;
> +
> if (info->family != AF_INET)
> return NOTIFY_DONE;
>
>
thanks
Acked-by: Roi Dayan <roid@mellanox.com>
^ permalink raw reply
* Re: [PATCH 1/2] Fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe
From: Kalle Valo @ 2019-09-01 7:55 UTC (permalink / raw)
To: Guenter Roeck
Cc: Hui Peng, davem, Mathias Payer, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20190831180219.GA20860@roeck-us.net>
Guenter Roeck <linux@roeck-us.net> writes:
> On Sat, Aug 03, 2019 at 08:29:04PM -0400, Hui Peng wrote:
>> The `ar_usb` field of `ath6kl_usb_pipe_usb_pipe` objects
>> are initialized to point to the containing `ath6kl_usb` object
>> according to endpoint descriptors read from the device side, as shown
>> below in `ath6kl_usb_setup_pipe_resources`:
>>
>> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
>> endpoint = &iface_desc->endpoint[i].desc;
>>
>> // get the address from endpoint descriptor
>> pipe_num = ath6kl_usb_get_logical_pipe_num(ar_usb,
>> endpoint->bEndpointAddress,
>> &urbcount);
>> ......
>> // select the pipe object
>> pipe = &ar_usb->pipes[pipe_num];
>>
>> // initialize the ar_usb field
>> pipe->ar_usb = ar_usb;
>> }
>>
>> The driver assumes that the addresses reported in endpoint
>> descriptors from device side to be complete. If a device is
>> malicious and does not report complete addresses, it may trigger
>> NULL-ptr-deref `ath6kl_usb_alloc_urb_from_pipe` and
>> `ath6kl_usb_free_urb_to_pipe`.
>>
>> This patch fixes the bug by preventing potential NULL-ptr-deref.
>>
>> Signed-off-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>
> I don't see this patch in the upstream kernel or in -next.
>
> At the same time, it is supposed to fix CVE-2019-15098, which has
> a CVSS v2.0 score of 7.8 (high).
>
> Is this patch going to be applied to the upstream kernel ?
Lately I have been very busy and I have not had a chance to apply ath6kl
nor ath10k patches. This patch is on my queue and my plan is to go
through my patch queue next week.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 1/2] Fix a NULL-ptr-deref bug in ath6kl_usb_alloc_urb_from_pipe
From: Kalle Valo @ 2019-09-01 7:58 UTC (permalink / raw)
To: Hui Peng
Cc: Guenter Roeck, David S. Miller, Mathias Payer, linux-wireless,
netdev, linux-kernel
In-Reply-To: <CAKpmkkXyhuTviRfJG9dG-=Pt0KKdoHaxhXdvW9tSadOoKfnP1w@mail.gmail.com>
Hui Peng <benquike@gmail.com> writes:
> The reason that this patch is still in the pending state is that it
> has not reviewed by maintainers (they are not responding).
> @Greg: can we apply it?
Who is "we" in this case? But anyway, I'll review the patch and if it's
ok I'll take it through my ath.git tree, as normal.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] Fix a double free bug in rsi_91x_deinit
From: Kalle Valo @ 2019-09-01 8:03 UTC (permalink / raw)
To: Guenter Roeck
Cc: Hui Peng, security, Mathias Payer, David S. Miller,
linux-wireless, netdev, linux-kernel
In-Reply-To: <20190831181852.GA22160@roeck-us.net>
Guenter Roeck <linux@roeck-us.net> writes:
> On Mon, Aug 19, 2019 at 06:02:29PM -0400, Hui Peng wrote:
>> `dev` (struct rsi_91x_usbdev *) field of adapter
>> (struct rsi_91x_usbdev *) is allocated and initialized in
>> `rsi_init_usb_interface`. If any error is detected in information
>> read from the device side, `rsi_init_usb_interface` will be
>> freed. However, in the higher level error handling code in
>> `rsi_probe`, if error is detected, `rsi_91x_deinit` is called
>> again, in which `dev` will be freed again, resulting double free.
>>
>> This patch fixes the double free by removing the free operation on
>> `dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
>> used in `rsi_disconnect`, in that code path, the `dev` field is not
>> (and thus needs to be) freed.
>>
>> This bug was found in v4.19, but is also present in the latest version
>> of kernel.
>>
>> Reported-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>> Signed-off-by: Hui Peng <benquike@gmail.com>
>
> FWIW:
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> This patch is listed as fix for CVE-2019-15504, which has a CVSS 2.0 score
> of 10.0 (high) and CVSS 3.0 score of 9.8 (critical).
A double free in error path is considered as a critical CVE issue? I'm
very curious, why is that?
> Are there any plans to apply this patch to the upstream kernel anytime
> soon ?
I was on vacation last week and hence I have not been able to apply any
wireless patches. I should be able to catch up next week.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 2/2] Fix a NULL-ptr-deref bug in ath10k_usb_alloc_urb_from_pipe
From: Kalle Valo @ 2019-09-01 8:06 UTC (permalink / raw)
To: Guenter Roeck
Cc: Hui Peng, davem, Mathias Payer, ath10k, linux-wireless, netdev,
linux-kernel
In-Reply-To: <20190831213139.GA32507@roeck-us.net>
Guenter Roeck <linux@roeck-us.net> writes:
> Hi,
>
> On Sat, Aug 03, 2019 at 08:31:01PM -0400, Hui Peng wrote:
>> The `ar_usb` field of `ath10k_usb_pipe_usb_pipe` objects
>> are initialized to point to the containing `ath10k_usb` object
>> according to endpoint descriptors read from the device side, as shown
>> below in `ath10k_usb_setup_pipe_resources`:
>>
>> for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i) {
>> endpoint = &iface_desc->endpoint[i].desc;
>>
>> // get the address from endpoint descriptor
>> pipe_num = ath10k_usb_get_logical_pipe_num(ar_usb,
>> endpoint->bEndpointAddress,
>> &urbcount);
>> ......
>> // select the pipe object
>> pipe = &ar_usb->pipes[pipe_num];
>>
>> // initialize the ar_usb field
>> pipe->ar_usb = ar_usb;
>> }
>>
>> The driver assumes that the addresses reported in endpoint
>> descriptors from device side to be complete. If a device is
>> malicious and does not report complete addresses, it may trigger
>> NULL-ptr-deref `ath10k_usb_alloc_urb_from_pipe` and
>> `ath10k_usb_free_urb_to_pipe`.
>>
>> This patch fixes the bug by preventing potential NULL-ptr-deref.
>>
>> Signed-off-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Hui Peng <benquike@gmail.com>
>> Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
>
> This patch fixes CVE-2019-15099, which has CVSS scores of 7.5 (CVSS 3.0)
> and 7.8 (CVSS 2.0). Yet, I don't find it in the upstream kernel or in Linux
> next.
>
> Is the patch going to be applied to the upstream kernel anytime soon ?
Same answer as in patch 1:
https://patchwork.kernel.org/patch/11074655/
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ 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