Netdev List
 help / color / mirror / Atom feed
* [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup
From: Paolo Abeni @ 2019-02-22 16:06 UTC (permalink / raw)
  To: netdev; +Cc: David S. Miller, David Ahern, Stefano Brivio
In-Reply-To: <cover.1550851038.git.pabeni@redhat.com>

Add a couple of new tests, explicitly checking that the kernel
timely releases pmtu exceptions on related device removal.
This is mostly a regression test vs the issue fixed by:
https://patchwork.ozlabs.org/patch/1045488/

Only 2 new test cases have been added, instead of extending all
the existing ones, because the reproducer requires executing
several commands and would slow down too much the tests otherwise.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 tools/testing/selftests/net/pmtu.sh | 52 ++++++++++++++++++++++++++++-
 1 file changed, 51 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 634e91e8fe25..4bc72bc26899 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -135,7 +135,9 @@ tests="
 	pmtu_vti6_default_mtu		vti6: default MTU assignment
 	pmtu_vti4_link_add_mtu		vti4: MTU setting on link creation
 	pmtu_vti6_link_add_mtu		vti6: MTU setting on link creation
-	pmtu_vti6_link_change_mtu	vti6: MTU changes on link changes"
+	pmtu_vti6_link_change_mtu	vti6: MTU changes on link changes
+	pmtu_ipv4_exception_cleanup	ipv4: cleanup of cached exceptions
+	pmtu_ipv6_exception_cleanup	ipv6: cleanup of cached exceptions"
 
 NS_A="ns-$(mktemp -u XXXXXX)"
 NS_B="ns-$(mktemp -u XXXXXX)"
@@ -1006,6 +1008,54 @@ test_pmtu_vti6_link_change_mtu() {
 	return ${fail}
 }
 
+test_cleanup_vxlanY_or_geneveY_exception() {
+	outer="${1}"
+	encap="${2}"
+	ll_mtu=4000
+
+	which taskset 2>/dev/null | return 2
+	which timeout 2>/dev/null | return 2
+	cpu_list=`grep processor /proc/cpuinfo | cut -d ' ' -f 2`
+
+	setup namespaces routing ${encap}${outer} || return 2
+	trace "${ns_a}" ${encap}_a   "${ns_b}"  ${encap}_b \
+	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
+	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B
+
+	# Create route exception by exceeding link layer MTU
+	mtu "${ns_a}"  veth_A-R1 $((${ll_mtu} + 1000))
+	mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
+	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
+	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
+
+	mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
+	mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
+
+	# fill exception cache for multiple cpus [2]
+	# we can always use inner ipv4 for that
+	ncpus=0
+	for cpu in $cpu_list; do
+		taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
+		ncpus=$((ncpus + 1))
+		[ ${ncpus} -gt 1 ] && break
+	done
+
+	fail=0
+	if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then
+		err "  can't delete veth device in a timely manner, pmtu dst likely leaked"
+		fail=1
+	fi
+	return ${fail}
+}
+
+test_pmtu_ipv6_exception_cleanup() {
+	test_cleanup_vxlanY_or_geneveY_exception 6 vxlan
+}
+
+test_pmtu_ipv4_exception_cleanup() {
+	test_cleanup_vxlanY_or_geneveY_exception 4 vxlan
+}
+
 usage() {
 	echo
 	echo "$0 [OPTIONS] [TEST]..."
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next v2] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet
From: Nikolay Aleksandrov @ 2019-02-22 16:08 UTC (permalink / raw)
  To: wenxu, davem; +Cc: netdev
In-Reply-To: <f4edcd7e-339c-4a55-ed85-ca16f4720f13@ucloud.cn>

On 22/02/2019 14:50, wenxu wrote:
> On 2019/2/22 下午6:26, Nikolay Aleksandrov wrote:
>> On 22/02/2019 11:20, wenxu@ucloud.cn wrote:
>>> From: wenxu <wenxu@ucloud.cn>
>>>
>>> Current fib_multipath_hash_policy can make hash based on the L3 or
>>> L4. But it only work on the outer IP. So a specific tunnel always
>>> has the same hash value. But a specific tunnel may contain so many
>>> inner connections. However there is no good way for tunnel packet.
>>> A specific tunnel route based on the percpu dst_cache. It will not
>>> lookup route table for each packet.
>>>
>>> This patch provide a based cpu id hash policy. The different
>>> connection run on different cpu and there will be different hash
>>> value for percpu dst_cache.
>>>
>>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>>> ---
>>>  net/ipv4/route.c           | 6 ++++++
>>>  net/ipv4/sysctl_net_ipv4.c | 2 +-
>>>  2 files changed, 7 insertions(+), 1 deletion(-)
>>>
>> Hi,
>> When we had the same issue in the bonding, we added a new mode which used
>> the flow dissector to get the inner headers and hash on them.
>> I believe that is even easier nowadays, but I think people wanted to
>> use different hash algorithms as well and last we discussed this we
>> were talking about yet another bpf use to generate the hash.
>> Now we got bpf_set_hash() which can be used to achieve this from various
>> places, if you use that with hash_policy=1 (L4) then skb->hash will
>> be used and you can achieve any hashing that you desire.
>>
>> Cheers,
>>  Nik
>>
>>
> Yes, we can set the skb->hash. But ip tunnel lookup the route table without skb.
> 
> The most important for performance issue most tunnel send work with dst_cache.
> 
> The dst_cache is percpu cache. So the number of dst_entry of a specific tunnel is the cpu cores .
> 
> So It's more better hash based on outer and cpu id.
> 
> 

I was just hoping for a more generic solution, anyway if this would go forward
please add documentation for the new option in Documentation/networking/ip-sysctl.txt
there's a description of fib_multipath_hash_policy. Also how is this supposed to work
if skb is present & skb->hash is set ? I believe it will be ignored, so it really only
works for lookups without an skb (or without a generated hash).

A thought (unchecked/untested, may not make much sense, feel free to ignore):
Why don't you make it mix flowi4_mark in with the hash and add some tunnel option
which mixes the CPU in that field ?
That is the new '2' mode will mix flowi4_mark with the hash and if some specific
tunnel option is set, the tunnel code will mix smp_current_id() with fl4's mark
before calling ip_route_output(), this sounds more useful to me as users will be
able to affect the hash calculation via the mark from other places in different
ways.

Cheers,
 Nik


^ permalink raw reply

* Re: [PATCH net] Revert "bridge: do not add port to router list when receives query with source 0.0.0.0"
From: Nikolay Aleksandrov @ 2019-02-22 16:20 UTC (permalink / raw)
  To: Hangbin Liu, netdev
  Cc: roopa, bridge, davem, yinxu, Greg Kroah-Hartman, stable,
	Sebastian Gottschall, Linus Lüssing
In-Reply-To: <20190222132232.24123-1-liuhangbin@gmail.com>

On 22/02/2019 15:22, Hangbin Liu wrote:
> This reverts commit 5a2de63fd1a5 ("bridge: do not add port to router list
> when receives query with source 0.0.0.0") and commit 0fe5119e267f ("net:
> bridge: remove ipv6 zero address check in mcast queries")
> 
> The reason is RFC 4541 is not a standard but suggestive. Currently we
> will elect 0.0.0.0 as Querier if there is no ip address configured on
> bridge. If we do not add the port which recives query with source
> 0.0.0.0 to router list, the IGMP reports will not be about to forward
> to Querier, IGMP data will also not be able to forward to dest.
> 
> As Nikolay suggested, revert this change first and add a boolopt api
> to disable none-zero election in future if needed.
> 
> Reported-by: Linus Lüssing <linus.luessing@c0d3.blue>
> Reported-by: Sebastian Gottschall <s.gottschall@newmedia-net.de>
> Fixes: 5a2de63fd1a5 ("bridge: do not add port to router list when receives query with source 0.0.0.0")
> Fixes: 0fe5119e267f ("net: bridge: remove ipv6 zero address check in mcast queries")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  net/bridge/br_multicast.c | 9 +--------
>  1 file changed, 1 insertion(+), 8 deletions(-)
> 

Thank you. Unfortunately we made a mistake and have to fix compatibility with
the current bridges. As noted in the commit message if this is needed it can
be added as optional behaviour with default off so we don't break any setups.

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply

* Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 16:35 UTC (permalink / raw)
  To: David Miller
  Cc: paul@paul-moore.com, netdev@vger.kernel.org,
	linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org
In-Reply-To: <20190218.172544.1436352995315454863.davem@davemloft.net>

I tried to analyze the cases of using icmp_send in kernel. It indirectly used by many protocols:
ARP, IP, UDP, Netfilter, IPVS, IPIP, GRE over IP, CLIP, XFRM, CIPSOv4.
Different IP tunnels and XFRM operating directly over IP layer and if using own skb->cb data,
having IP header data in front of it. CLIP uses icmp_send for packets from arp queue only.
So, If I right, only TCP layer moves IP header data and only CIPSOv4 operates on both IP and
TCP layers now. 

19.02.2019, 04:25, "David Miller" <davem@davemloft.net>:
> From: Nazarov Sergey <s-nazarov@yandex.ru>
> Date: Mon, 18 Feb 2019 16:39:11 +0300
>
>>  I think, it would not be a good solution, if I will analyze all
>>  subsystems using icmp_send, because I do not have enough knowledge
>>  for this. I propose to add a new function, for example,
>>  ismp_send_safe, something like that:
>
> Please don't do this.
>
> Solve the problem properly by auditing each case, there aren't a lot and
> it is not too difficult to see the upcall sites.

^ permalink raw reply

* Re: [PATCH] tcp: detect use sendpage for slab-based objects
From: Eric Dumazet @ 2019-02-22 16:39 UTC (permalink / raw)
  To: Vasily Averin; +Cc: netdev
In-Reply-To: <80f35733-e3cf-c7da-1822-87054903dc67@virtuozzo.com>

On Fri, Feb 22, 2019 at 6:02 AM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> On 2/21/19 7:00 PM, Eric Dumazet wrote:
> > On Thu, Feb 21, 2019 at 7:30 AM Vasily Averin <vvs@virtuozzo.com> wrote:
> >> index 2079145a3b7c..cf9572f4fc0f 100644
> >> --- a/net/ipv4/tcp.c
> >> +++ b/net/ipv4/tcp.c
> >> @@ -996,6 +996,7 @@ ssize_t do_tcp_sendpages(struct sock *sk, struct page *page, int offset,
> >>                         goto wait_for_memory;
> >>
> >>                 if (can_coalesce) {
> >> +                       WARN_ON_ONCE(PageSlab(page));
> >
> > Please use VM_WARN_ON_ONCE() to make this a nop for CONFIG_VM_DEBUG=n
> >
> > Also the whole tcp_sendpage() should be protected, not only the coalescing part.
> >
> > (The get_page()  done few lines later should not be attempted either)
> >
> >>                         skb_frag_size_add(&skb_shinfo(skb)->frags[i - 1], copy);
> >>                 } else {
> >>                         get_page(page);
> >> --
> >
> > It seems the bug has nothing to do with TCP, and belongs to the caller.
> >
> > Otherwise you need to add the check to all existing .sendpage() /
> > .sendpage_locked() handler out there.
>
> Eric, could you please elaborate once again why tcp_sendpage() should not handle slab objects?

Simply because SLAB has its own way to manage objects from a page, and
does not care
about the underlying page having its refcount elevated.

ptr = kmalloc(xx)
...  < here you can attempt cheating and add one to the underlying page>
kfree(ptr); // SLAB does not care of page count, it will effectively
put ptr in the free list.

ptr2 = kmalloc(xx); //

ptr2 can be the same than ptr (object was kfreed() earlier)

This means that some other stuff will happily reuse the piece of
memory that you wanted to use for zero-copy.

This is a serious bug IMO, since this would allow for data corruption.



>
> There is known restriction: sendpage should not be called for pages with counter=0,
> because internal put_page() releases the page. All sendpage callers I know have such check.
>
> However why they should add one check for PageSlab?
>
> Let me explain the problem once again:
> I do not see any bugs neither in tcp nor in any sendpage callers,
> there is false alert on receiving side that crashes correctly worked host.

This is not a false alert, but a very fundamental issue.

We can not mix kmalloc() and page fragments, this simply is buggy.

>
> There is network block device with XFS,
> XFS submit IO request with slab objects,
> block device driver checks that page count is positive and decides to use sendpage.
> sendpage calls tcp_sendpage() that can merge 2 neighbour slab objects into one tcp fragment.
>
> If data is transferred outside -- nothing bad happen, network device successfully send data outside.
> However if data is received locally tcp_recvmsg detects strange vector with "merged" slab objects.
> It is not real problem, data can be accessed correctly, however this check calls BUG_ON and crashes the host.
>
> By this way recently added hardening check forces all .sendpage callers modify code that worked correctly for ages.
>
> It looks abnormal to me, but I do not understand how to fix this problem correctly.
>
> I do not like an idea to keep current state -- it can trigger crash of correctly worked hosts in some rare corner cases.
> I do not like an idea to fix all callers -- why they need modify correctly worked code to protect from false positive?
> I do not like an idea to modify tcp -- to block merge of fragments with slab objects like I proposed earlier.
> We can trigger warning in tcp code -- to inform .sendpage callers that they are under fire,
> however I agree with yours "bug has nothing to do with TCP" and do not understand why we need to modify tcp_sendpage().
>
> May be it's better to replace BUG_ON to WARN_ON in hardening check?
> Could you probably advise some other solution?
>
> Thank you,
>         Vasily Averin

^ permalink raw reply

* Re: [PATCH net-next] net: phy: let genphy_c45_read_abilities also check aneg capability
From: Maxime Chevallier @ 2019-02-22 16:51 UTC (permalink / raw)
  To: Heiner Kallweit
  Cc: Andrew Lunn, Florian Fainelli, David Miller,
	netdev@vger.kernel.org
In-Reply-To: <a798f789-94a9-f1ac-1cba-276a2c517089@gmail.com>

Hello Heiner,

On Fri, 22 Feb 2019 08:23:04 +0100
Heiner Kallweit <hkallweit1@gmail.com> wrote:

>When using genphy_c45_read_abilities() as get_features callback we
>also have to set the autoneg capability in phydev->supported.

>@@ -336,6 +336,17 @@ int genphy_c45_pma_read_abilities(struct phy_device *phydev)

This makes the helper not reading PMA-only registers, so the naming
isn't really relevant anymore, but that's a small detail.

Other than that,

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Thanks,

Maxime

^ permalink raw reply

* Re: [PATCH net-next 3/5] devlink: create a special NDO for getting the devlink instance
From: Jakub Kicinski @ 2019-02-22 16:56 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222095131.GQ23151@unicorn.suse.cz>

On Fri, 22 Feb 2019 10:51:31 +0100, Michal Kubecek wrote:
> On Thu, Feb 21, 2019 at 09:46:18AM -0800, Jakub Kicinski wrote:
> > Instead of iterating over all devlink ports add a NDO which
> > will return the devlink instance from the driver.
> > 
> > Suggested-by: Jiri Pirko <jiri@resnulli.us>
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> >  include/linux/netdevice.h |  6 ++++
> >  net/core/devlink.c        | 62 +++++++++++++--------------------------
> >  2 files changed, 26 insertions(+), 42 deletions(-)
> > 
> > diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> > index aab4d9f6613d..eebcef6b8191 100644
> > --- a/include/linux/netdevice.h
> > +++ b/include/linux/netdevice.h
> > @@ -940,6 +940,8 @@ struct dev_ifalias {
> >  	char ifalias[];
> >  };
> >  
> > +struct devlink;
> > +
> >  /*
> >   * This structure defines the management hooks for network devices.
> >   * The following hooks can be defined; unless noted otherwise, they are
> > @@ -1248,6 +1250,9 @@ struct dev_ifalias {
> >   *	that got dropped are freed/returned via xdp_return_frame().
> >   *	Returns negative number, means general error invoking ndo, meaning
> >   *	no frames were xmit'ed and core-caller will free all frames.
> > + * struct devlink *(*ndo_get_devlink)(struct net_device *dev);
> > + *	Get devlink instance associated with a given netdev.
> > + *	Called with a reference on the device only, rtnl_lock is not held.  
> 
> The formulation is a bit ambiguous. Do I understand correctly that what
> it says is that device reference is sufficient and rtnl_lock is not
> necessary (but there is no harm if caller holds rtnl_lock)?

Yes, only reference is held, the expectation is that drivers free
netdevs before they unregister devlink.  The devlink vs rtnl ordering
gets a bit tricky, drivers should definitely not expect rtnl to be held
in this NDO.  If the caller comes from rtnl path (like ethtool) it will
likely have to release rtnl before taking devlink->lock.

^ permalink raw reply

* INFO: task hung in rtnetlink_rcv_msg
From: syzbot @ 2019-02-22 17:00 UTC (permalink / raw)
  To: ast, christian, daniel, davem, dsahern, hawk, idosch,
	jakub.kicinski, john.fastabend, kafai, ktkhai, linux-kernel,
	netdev, petrm, roopa, songliubraving, syzkaller-bugs, xdp-newbies,
	yhs

Hello,

syzbot found the following crash on:

HEAD commit:    64c0133eb88a Merge tag 'armsoc-fixes' of git://git.kernel...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=14976800c00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=ee434566c893c7b1
dashboard link: https://syzkaller.appspot.com/bug?extid=8218a8a0ff60c19b8eae
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11c8a14cc00000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=1189db74c00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+8218a8a0ff60c19b8eae@syzkaller.appspotmail.com

INFO: task syz-executor725:8005 blocked for more than 140 seconds.
       Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D27984  8005   7591 0x00000004
Call Trace:
  context_switch kernel/sched/core.c:2844 [inline]
  __schedule+0x817/0x1cc0 kernel/sched/core.c:3485
  schedule+0x92/0x180 kernel/sched/core.c:3529
  schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
  __mutex_lock_common kernel/locking/mutex.c:1002 [inline]
  __mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
  rtnl_lock net/core/rtnetlink.c:77 [inline]
  rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
  netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x806/0x930 net/socket.c:2114
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2152
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg net/socket.c:2159 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
  do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac037 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8008 blocked for more than 140 seconds.
       Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D27552  8008   7590 0x00000004
Call Trace:
  context_switch kernel/sched/core.c:2844 [inline]
  __schedule+0x817/0x1cc0 kernel/sched/core.c:3485
  schedule+0x92/0x180 kernel/sched/core.c:3529
  schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
  __mutex_lock_common kernel/locking/mutex.c:1002 [inline]
  __mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
  rtnl_lock net/core/rtnetlink.c:77 [inline]
  rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
  netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x806/0x930 net/socket.c:2114
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2152
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg net/socket.c:2159 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
  do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03d R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8009 blocked for more than 140 seconds.
       Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D28008  8009   7595 0x00000004
Call Trace:
  context_switch kernel/sched/core.c:2844 [inline]
  __schedule+0x817/0x1cc0 kernel/sched/core.c:3485
  schedule+0x92/0x180 kernel/sched/core.c:3529
  schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
  __mutex_lock_common kernel/locking/mutex.c:1002 [inline]
  __mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
  rtnl_lock net/core/rtnetlink.c:77 [inline]
  rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
  netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x806/0x930 net/socket.c:2114
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2152
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg net/socket.c:2159 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
  do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03d R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8010 blocked for more than 140 seconds.
       Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D28000  8010   7596 0x00000004
Call Trace:
  context_switch kernel/sched/core.c:2844 [inline]
  __schedule+0x817/0x1cc0 kernel/sched/core.c:3485
  schedule+0x92/0x180 kernel/sched/core.c:3529
  schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
  __mutex_lock_common kernel/locking/mutex.c:1002 [inline]
  __mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
  rtnl_lock net/core/rtnetlink.c:77 [inline]
  rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
  netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x806/0x930 net/socket.c:2114
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2152
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg net/socket.c:2159 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
  do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03e R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000
INFO: task syz-executor725:8011 blocked for more than 140 seconds.
       Not tainted 5.0.0-rc6+ #75
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor725 D28008  8011   7594 0x00000004
Call Trace:
  context_switch kernel/sched/core.c:2844 [inline]
  __schedule+0x817/0x1cc0 kernel/sched/core.c:3485
  schedule+0x92/0x180 kernel/sched/core.c:3529
  schedule_preempt_disabled+0x13/0x20 kernel/sched/core.c:3587
  __mutex_lock_common kernel/locking/mutex.c:1002 [inline]
  __mutex_lock+0x726/0x1310 kernel/locking/mutex.c:1072
  mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1087
  rtnl_lock net/core/rtnetlink.c:77 [inline]
  rtnetlink_rcv_msg+0x40a/0xb00 net/core/rtnetlink.c:5127
  netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x806/0x930 net/socket.c:2114
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2152
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg net/socket.c:2159 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
  do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 <7a> fd ff 11 7a  
fd ff 11 7a fd ff 11 7a fd ff 11 7a fd ff 11 7a fd
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac03b R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000

Showing all locks held in the system:
3 locks held by kworker/1:1/22:
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
__write_once_size include/linux/compiler.h:220 [inline]
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
arch_atomic64_set arch/x86/include/asm/atomic64_64.h:34 [inline]
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
atomic64_set include/asm-generic/atomic-instrumented.h:40 [inline]
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
atomic_long_set include/asm-generic/atomic-long.h:59 [inline]
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
set_work_data kernel/workqueue.c:617 [inline]
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
set_work_pool_and_clear_pending kernel/workqueue.c:644 [inline]
  #0: 000000009b0b09b4 ((wq_completion)"%s"("ipv6_addrconf")){+.+.}, at:  
process_one_work+0x87e/0x1790 kernel/workqueue.c:2144
  #1: 00000000e8391065 ((addr_chk_work).work){+.+.}, at:  
process_one_work+0x8b4/0x1790 kernel/workqueue.c:2148
  #2: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock+0x17/0x20  
net/core/rtnetlink.c:77
1 lock held by khungtaskd/1040:
  #0: 00000000083344d4 (rcu_read_lock){....}, at:  
debug_show_all_locks+0x5f/0x27e kernel/locking/lockdep.c:4389
1 lock held by rsyslogd/7472:
  #0: 000000007562dcc1 (&f->f_pos_lock){+.+.}, at: __fdget_pos+0xee/0x110  
fs/file.c:795
2 locks held by getty/7562:
  #0: 000000000837ceb2 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 00000000d91e6e17 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7563:
  #0: 00000000243db696 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 000000000002034d (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7564:
  #0: 00000000a59e483b (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 000000004032d53a (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7565:
  #0: 0000000025343c97 (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 00000000727bf19a (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7566:
  #0: 000000003dac20af (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 00000000fef38c44 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7567:
  #0: 00000000da1bfc9c (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 00000000f82cf636 (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
2 locks held by getty/7568:
  #0: 00000000a5d9fcdf (&tty->ldisc_sem){++++}, at:  
ldsem_down_read+0x33/0x40 drivers/tty/tty_ldsem.c:341
  #1: 000000005ae165cb (&ldata->atomic_read_lock){+.+.}, at:  
n_tty_read+0x232/0x1b70 drivers/tty/n_tty.c:2154
1 lock held by syz-executor725/8002:
1 lock held by syz-executor725/8005:
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock  
net/core/rtnetlink.c:77 [inline]
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00  
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8008:
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock  
net/core/rtnetlink.c:77 [inline]
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00  
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8009:
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock  
net/core/rtnetlink.c:77 [inline]
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00  
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8010:
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock  
net/core/rtnetlink.c:77 [inline]
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00  
net/core/rtnetlink.c:5127
1 lock held by syz-executor725/8011:
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnl_lock  
net/core/rtnetlink.c:77 [inline]
  #0: 00000000da1e0257 (rtnl_mutex){+.+.}, at: rtnetlink_rcv_msg+0x40a/0xb00  
net/core/rtnetlink.c:5127

=============================================

NMI backtrace for cpu 0
CPU: 0 PID: 1040 Comm: khungtaskd Not tainted 5.0.0-rc6+ #75
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  nmi_cpu_backtrace.cold+0x63/0xa4 lib/nmi_backtrace.c:101
  nmi_trigger_cpumask_backtrace+0x1be/0x236 lib/nmi_backtrace.c:62
  arch_trigger_cpumask_backtrace+0x14/0x20 arch/x86/kernel/apic/hw_nmi.c:38
  trigger_all_cpu_backtrace include/linux/nmi.h:146 [inline]
  check_hung_uninterruptible_tasks kernel/hung_task.c:203 [inline]
  watchdog+0x9df/0xee0 kernel/hung_task.c:287
  kthread+0x357/0x430 kernel/kthread.c:246
  ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Sending NMI from CPU 0 to CPUs 1:
INFO: NMI handler (nmi_cpu_backtrace_handler) took too long to run: 1.415  
msecs
NMI backtrace for cpu 1
CPU: 1 PID: 8002 Comm: syz-executor725 Not tainted 5.0.0-rc6+ #75
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:unwind_next_frame.part.0+0x1a8/0x9a0  
arch/x86/kernel/unwind_frame.c:329
Code: 8b 6d 20 4c 89 ee 48 89 df 4c 89 95 60 ff ff ff 4c 89 9d 68 ff ff ff  
4c 89 8d 70 ff ff ff e8 6f f7 ff ff 4c 8b 8d 70 ff ff ff <41> 89 c0 4c 8b  
9d 68 ff ff ff b8 01 00 00 00 45 84 c0 4c 8b 95 60
RSP: 0018:ffff888096136e28 EFLAGS: 00000286
RAX: 0000000000000001 RBX: ffff888096136f00 RCX: 1ffff11012c26de8
RDX: dffffc0000000000 RSI: 1ffff11012c26d00 RDI: ffff888096136f48
RBP: ffff888096136ed8 R08: 0000000000000001 R09: ffff888096136f50
R10: ffff888096136f28 R11: ffff888096136f38 R12: 1ffff11012c26dca
R13: ffff888096137b70 R14: ffff888096137b40 R15: ffff888096137f58
FS:  00000000024b2880(0000) GS:ffff8880ae900000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: ffffffffff600400 CR3: 000000009810b000 CR4: 00000000001406e0
Call Trace:
  unwind_next_frame+0x3b/0x50 arch/x86/kernel/unwind_frame.c:287
  __save_stack_trace+0x7a/0xf0 arch/x86/kernel/stacktrace.c:44
  save_stack_trace+0x1a/0x20 arch/x86/kernel/stacktrace.c:60
  save_stack+0x45/0xd0 mm/kasan/common.c:73
  set_track mm/kasan/common.c:85 [inline]
  __kasan_kmalloc mm/kasan/common.c:496 [inline]
  __kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:469
  kasan_kmalloc+0x9/0x10 mm/kasan/common.c:504
  __do_kmalloc_node mm/slab.c:3673 [inline]
  __kmalloc_node_track_caller+0x4e/0x70 mm/slab.c:3687
  __kmalloc_reserve.isra.0+0x40/0xf0 net/core/skbuff.c:140
  pskb_expand_head+0x14e/0xdd0 net/core/skbuff.c:1465
  netlink_trim+0x215/0x270 net/netlink/af_netlink.c:1292
  netlink_unicast+0xbf/0x720 net/netlink/af_netlink.c:1326
  rtnetlink_send+0xf0/0x110 net/core/rtnetlink.c:721
  tcf_add_notify net/sched/act_api.c:1325 [inline]
  tcf_action_add+0x243/0x370 net/sched/act_api.c:1344
  tc_ctl_action+0x3b6/0x4bd net/sched/act_api.c:1392
  rtnetlink_rcv_msg+0x465/0xb00 net/core/rtnetlink.c:5130
  netlink_rcv_skb+0x17a/0x460 net/netlink/af_netlink.c:2477
  rtnetlink_rcv+0x1d/0x30 net/core/rtnetlink.c:5148
  netlink_unicast_kernel net/netlink/af_netlink.c:1310 [inline]
  netlink_unicast+0x536/0x720 net/netlink/af_netlink.c:1336
  netlink_sendmsg+0x8ae/0xd70 net/netlink/af_netlink.c:1917
  sock_sendmsg_nosec net/socket.c:621 [inline]
  sock_sendmsg+0xdd/0x130 net/socket.c:631
  ___sys_sendmsg+0x806/0x930 net/socket.c:2114
  __sys_sendmsg+0x105/0x1d0 net/socket.c:2152
  __do_sys_sendmsg net/socket.c:2161 [inline]
  __se_sys_sendmsg net/socket.c:2159 [inline]
  __x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
  do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4459f9
Code: e8 ac e8 ff ff 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 0b 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffdc979ac28 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000004459f9
RDX: 00000000000000c4 RSI: 0000000020000300 RDI: 0000000000000003
RBP: 00000000000ac031 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000200015a6 R11: 0000000000000246 R12: 0000000000406890
R13: 0000000000406920 R14: 0000000000000000 R15: 0000000000000000


---
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#bug-status-tracking 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 net-next 4/5] nfp: add .ndo_get_devlink
From: Jakub Kicinski @ 2019-02-22 17:02 UTC (permalink / raw)
  To: Michal Kubecek; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222100450.GR23151@unicorn.suse.cz>

On Fri, 22 Feb 2019 11:04:50 +0100, Michal Kubecek wrote:
> On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
> > Support getting devlink instance from a new NDO.
> > 
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> >  drivers/net/ethernet/netronome/nfp/nfp_app.h        |  2 ++
> >  drivers/net/ethernet/netronome/nfp/nfp_devlink.c    | 11 +++++++++++
> >  drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  1 +
> >  drivers/net/ethernet/netronome/nfp/nfp_net_repr.c   |  1 +
> >  4 files changed, 15 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > index d578d856a009..f8d422713705 100644
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > @@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
> >  int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
> >  				   struct nfp_net *nn, unsigned int id);
> >  
> > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
> > +
> >  #endif
> > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > index db2da99f6aa7..e9eca99cf493 100644
> > --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > @@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
> >  {
> >  	devlink_port_unregister(&port->dl_port);
> >  }
> > +
> > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
> > +{
> > +	struct nfp_app *app;
> > +
> > +	app = nfp_app_from_netdev(netdev);
> > +	if (!app)
> > +		return NULL;
> > +
> > +	return priv_to_devlink(app->pf);
> > +}  
> 
> AFAICS this would return a pointer to zero initialized struct devlink
> when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
> would execute
> 
> 	if (!dev->netdev_ops->ndo_get_devlink)
> 		return;
> 
> 	devlink = dev->netdev_ops->ndo_get_devlink(dev);
> 	if (!devlink || !devlink->ops->info_get)
> 		return;
> 
> with non-null devlink but null devlink->ops so that it dereferences null
> pointer (and so does devlink_compat_flash_update()).

devlink_compat_flash_update() is this if CONFIG_DEVLINK=n:

static inline int
devlink_compat_flash_update(struct net_device *dev, const char *file_name)
{
	return -EOPNOTSUPP;
}

No?

> Maybe it would be safer not to call ndo_get_devlink directly and have
> an inline wrapper like
> 
> #if IS_ENABLED(CONFIG_NET_DEVLINK)
> static inline struct devlink *dev_get_devlink(struct net_device *dev)
> {
> 	if (dev->netdev_ops->ndo_get_devlink)
> 		return dev->netdev_ops->ndo_get_devlink();
> 	else
> 		retrurn NULL;
> }
> #else
> static inline struct devlink *dev_get_devlink(struct net_device *dev)
> {
> 	return NULL;
> }
> #endif
> 
> so that one can simply call the wrapper and check return value for NULL.

Only devlink code can call this ndo, and it doesn't exist with
DEVLINK=n.  I don't dislike wrappers for NDOs, but I'll defer to Jiri
to decide if we want a wrapper here (without the #if/#else, just the
first part for code clarity) :)

^ permalink raw reply

* Re: [PATCH v5 0/5] M_CAN Framework re-write
From: Dan Murphy @ 2019-02-22 17:05 UTC (permalink / raw)
  To: Wolfgang Grandegger, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <b60b8781-d979-24e7-805a-95f9237c816e@ti.com>

Wolfgang

On 2/22/19 6:50 AM, Dan Murphy wrote:
> Wolfgang
> 
> On 2/22/19 3:38 AM, Wolfgang Grandegger wrote:
>> Hello Dan,
>>
>> what kernel version is that patch series for. I have problems to apply it!
>>
> 
> It is based off of Master
> 
> commit 2137397c92aec3713fa10be3c9b830f9a1674e60 (linux_master/master)
> 
> And I successfully rebased on top of
> 
> commit 8a61716ff2ab23eddd1f7a05a075a374e4d0c3d4 (linux_master/master)
> Merge tag 'ceph-for-5.0-rc8' of git://github.com/ceph/ceph-client
> 

I just pulled these patches and they applied fine to the top commit of linux master

Do I need to rebase on top of a for-next branch for you?

Dan

> Dan
> 
>> Wolfgang.
>>
>> Am 21.02.19 um 17:41 schrieb Wolfgang Grandegger:
>>> Hello Dan,
>>>
>>> I will have a closer look end of this week!
>>>
>>> Wolfgang.
>>>
>>> Am 21.02.19 um 17:24 schrieb Dan Murphy:
>>>> Bump
>>>>
>>>> On 2/14/19 12:27 PM, Dan Murphy wrote:
>>>>> Hello
>>>>>
>>>>> OK I did not give up on this patch series just got a little preoccupied with
>>>>> some other kernel work.  But here is the update per the comments.
>>>>>
>>>>> It should be understood I broke these out for reviewability.
>>>>> For instance the first patch does not compile on its own as including this
>>>>> patch should not change the current functionality and it pulls all the io-mapped
>>>>> code from the m_can base file to a platfrom file.
>>>>>
>>>>> The next patch "Migrate the m_can code to use the framework"
>>>>> is the change to the kernel for the io-mapped conversion from a flat file to use
>>>>> the framework.  Finally the rename patch just renames the m_can_priv to 
>>>>> m_can_classdev.  I broke this change out specifically for readability of the
>>>>> migration patch per comments on the code.
>>>>>
>>>>> AFAIC the first 3 patches can all be squashed into a single patch.  Or the
>>>>> first 2 patches in the series can be re-arranged but then m_can functionality is
>>>>> affected in the migration patch.
>>>>>
>>>>> Again the first 3 patches here are all just for readability and review purposes.
>>>>>
>>>>> Dan
>>>>>
>>>>> Dan Murphy (5):
>>>>>   can: m_can: Create a m_can platform framework
>>>>>   can: m_can: Migrate the m_can code to use the framework
>>>>>   can: m_can: Rename m_can_priv to m_can_classdev
>>>>>   dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
>>>>>   can: tcan4x5x: Add tcan4x5x driver to the kernel
>>>>>
>>>>>  .../devicetree/bindings/net/can/tcan4x5x.txt  |  37 +
>>>>>  drivers/net/can/m_can/Kconfig                 |  14 +-
>>>>>  drivers/net/can/m_can/Makefile                |   2 +
>>>>>  drivers/net/can/m_can/m_can.c                 | 788 +++++++++---------
>>>>>  drivers/net/can/m_can/m_can.h                 | 159 ++++
>>>>>  drivers/net/can/m_can/m_can_platform.c        | 198 +++++
>>>>>  drivers/net/can/m_can/tcan4x5x.c              | 531 ++++++++++++
>>>>>  7 files changed, 1320 insertions(+), 409 deletions(-)
>>>>>  create mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
>>>>>  create mode 100644 drivers/net/can/m_can/m_can.h
>>>>>  create mode 100644 drivers/net/can/m_can/m_can_platform.c
>>>>>  create mode 100644 drivers/net/can/m_can/tcan4x5x.c
>>>>>
>>>>
>>>>
> 
> 


-- 
------------------
Dan Murphy

^ permalink raw reply

* WARNING in xt_compat_add_offset
From: syzbot @ 2019-02-22 17:10 UTC (permalink / raw)
  To: coreteam, davem, fw, kadlec, linux-kernel, netdev,
	netfilter-devel, pablo, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    8a61716ff2ab Merge tag 'ceph-for-5.0-rc8' of git://github...
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1456fa6cc00000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7132344728e7ec3f
dashboard link: https://syzkaller.appspot.com/bug?extid=276ddebab3382bbf72db
compiler:       gcc (GCC) 9.0.0 20181231 (experimental)
userspace arch: i386
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=140c0914c00000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+276ddebab3382bbf72db@syzkaller.appspotmail.com

IPv6: ADDRCONF(NETDEV_CHANGE): hsr_slave_0: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): hsr_slave_1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): hsr0: link becomes ready
8021q: adding VLAN 0 to HW filter on device batadv0
cannot load conntrack support for proto=7
WARNING: CPU: 1 PID: 7458 at net/netfilter/x_tables.c:654  
xt_compat_add_offset+0x22a/0x290 net/netfilter/x_tables.c:654
Kernel panic - not syncing: panic_on_warn set ...
CPU: 1 PID: 7458 Comm: syz-executor.0 Not tainted 5.0.0-rc7+ #83
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x172/0x1f0 lib/dump_stack.c:113
  panic+0x2cb/0x65c kernel/panic.c:214
  __warn.cold+0x20/0x45 kernel/panic.c:571
  report_bug+0x263/0x2b0 lib/bug.c:186
  fixup_bug arch/x86/kernel/traps.c:178 [inline]
  fixup_bug arch/x86/kernel/traps.c:173 [inline]
  do_error_trap+0x11b/0x200 arch/x86/kernel/traps.c:271
  do_invalid_op+0x37/0x50 arch/x86/kernel/traps.c:290
  invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:973
RIP: 0010:xt_compat_add_offset+0x22a/0x290 net/netfilter/x_tables.c:654
Code: 00 01 e8 59 67 bb fb 44 89 e0 48 83 c4 08 5b 41 5c 41 5d 41 5e 41 5f  
5d c3 e8 42 67 bb fb 0f 0b e9 56 fe ff ff e8 36 67 bb fb <0f> 0b 41 bc f4  
ff ff ff eb ce 4c 89 f7 e8 14 6a f2 fb e9 75 ff ff
RSP: 0018:ffff8880a8197808 EFLAGS: 00010293
RAX: ffff88809055e040 RBX: ffff8882166548d0 RCX: ffffffff85b47892
RDX: 0000000000000000 RSI: ffffffff85b47a4a RDI: ffff8882166549f0
RBP: ffff8880a8197838 R08: ffff88809055e040 R09: ffffed1042cca92f
R10: ffffed1042cca92e R11: ffff888216654977 R12: 0000000000000018
R13: 0000000000000030 R14: ffff88809055e040 R15: 0000000000000000
  size_entry_mwt net/bridge/netfilter/ebtables.c:2183 [inline]
  compat_copy_entries+0x51b/0x1360 net/bridge/netfilter/ebtables.c:2208
  compat_do_replace+0x3b3/0x680 net/bridge/netfilter/ebtables.c:2302
  compat_do_ebt_set_ctl+0x229/0x278 net/bridge/netfilter/ebtables.c:2384
  compat_nf_sockopt net/netfilter/nf_sockopt.c:144 [inline]
  compat_nf_setsockopt+0x9b/0x140 net/netfilter/nf_sockopt.c:156
  compat_ip_setsockopt net/ipv4/ip_sockglue.c:1284 [inline]
  compat_ip_setsockopt+0x106/0x140 net/ipv4/ip_sockglue.c:1265
  compat_udp_setsockopt+0x68/0xb0 net/ipv4/udp.c:2629
  compat_ipv6_setsockopt+0xca/0x210 net/ipv6/ipv6_sockglue.c:959
  inet_csk_compat_setsockopt+0x99/0x120 net/ipv4/inet_connection_sock.c:1054
  compat_tcp_setsockopt+0x4d/0x80 net/ipv4/tcp.c:3079
  compat_sock_common_setsockopt+0xb4/0x150 net/core/sock.c:3002
  __compat_sys_setsockopt+0x176/0x610 net/compat.c:404
  __do_compat_sys_setsockopt net/compat.c:417 [inline]
  __se_compat_sys_setsockopt net/compat.c:414 [inline]
  __ia32_compat_sys_setsockopt+0xbd/0x150 net/compat.c:414
  do_syscall_32_irqs_on arch/x86/entry/common.c:326 [inline]
  do_fast_syscall_32+0x281/0xc98 arch/x86/entry/common.c:397
  entry_SYSENTER_compat+0x70/0x7f arch/x86/entry/entry_64_compat.S:139
RIP: 0023:0xf7fd3869
Code: 85 d2 74 02 89 0a 5b 5d c3 8b 04 24 c3 8b 14 24 c3 8b 3c 24 c3 90 90  
90 90 90 90 90 90 90 90 90 90 51 52 55 89 e5 0f 34 cd 80 <5d> 5a 59 c3 90  
90 90 90 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90
RSP: 002b:00000000f7fcf0cc EFLAGS: 00000296 ORIG_RAX: 000000000000016e
RAX: ffffffffffffffda RBX: 0000000000000006 RCX: 0000000000000000
RDX: 0000000000000080 RSI: 00000000200000c0 RDI: 0000000000000270
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
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#bug-status-tracking 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 1/3] ipv4: icmp: use icmp_sk_exit()
From: Eric Dumazet @ 2019-02-22 17:11 UTC (permalink / raw)
  To: Kefeng Wang, davem, netdev
In-Reply-To: <20190222015800.192135-1-wangkefeng.wang@huawei.com>



On 02/21/2019 05:57 PM, Kefeng Wang wrote:
> Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
> 
> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
> ---
>  net/ipv4/icmp.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 065997f414e6..364cfe5e414b 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
>  	return 0;
>  
>  fail:
> -	for_each_possible_cpu(i)
> -		inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
> -	free_percpu(net->ipv4.icmp_sk);
> +	icmp_sk_exit(net);
>  	return err;
>  }
>  
> 


I do not like this. Future changes in icmp_sk_exit() might trigger a bug in this seldom tested path.

^ permalink raw reply

* Re: [PATCH 1/3] ipv4: icmp: use icmp_sk_exit()
From: Eric Dumazet @ 2019-02-22 17:16 UTC (permalink / raw)
  To: Kefeng Wang, davem, netdev
In-Reply-To: <bb408779-5ddb-f9ef-e16b-1d0d7a5c3719@gmail.com>



On 02/22/2019 09:11 AM, Eric Dumazet wrote:
> 
> 
> On 02/21/2019 05:57 PM, Kefeng Wang wrote:
>> Simply use icmp_sk_exit() when inet_ctl_sock_create() fail in icmp_sk_init().
>>
>> Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
>> ---
>>  net/ipv4/icmp.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>> index 065997f414e6..364cfe5e414b 100644
>> --- a/net/ipv4/icmp.c
>> +++ b/net/ipv4/icmp.c
>> @@ -1245,9 +1245,7 @@ static int __net_init icmp_sk_init(struct net *net)
>>  	return 0;
>>  
>>  fail:
>> -	for_each_possible_cpu(i)
>> -		inet_ctl_sock_destroy(*per_cpu_ptr(net->ipv4.icmp_sk, i));
>> -	free_percpu(net->ipv4.icmp_sk);
>> +	icmp_sk_exit(net);
>>  	return err;
>>  }
>>  
>>
> 
> 
> I do not like this. Future changes in icmp_sk_exit() might trigger a bug in this seldom tested path.
> 

Oh I see the next patches, now it makes sense.

Please send a proper patch series, with a cover letter, so that it is obvious.

Also you need to include "net-next" instead of plain [PATCH] to express this targets net-next tree.

Carefully read Documentation/networking/netdev-FAQ.rst

Thank you.

^ permalink raw reply

* Re: stmmac / meson8b-dwmac
From: Anand Moon @ 2019-02-22 17:21 UTC (permalink / raw)
  To: Simon Huelck
  Cc: Jerome Brunet, Jose Abreu, Martin Blumenstingl, linux-amlogic,
	netdev, Alexandre TORGUE, Emiliano Ingrassia, Gpeppe.cavallaro
In-Reply-To: <e009cbbb-e31d-2438-e454-2fa10ebec542@gmx.de>

Hi All,

On Fri, 22 Feb 2019 at 01:05, Simon Huelck <simonmail@gmx.de> wrote:
>
> Am 21.02.2019 um 18:46 schrieb Jerome Brunet:
> > On Thu, 2019-02-21 at 18:27 +0100, Simon Huelck wrote:
> >> Hi,
> >>
> >>
> >>
> >> this was changed recently, with a patch for the EEE stuff , see here:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.0-rc7&id=e35e26b26e955c53e61c154ba26b9bb15da6b858
> > Hu, I was not aware this finally went through. Good !
> > As explained in the patch and by Jose, the GMAC should be using IRQ_LEVEL.
> >
> > The realtek PHY has EEE enabled by default. Having this enabled generates a
> > lot of (Low Power) Interrupts.
> >
> > Previously, when the GMAC used IRQ_EDGE. Because it is wrong, we would
> > eventually miss an IRQ and the interface would just die. Unfortunately, it was
> > not that easy find out.
> >
> > 2 years ago, we just noticed that disabling EEE would make the failure go
> > away. Forcing this EEE feature off through DT was merely a work around.
> >
> > Now that the real cause of the problem is known, there is no reason to keep
> > this hack around.
> >
> > Whether EEE adds a performance penality and why, is another topic.
> > As Jose pointed out, you can disable EEE at runtime, using ethtool.
> >
> > Jerome
> >
> Hi,
>
>
>
> i disabled EEE via ethtool and via the .dtb , but the performance
> penalty stays. Kernel 4.14 still gives me the former "good" performance.
>
>
>
> regards,
>
> Simon
>
>
Sorry this is off the topic.

I am using Archlinux on Odroid C1+ and the latest kernel loads with no issue.
only issue is I have is the each time their is random MAC address so I
get new IP from dhcp server.
How can I avoid this. I have tried to enable eFuse driver but with no success.

On Odroid C2 I dont have any issue, it always retained the unique MAC address.

Best Regards
-Anand

^ permalink raw reply

* Re: [PATCH net-next 4/5] nfp: add .ndo_get_devlink
From: Michal Kubecek @ 2019-02-22 17:27 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, jiri, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190222090204.20c410e3@cakuba.netronome.com>

On Fri, Feb 22, 2019 at 09:02:04AM -0800, Jakub Kicinski wrote:
> On Fri, 22 Feb 2019 11:04:50 +0100, Michal Kubecek wrote:
> > On Thu, Feb 21, 2019 at 09:46:19AM -0800, Jakub Kicinski wrote:
> > > Support getting devlink instance from a new NDO.
> > > 
> > > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > > ---
> > >  drivers/net/ethernet/netronome/nfp/nfp_app.h        |  2 ++
> > >  drivers/net/ethernet/netronome/nfp/nfp_devlink.c    | 11 +++++++++++
> > >  drivers/net/ethernet/netronome/nfp/nfp_net_common.c |  1 +
> > >  drivers/net/ethernet/netronome/nfp/nfp_net_repr.c   |  1 +
> > >  4 files changed, 15 insertions(+)
> > > 
> > > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_app.h b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > index d578d856a009..f8d422713705 100644
> > > --- a/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > +++ b/drivers/net/ethernet/netronome/nfp/nfp_app.h
> > > @@ -433,4 +433,6 @@ int nfp_app_nic_vnic_alloc(struct nfp_app *app, struct nfp_net *nn,
> > >  int nfp_app_nic_vnic_init_phy_port(struct nfp_pf *pf, struct nfp_app *app,
> > >  				   struct nfp_net *nn, unsigned int id);
> > >  
> > > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev);
> > > +
> > >  #endif
> > > diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > index db2da99f6aa7..e9eca99cf493 100644
> > > --- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > +++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
> > > @@ -376,3 +376,14 @@ void nfp_devlink_port_unregister(struct nfp_port *port)
> > >  {
> > >  	devlink_port_unregister(&port->dl_port);
> > >  }
> > > +
> > > +struct devlink *nfp_devlink_get_devlink(struct net_device *netdev)
> > > +{
> > > +	struct nfp_app *app;
> > > +
> > > +	app = nfp_app_from_netdev(netdev);
> > > +	if (!app)
> > > +		return NULL;
> > > +
> > > +	return priv_to_devlink(app->pf);
> > > +}  
> > 
> > AFAICS this would return a pointer to zero initialized struct devlink
> > when built with CONFIG_DEVLINK=n. Then devlink_compat_running_version()
> > would execute
> > 
> > 	if (!dev->netdev_ops->ndo_get_devlink)
> > 		return;
> > 
> > 	devlink = dev->netdev_ops->ndo_get_devlink(dev);
> > 	if (!devlink || !devlink->ops->info_get)
> > 		return;
> > 
> > with non-null devlink but null devlink->ops so that it dereferences null
> > pointer (and so does devlink_compat_flash_update()).
> 
> devlink_compat_flash_update() is this if CONFIG_DEVLINK=n:
> 
> static inline int
> devlink_compat_flash_update(struct net_device *dev, const char *file_name)
> {
> 	return -EOPNOTSUPP;
> }
> 
> No?

You are right, I missed that devlink_compat_running_version() and
devlink_compat_flash_update() also have alternative versions when
devlink is disabled. So there is no problem.

> > Maybe it would be safer not to call ndo_get_devlink directly and have
> > an inline wrapper like
> > 
> > #if IS_ENABLED(CONFIG_NET_DEVLINK)
> > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > {
> > 	if (dev->netdev_ops->ndo_get_devlink)
> > 		return dev->netdev_ops->ndo_get_devlink();
> > 	else
> > 		retrurn NULL;
> > }
> > #else
> > static inline struct devlink *dev_get_devlink(struct net_device *dev)
> > {
> > 	return NULL;
> > }
> > #endif
> > 
> > so that one can simply call the wrapper and check return value for NULL.
> 
> Only devlink code can call this ndo, and it doesn't exist with
> DEVLINK=n.  I don't dislike wrappers for NDOs, but I'll defer to Jiri
> to decide if we want a wrapper here (without the #if/#else, just the
> first part for code clarity) :)

If the NDO is only supposed to be called from devlink code (or, more
precisely, code built only with CONFIG_DEVLINK=y), it should be IMHO
mentioned in its description. Another option would be enforcing it by
adding #ifdef around the ndo_get_devlink entry in struct net_device_ops
but that would require using ifdefs also in each driver providing the
NDO which seems inconvenient.

Michal

^ permalink raw reply

* Re: [PATCH net-next 0/8] Add tests for unlocked flower classifier implementation
From: Cong Wang @ 2019-02-22 17:35 UTC (permalink / raw)
  To: Vlad Buslov
  Cc: Linux Kernel Network Developers, Jamal Hadi Salim, David Miller,
	shuah, batuhanosmantaskaya, Lucas Bates, Davide Caratti,
	Marcelo Ricardo Leitner, Chris Mi
In-Reply-To: <20190222140047.13215-1-vladbu@mellanox.com>

On Fri, Feb 22, 2019 at 6:00 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>
> Implement tests for tdc testsuite to verify concurrent rules update with
> rtnl-unlocked flower classifier implementation. The goal of these tests
> is to verify general flower classifier correctness by updating filters
> on same classifier instance in parallel and to verify its atomicity by
> concurrently updating filters in same handle range. All three filter
> update operations (add, replace, delete) are tested.
>
> Existing script tdc_batch.py is re-used for batch file generation. It is
> extended with several optional CLI arguments that are needed for
> concurrency tests. Thin wrapper tdc_multibatch.py is implemented on top
> of tdc_batch.py to simplify its usage when generating multiple batch
> files for several test configurations.
>
> Parallelism in tests is implemented by running multiple instances of tc
> in batch mode with xargs tool. Xargs is chosen for its ease of use and
> because it is available by default on most modern Linux distributions.

I don't look into the details, but I love to see we have tests for parallel
filter updates! So for the whole patchset:

Acked-by: Cong Wang <xiyou.wangcong@gmail.com>

Thanks!

^ permalink raw reply

* [PATCH v2 0/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:39 UTC (permalink / raw)
  To: David Miller
  Cc: paul@paul-moore.com, netdev@vger.kernel.org,
	linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>

The original discussion is here:
https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/

Changes in v2:
- original patch divided on 2 parts: add __icmp_send function, fix cipso_v4_error
- add __ip_option_compile not using icmp_send in case nof error
---
 include/net/icmp.h    |    9 ++++++++-
 include/net/ip.h      |    2 ++
 net/ipv4/cipso_ipv4.c |   17 +++++++++++++++--
 net/ipv4/icmp.c       |    7 ++++---
 net/ipv4/ip_options.c |   22 +++++++++++++++++-----
 5 files changed, 46 insertions(+), 11 deletions(-)


^ permalink raw reply

* Re: [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:43 UTC (permalink / raw)
  To: David Miller
  Cc: paul@paul-moore.com, netdev@vger.kernel.org,
	linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>

Add __icmp_send function having ip_options struct parameter
---
 include/net/icmp.h    |    9 ++++++++-
 net/ipv4/icmp.c       |    7 ++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/include/net/icmp.h b/include/net/icmp.h
index 6ac3a5b..e0f709d 100644
--- a/include/net/icmp.h
+++ b/include/net/icmp.h
@@ -22,6 +22,7 @@
 
 #include <net/inet_sock.h>
 #include <net/snmp.h>
+#include <net/ip.h>
 
 struct icmp_err {
   int		errno;
@@ -39,7 +40,13 @@ struct icmp_err {
 struct sk_buff;
 struct net;
 
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info);
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+		 const struct ip_options *opt);
+static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+{
+	__icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
+}
+
 int icmp_rcv(struct sk_buff *skb);
 int icmp_err(struct sk_buff *skb, u32 info);
 int icmp_init(void);
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 065997f..3f24414 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
  *			MUST reply to only the first fragment.
  */
 
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+		 const struct ip_options *opt)
 {
 	struct iphdr *iph;
 	int room;
@@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
 					  iph->tos;
 	mark = IP4_REPLY_MARK(net, skb_in->mark);
 
-	if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
+	if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
 		goto out_unlock;
 
 
@@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
 	local_bh_enable();
 out:;
 }
-EXPORT_SYMBOL(icmp_send);
+EXPORT_SYMBOL(__icmp_send);
 
 
 static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
---

^ permalink raw reply related

* Re: [PATCH net-next v3 3/4] enetc: Add ENETC PF level external MDIO support
From: Andrew Lunn @ 2019-02-22 17:44 UTC (permalink / raw)
  To: Claudiu Manoil
  Cc: Shawn Guo, Li Yang, David S . Miller, devicetree,
	alexandru.marginean, linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1550847859-17346-4-git-send-email-claudiu.manoil@nxp.com>

On Fri, Feb 22, 2019 at 05:04:18PM +0200, Claudiu Manoil wrote:
> Each ENETC PF has its own MDIO interface, the corresponding
> MDIO registers are mapped in the ENETC's Port register block.
> The current patch adds a driver for these PF level MDIO buses,
> so that each PF can manage directly its own external link.
> 
> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply

* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Dumazet @ 2019-02-22 17:45 UTC (permalink / raw)
  To: Eric Biggers, netdev, David S . Miller
  Cc: linux-kernel, Mao Wenan, Cong Wang, Lorenzo Colitti, Tetsuo Handa,
	Al Viro
In-Reply-To: <20190221221356.173485-1-ebiggers@kernel.org>



On 02/21/2019 02:13 PM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> closed concurrently with fchownat().  However, it ignored that many
> other proto_ops::release() methods don't set sock->sk to NULL and
> therefore allow the same use-after-free:
> 

I fail to see how setting a pointer to NULL can avoid races.


We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
while sockfs_setattr() is used.

sockfs_setattr()
...
     if (sock->sk)

// even if sock->sk was not NULL for the if (...).

// it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.

        sock->sk->sk_uid = iattr->ia_uid;



>     - base_sock_release
>     - bnep_sock_release
>     - cmtp_sock_release
>     - data_sock_release
>     - dn_release
>     - hci_sock_release
>     - hidp_sock_release
>     - iucv_sock_release
>     - l2cap_sock_release
>     - llcp_sock_release
>     - llc_ui_release
>     - rawsock_release
>     - rfcomm_sock_release
>     - sco_sock_release
>     - svc_release
>     - vcc_release
>     - x25_release
> 
> Rather than fixing all these and relying on every socket type to get
> this right forever, just make __sock_release() set sock->sk to NULL
> itself after calling proto_ops::release().
> 
> Reproducer that produces the KASAN splat when any of these socket types
> are configured into the kernel:
> 
>     #include <pthread.h>
>     #include <stdlib.h>
>     #include <sys/socket.h>
>     #include <unistd.h>
> 
>     pthread_t t;
>     volatile int fd;
> 
>     void *close_thread(void *arg)
>     {
>         for (;;) {
>             usleep(rand() % 100);
>             close(fd);
>         }
>     }
> 
>     int main()
>     {
>         pthread_create(&t, NULL, close_thread, NULL);
>         for (;;) {
>             fd = socket(rand() % 50, rand() % 11, 0);
>             fchownat(fd, "", 1000, 1000, 0x1000);
>             close(fd);
>         }
>     }
> 
> Fixes: 86741ec25462 ("net: core: Add a UID field to struct sock.")
> Cc: <stable@vger.kernel.org> # v4.10+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> NOTE: I am not an expert in the networking code, so please carefully
> check that I haven't missed some reason why this simple fix won't do.
> 
>  net/socket.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/socket.c b/net/socket.c
> index d80d87a395ea..320f51b22b19 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -577,6 +577,7 @@ static void __sock_release(struct socket *sock, struct inode *inode)
>  		if (inode)
>  			inode_lock(inode);
>  		sock->ops->release(sock);
> +		sock->sk = NULL;
>  		if (inode)
>  			inode_unlock(inode);
>  		sock->ops = NULL;
> 

^ permalink raw reply

* Re: [PATCH] net: dsa: mv88e6xxx: Add lockdep classes to fix false positive splat
From: Russell King - ARM Linux admin @ 2019-02-22 17:48 UTC (permalink / raw)
  To: Andrew Lunn; +Cc: netdev, Vivien Didelot
In-Reply-To: <20190217195524.rhc4rsvzsalptyua@shell.armlinux.org.uk>

On Sun, Feb 17, 2019 at 07:55:24PM +0000, Russell King - ARM Linux admin wrote:
> On Sun, Feb 17, 2019 at 07:11:43PM +0100, Andrew Lunn wrote:
> > The following false positive lockdep splat has been observed.
> > 
> > ======================================================
> > WARNING: possible circular locking dependency detected
> > 4.20.0+ #302 Not tainted
> > ------------------------------------------------------
> > systemd-udevd/160 is trying to acquire lock:
> > edea6080 (&chip->reg_lock){+.+.}, at: __setup_irq+0x640/0x704
> > 
> > but task is already holding lock:
> > edff0340 (&desc->request_mutex){+.+.}, at: __setup_irq+0xa0/0x704
> > 
> > which lock already depends on the new lock.
> > 
> > the existing dependency chain (in reverse order) is:
> > 
> > -> #1 (&desc->request_mutex){+.+.}:
> >        mutex_lock_nested+0x1c/0x24
> >        __setup_irq+0xa0/0x704
> >        request_threaded_irq+0xd0/0x150
> >        mv88e6xxx_probe+0x41c/0x694 [mv88e6xxx]
> >        mdio_probe+0x2c/0x54
> >        really_probe+0x200/0x2c4
> >        driver_probe_device+0x5c/0x174
> >        __driver_attach+0xd8/0xdc
> >        bus_for_each_dev+0x58/0x7c
> >        bus_add_driver+0xe4/0x1f0
> >        driver_register+0x7c/0x110
> >        mdio_driver_register+0x24/0x58
> >        do_one_initcall+0x74/0x2e8
> >        do_init_module+0x60/0x1d0
> >        load_module+0x1968/0x1ff4
> >        sys_finit_module+0x8c/0x98
> >        ret_fast_syscall+0x0/0x28
> >        0xbedf2ae8
> > 
> > -> #0 (&chip->reg_lock){+.+.}:
> >        __mutex_lock+0x50/0x8b8
> >        mutex_lock_nested+0x1c/0x24
> >        __setup_irq+0x640/0x704
> >        request_threaded_irq+0xd0/0x150
> >        mv88e6xxx_g2_irq_setup+0xcc/0x1b4 [mv88e6xxx]
> >        mv88e6xxx_probe+0x44c/0x694 [mv88e6xxx]
> >        mdio_probe+0x2c/0x54
> >        really_probe+0x200/0x2c4
> >        driver_probe_device+0x5c/0x174
> >        __driver_attach+0xd8/0xdc
> >        bus_for_each_dev+0x58/0x7c
> >        bus_add_driver+0xe4/0x1f0
> >        driver_register+0x7c/0x110
> >        mdio_driver_register+0x24/0x58
> >        do_one_initcall+0x74/0x2e8
> >        do_init_module+0x60/0x1d0
> >        load_module+0x1968/0x1ff4
> >        sys_finit_module+0x8c/0x98
> >        ret_fast_syscall+0x0/0x28
> >        0xbedf2ae8
> > 
> > other info that might help us debug this:
> > 
> >  Possible unsafe locking scenario:
> > 
> >        CPU0                    CPU1
> >        ----                    ----
> >   lock(&desc->request_mutex);
> >                                lock(&chip->reg_lock);
> >                                lock(&desc->request_mutex);
> >   lock(&chip->reg_lock);
> > 
> > &desc->request_mutex refer to two different mutex. #1 is the GPIO for
> > the chip interrupt. #2 is the chained interrupt between global 1 and
> > global 2.
> > 
> > Add lockdep classes to the GPIO interrupt to avoid this.
> > 
> > Reported-by: Russell King <rmk+kernel@armlinux.org.uk>
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > ---
> > 
> > Hi Russell
> > 
> > Does this fix it for you on Clearfog?
> 
> Yes, that also fixes the problem, but I do think this is just papering
> over mv88e6xxx needlessly holding locks when it doesn't need to do so.

Hi Andrew,

Do we have a way forward for this issue?

Thanks.

> 
> > 
> > drivers/net/dsa/mv88e6xxx/chip.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> > index 32e7af5caa69..936d53a92144 100644
> > --- a/drivers/net/dsa/mv88e6xxx/chip.c
> > +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> > @@ -442,12 +442,20 @@ static int mv88e6xxx_g1_irq_setup_common(struct mv88e6xxx_chip *chip)
> >  
> >  static int mv88e6xxx_g1_irq_setup(struct mv88e6xxx_chip *chip)
> >  {
> > +	static struct lock_class_key lock_key;
> > +	static struct lock_class_key request_key;
> >  	int err;
> >  
> >  	err = mv88e6xxx_g1_irq_setup_common(chip);
> >  	if (err)
> >  		return err;
> >  
> > +	/* These lock classes tells lockdep that global 1 irqs are in
> > +	 * a different category than their parent GPIO, so it won't
> > +	 * report false recursion.
> > +	 */
> > +	irq_set_lockdep_class(chip->irq, &lock_key, &request_key);
> > +
> >  	err = request_threaded_irq(chip->irq, NULL,
> >  				   mv88e6xxx_g1_irq_thread_fn,
> >  				   IRQF_ONESHOT | IRQF_SHARED,
> > -- 
> > 2.20.1
> > 
> > 
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
> According to speedtest.net: 11.9Mbps down 500kbps up

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [PATCH net 1/2] selftests: pmtu: disable dad in all namespaces
From: Stefano Brivio @ 2019-02-22 17:49 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <1d298b6839828c96cd8a0f50bdfac893cc645895.1550851038.git.pabeni@redhat.com>

On Fri, 22 Feb 2019 17:06:32 +0100
Paolo Abeni <pabeni@redhat.com> wrote:

> Otherwise, the configured IPv6 address could be still "tentative"
> at test time, possibly causing tests failures.
> We can also drop some sleep along the code and decrease the
> timeout for most commands so that the test runtime decreases.

Thanks for fixing this!

I tried this back then and it didn't work, perhaps some DAD changes
intervened meanwhile (or I simply tried it wrong).

Just one comment:

> Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  tools/testing/selftests/net/pmtu.sh | 28 +++++++++++++---------------
>  1 file changed, 13 insertions(+), 15 deletions(-)
> 
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index e2c94e47707c..634e91e8fe25 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -263,8 +263,6 @@ setup_fou_or_gue() {
>  
>  	${ns_a} ip link set ${encap}_a up
>  	${ns_b} ip link set ${encap}_b up
> -
> -	sleep 1
>  }
>  
>  setup_fou44() {
> @@ -302,6 +300,10 @@ setup_gue66() {
>  setup_namespaces() {
>  	for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
>  		ip netns add ${n} || return 1
> +
> +		# disable dad, so that we don't have to wait to use the
> +		# configured IPv6 addresses

For consistency: "Disable DAD ..."

-- 
Stefano

^ permalink raw reply

* Re: [PATCH net 2/2] selftests: pmtu: add explicit tests for pmtu exceptions cleanup
From: Stefano Brivio @ 2019-02-22 17:50 UTC (permalink / raw)
  To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern
In-Reply-To: <06a88ed3b2efab312f6301e9fe8eabd4383a9d3a.1550851038.git.pabeni@redhat.com>

On Fri, 22 Feb 2019 17:06:33 +0100
Paolo Abeni <pabeni@redhat.com> wrote:

> Add a couple of new tests, explicitly checking that the kernel
> timely releases pmtu exceptions on related device removal.
> This is mostly a regression test vs the issue fixed by:
> https://patchwork.ozlabs.org/patch/1045488/
> 
> Only 2 new test cases have been added, instead of extending all
> the existing ones, because the reproducer requires executing
> several commands and would slow down too much the tests otherwise.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>  tools/testing/selftests/net/pmtu.sh | 52 ++++++++++++++++++++++++++++-
>  1 file changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
> index 634e91e8fe25..4bc72bc26899 100755
> --- a/tools/testing/selftests/net/pmtu.sh
> +++ b/tools/testing/selftests/net/pmtu.sh
> @@ -135,7 +135,9 @@ tests="
>  	pmtu_vti6_default_mtu		vti6: default MTU assignment
>  	pmtu_vti4_link_add_mtu		vti4: MTU setting on link creation
>  	pmtu_vti6_link_add_mtu		vti6: MTU setting on link creation
> -	pmtu_vti6_link_change_mtu	vti6: MTU changes on link changes"
> +	pmtu_vti6_link_change_mtu	vti6: MTU changes on link changes
> +	pmtu_ipv4_exception_cleanup	ipv4: cleanup of cached exceptions
> +	pmtu_ipv6_exception_cleanup	ipv6: cleanup of cached exceptions"
>  
>  NS_A="ns-$(mktemp -u XXXXXX)"
>  NS_B="ns-$(mktemp -u XXXXXX)"
> @@ -1006,6 +1008,54 @@ test_pmtu_vti6_link_change_mtu() {
>  	return ${fail}
>  }
>  
> +test_cleanup_vxlanY_or_geneveY_exception() {
> +	outer="${1}"
> +	encap="${2}"

As you don't implement GENEVE tests, this function can be kept simpler.

> +	ll_mtu=4000
> +
> +	which taskset 2>/dev/null | return 2
> +	which timeout 2>/dev/null | return 2

This test will not be skipped if taskset or timeout are not available,
I guess you meant: || return 2.

Besides, 'which' can output to both stdout and stderr. See how it's
used anywhere else in this script, e.g.:

	which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)

You might also want to skip this test on non-SMP.

You should also indicate why the test is skipped, see how it's done in
other places, e.g.:

	${ns_a} ip link add dummy0 mtu 1500 type dummy
	[ $? -ne 0 ] && err "  dummy not supported" && return 2

> +	cpu_list=`grep processor /proc/cpuinfo | cut -d ' ' -f 2`

For consistency, please use $( ... )

If you grep -m2, then you can skip the ncpus thing below.

> +
> +	setup namespaces routing ${encap}${outer} || return 2
> +	trace "${ns_a}" ${encap}_a   "${ns_b}"  ${encap}_b \
> +	      "${ns_a}" veth_A-R1    "${ns_r1}" veth_R1-A \
> +	      "${ns_b}" veth_B-R1    "${ns_r1}" veth_R1-B
> +
> +	# Create route exception by exceeding link layer MTU
> +	mtu "${ns_a}"  veth_A-R1 $((${ll_mtu} + 1000))
> +	mtu "${ns_r1}" veth_R1-A $((${ll_mtu} + 1000))
> +	mtu "${ns_b}"  veth_B-R1 ${ll_mtu}
> +	mtu "${ns_r1}" veth_R1-B ${ll_mtu}
> +
> +	mtu "${ns_a}" ${encap}_a $((${ll_mtu} + 1000))
> +	mtu "${ns_b}" ${encap}_b $((${ll_mtu} + 1000))
> +
> +	# fill exception cache for multiple cpus [2]
> +	# we can always use inner ipv4 for that

Please keep comments consistent with the rest, "Fill ... CPUs (2) ... IPv4".

> +	ncpus=0
> +	for cpu in $cpu_list; do

Nit: ${cpu_list}

> +		taskset --cpu-list ${cpu} ${ns_a} ping -q -M want -i 0.1 -w 1 -s $((${ll_mtu} + 500)) ${tunnel4_b_addr} > /dev/null
> +		ncpus=$((ncpus + 1))
> +		[ ${ncpus} -gt 1 ] && break
> +	done
> +
> +	fail=0

No need for fail=0...

> +	if ! timeout 1 ${ns_a} ip link del dev veth_A-R1; then
> +		err "  can't delete veth device in a timely manner, pmtu dst likely leaked"

(For consistency: "PMTU")

> +		fail=1

...just return 1 here and

> +	fi
> +	return ${fail}

...skip the explicit return. The return code comes from the last
command executed.

> +}
> +
> +test_pmtu_ipv6_exception_cleanup() {
> +	test_cleanup_vxlanY_or_geneveY_exception 6 vxlan
> +}
> +
> +test_pmtu_ipv4_exception_cleanup() {
> +	test_cleanup_vxlanY_or_geneveY_exception 4 vxlan
> +}
> +
>  usage() {
>  	echo
>  	echo "$0 [OPTIONS] [TEST]..."

-- 
Stefano

^ permalink raw reply

* Re: [PATCH v2 2/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-22 17:50 UTC (permalink / raw)
  To: David Miller
  Cc: paul@paul-moore.com, netdev@vger.kernel.org,
	linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
	yoshfuji@linux-ipv6.org
In-Reply-To: <8873761550853329@myt6-67cd1de25d8a.qloud-c.yandex.net>

Extract IP options in cipso_v4_error and use __icmp_send.
---
 include/net/ip.h      |    2 ++
 net/ipv4/cipso_ipv4.c |   17 +++++++++++++++--
 net/ipv4/ip_options.c |   22 +++++++++++++++++-----
 3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/include/net/ip.h b/include/net/ip.h
index 8866bfc..f0e8d06 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -667,6 +667,8 @@ static inline int ip_options_echo(struct net *net, struct ip_options *dopt,
 }
 
 void ip_options_fragment(struct sk_buff *skb);
+int __ip_options_compile(struct net *net, struct ip_options *opt,
+			 struct sk_buff *skb, __be32 *info);
 int ip_options_compile(struct net *net, struct ip_options *opt,
 		       struct sk_buff *skb);
 int ip_options_get(struct net *net, struct ip_options_rcu **optp,
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b..eff86a7 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1735,13 +1735,26 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
  */
 void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
 {
+	unsigned char optbuf[sizeof(struct ip_options) + 40];
+	struct ip_options *opt = (struct ip_options *)optbuf;
+
 	if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
 		return;
 
+	/*
+	 * We might be called above the IP layer,
+	 * so we can not use icmp_send and IPCB here.
+	 */
+
+	memset(opt, 0, sizeof(struct ip_options));
+	opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
+	if (__ip_options_compile(dev_net(skb->dev), opt, skb, NULL))
+		return;
+
 	if (gateway)
-		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
+		__icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
 	else
-		icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
+		__icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
 }
 
 /**
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ed194d4..32a3504 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -251,8 +251,9 @@ static void spec_dst_fill(__be32 *spec_dst, struct sk_buff *skb)
  * If opt == NULL, then skb->data should point to IP header.
  */
 
-int ip_options_compile(struct net *net,
-		       struct ip_options *opt, struct sk_buff *skb)
+int __ip_options_compile(struct net *net,
+			 struct ip_options *opt, struct sk_buff *skb,
+			 __be32 *info)
 {
 	__be32 spec_dst = htonl(INADDR_ANY);
 	unsigned char *pp_ptr = NULL;
@@ -468,11 +469,22 @@ int ip_options_compile(struct net *net,
 		return 0;
 
 error:
-	if (skb) {
-		icmp_send(skb, ICMP_PARAMETERPROB, 0, htonl((pp_ptr-iph)<<24));
-	}
+	if (info)
+		*info = htonl((pp_ptr-iph)<<24);
 	return -EINVAL;
 }
+
+int ip_options_compile(struct net *net,
+		       struct ip_options *opt, struct sk_buff *skb)
+{
+	int ret;
+	__be32 info;
+
+	ret = __ip_options_compile(net, opt, skb, &info);
+	if (ret != 0 && skb)
+		icmp_send(skb, ICMP_PARAMETERPROB, 0, info);
+	return ret;
+}
 EXPORT_SYMBOL(ip_options_compile);
 
 /*
---

^ permalink raw reply related

* Re: [PATCH net] net: socket: set sock->sk to NULL after calling proto_ops::release()
From: Eric Biggers @ 2019-02-22 17:57 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: netdev, David S . Miller, linux-kernel, Mao Wenan, Cong Wang,
	Lorenzo Colitti, Tetsuo Handa, Al Viro
In-Reply-To: <e4d0f1e8-68ff-512e-ad69-480523ff300f@gmail.com>

Hi Eric,

On Fri, Feb 22, 2019 at 09:45:35AM -0800, Eric Dumazet wrote:
> 
> 
> On 02/21/2019 02:13 PM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Commit 9060cb719e61 ("net: crypto set sk to NULL when af_alg_release.")
> > fixed a use-after-free in sockfs_setattr() when an AF_ALG socket is
> > closed concurrently with fchownat().  However, it ignored that many
> > other proto_ops::release() methods don't set sock->sk to NULL and
> > therefore allow the same use-after-free:
> > 
> 
> I fail to see how setting a pointer to NULL can avoid races.
> 
> 
> We lack some kind of protection, rcu or something, if another thread can change sock->sk at anytime
> while sockfs_setattr() is used.
> 
> sockfs_setattr()
> ...
>      if (sock->sk)
> 
> // even if sock->sk was not NULL for the if (...).
> 
> // it can be NULL right now, compiler could read sock->sk a second time and catch a NULL.
> 
>         sock->sk->sk_uid = iattr->ia_uid;
> 
> 

->setattr() is called under inode_lock(), which __sock_release() also takes.  So
the uses of sock->sk are serialized.  See commit 6d8c50dcb029 ("socket: close
race condition between sock_close() and sockfs_setattr()").

The issue now is that if ->setattr() happens *after* __sock_release() (which is
possible if fchownat() gets the reference to the file's 'struct path', then the
file is close()d by another thread, then fchownat() continues), it will see
stale sock->sk because for many socket types it wasn't set to NULL earlier.

- Eric

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox