* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Eric Dumazet @ 2019-02-26 4:36 UTC (permalink / raw)
To: Hangbin Liu, David Ahern; +Cc: netdev, Stefano Brivio, David S . Miller
In-Reply-To: <20190226040759.GU10051@dhcp-12-139.nay.redhat.com>
On 02/25/2019 08:08 PM, Hangbin Liu wrote:
> Hi David,
> On Mon, Feb 25, 2019 at 07:15:26PM -0700, David Ahern wrote:
>> On 2/25/19 6:55 PM, Hangbin Liu wrote:
>>> Just as I said, this issue only occurs when IPv6 is disabled at boot time
>>> as there is no IPv6 route entry. Disable ipv6 on specific interface should
>>> not be affected(IPv6 route/fib has inited). So I think use ipv6_mod_enabled()
>>> would be more suitable in this scenario. Did I miss something?
>>
>> From a readability perspective the code path depends on whether ipv6 is
>> enabled on the device. I think it is better to leave that as it is.
>
> When I posted 173656accaf5 ("sit: check if IPv6 enabled before calling
> ip6_err_gen_icmpv6_unreach()"), my purpose is to check if IPv6 disabled
> at boot time. I didn't know we have ipv6_mod_enabled() at that time, so I
> just used __in6_dev_get() as a trick way/work around.
>
> A few days later I saw your commit e434863718d4 ("net: vrf: Fix crash when
> IPv6 is disabled at boot time") and I thought this would be a more clear way
> to tell people that we are checking if IPv6 disabled at boot time. So I posted
> these two follow up fixes.
>
> I don't know why you thought check IPv6 is enbled on the device is better.
> Because it's more strict? Maybe I missed something here. But if you feel it
> is better to leave as it it, then let's keep it.
>
I do not get it.
We really do not care if IPv6 is enabled on some device on the host.
Here the correct test is checking if IPv6 is enabled for _this_ device.
It is not about fixing a crash (it is already fixed), but not having to call
ip6_err_gen_icmpv6_unreach() knowing it will fail anyway.
So the current code is better.
Thank you.
^ permalink raw reply
* Re: [PATCH] tools: testing: selftests: Remove duplicate headers
From: Souptick Joarder @ 2019-02-26 5:29 UTC (permalink / raw)
To: Michael Ellerman
Cc: Bamvor Zhang, shuah, David S. Miller, Benjamin Herrenschmidt,
Paul Mackerras, Alexey Dobriyan, Mathieu Desnoyers,
Peter Zijlstra, Paul McKenney, boqun.feng, John Stultz,
Thomas Gleixner, sboyd, Andrew Morton, linux-gpio,
linux-kselftest, linux-kernel, netdev, linuxppc-dev,
linux-fsdevel, Sabyasachi Gupta
In-Reply-To: <87tvgr1fr4.fsf@concordia.ellerman.id.au>
On Tue, Feb 26, 2019 at 7:18 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Souptick Joarder <jrdr.linux@gmail.com> writes:
> > Remove duplicate headers which are included twice.
> >
> > Signed-off-by: Sabyasachi Gupta <sabyasachi.linux@gmail.com>
> > Signed-off-by: Souptick Joarder <jrdr.linux@gmail.com>
> > ---
> ...
> > tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c | 1 -
>
> I took this hunk via the powerpc tree.
How about taking this entirely through a single tree ?
or Shall I send these changes in different patches ?
>
> > diff --git a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> > index 167135b..af1b802 100644
> > --- a/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> > +++ b/tools/testing/selftests/powerpc/pmu/ebb/fork_cleanup_test.c
> > @@ -11,7 +11,6 @@
> > #include <sys/wait.h>
> > #include <unistd.h>
> > #include <setjmp.h>
> > -#include <signal.h>
> >
> > #include "ebb.h"
>
>
> cheers
^ permalink raw reply
* Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta
From: kbuild test robot @ 2019-02-26 5:31 UTC (permalink / raw)
To: Leslie Monis
Cc: kbuild-all, jhs, netdev, dave, Mohit P. Tahiliani, Dhaval Khandla,
Hrishikesh Hiraskar, Manish Kumar B, Sachin D . Patil,
Leslie Monis
In-Reply-To: <20190225102051.12268-6-lesliemonis@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1179 bytes --]
Hi Mohit,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Leslie-Monis/net-sched-pie-align-PIE-implementation-with-RFC-8033/20190226-041701
config: arm-allyesconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=arm
All errors (new ones prefixed by >>):
arm-linux-gnueabi-ld: net/sched/sch_pie.o: in function `pie_timer':
>> sch_pie.c:(.text+0xc3c): undefined reference to `__aeabi_uldivmod'
>> arm-linux-gnueabi-ld: sch_pie.c:(.text+0xed8): undefined reference to `__aeabi_uldivmod'
arm-linux-gnueabi-ld: net/sched/sch_pie.o: in function `pie_qdisc_enqueue':
sch_pie.c:(.text+0x168c): undefined reference to `__aeabi_uldivmod'
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 67762 bytes --]
^ permalink raw reply
* Re: [PATCH 1/3] bpf: add helper to check for a valid SYN cookie
From: Martin Lau @ 2019-02-26 5:37 UTC (permalink / raw)
To: Lorenz Bauer
Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
linux-api@vger.kernel.org
In-Reply-To: <CACAyw9_smq+c8rUt3ONk+J28CzPefuEOgs8at3q9_5s3zN0_1w@mail.gmail.com>
On Mon, Feb 25, 2019 at 06:26:42PM +0000, Lorenz Bauer wrote:
> On Sat, 23 Feb 2019 at 00:44, Martin Lau <kafai@fb.com> wrote:
> >
> > On Fri, Feb 22, 2019 at 09:50:55AM +0000, Lorenz Bauer wrote:
> > > Using bpf_sk_lookup_tcp it's possible to ascertain whether a packet belongs
> > > to a known connection. However, there is one corner case: no sockets are
> > > created if SYN cookies are active. This means that the final ACK in the
> > > 3WHS is misclassified.
> > >
> > > Using the helper, we can look up the listening socket via bpf_sk_lookup_tcp
> > > and then check whether a packet is a valid SYN cookie ACK.
> > >
> > > Signed-off-by: Lorenz Bauer <lmb@cloudflare.com>
> > > ---
> > > include/uapi/linux/bpf.h | 18 ++++++++++-
> > > net/core/filter.c | 68 ++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 85 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > > index bcdd2474eee7..bc2af87e9621 100644
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -2359,6 +2359,21 @@ union bpf_attr {
> > > * Return
> > > * A **struct bpf_tcp_sock** pointer on success, or NULL in
> > > * case of failure.
> > > + *
> > > + * int bpf_sk_check_syncookie(struct bpf_sock *sk, void *iph, u32 iph_len, struct tcphdr *th, u32 th_len)
> > > + * Description
> > > + * Check whether iph and th contain a valid SYN cookie ACK for
> > > + * the listening socket in sk.
> > > + *
> > > + * iph points to the start of the IPv4 or IPv6 header, while
> > > + * iph_len contains sizeof(struct iphdr) or sizeof(struct ip6hdr).
> > > + *
> > > + * th points to the start of the TCP header, while th_len contains
> > > + * sizeof(struct tcphdr).
> > > + *
> > > + * Return
> > > + * 0 if iph and th are a valid SYN cookie ACK, or a negative error
> > > + * otherwise.
> > > */
> > > #define __BPF_FUNC_MAPPER(FN) \
> > > FN(unspec), \
> > > @@ -2457,7 +2472,8 @@ union bpf_attr {
> > > FN(spin_lock), \
> > > FN(spin_unlock), \
> > > FN(sk_fullsock), \
> > > - FN(tcp_sock),
> > > + FN(tcp_sock), \
> > > + FN(sk_check_syncookie),
> > >
> > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> > > * function eBPF program intends to call
> > > diff --git a/net/core/filter.c b/net/core/filter.c
> > > index 85749f6ec789..9e68897cc7ed 100644
> > > --- a/net/core/filter.c
> > > +++ b/net/core/filter.c
> > > @@ -5426,6 +5426,70 @@ static const struct bpf_func_proto bpf_tcp_sock_proto = {
> > > .arg1_type = ARG_PTR_TO_SOCK_COMMON,
> > > };
> > >
> > > +BPF_CALL_5(bpf_sk_check_syncookie, struct sock *, sk, void *, iph, u32, iph_len,
> > s/bpf_sk_check_syncookie/bpf_tcp_check_syncookie/>
> >
> > > + struct tcphdr *, th, u32, th_len)
> > > +{
> > > +#if IS_ENABLED(CONFIG_SYN_COOKIES)
> > nit. "#ifdef CONFIG_SYN_COOKIES" such that it is clear it is a bool kconfig.
> >
> > > + u32 cookie;
> > > + int ret;
> > > +
> > > + if (unlikely(th_len < sizeof(*th)))
> > > + return -EINVAL;
> > > +
> > > + /* sk_listener() allows TCP_NEW_SYN_RECV, which makes no sense here. */
> > > + if (sk->sk_protocol != IPPROTO_TCP || sk->sk_state != TCP_LISTEN)
> > From the test program in patch 3, the "sk" here is obtained from
> > bpf_sk_lookup_tcp() which does a sk_to_full_sk() before returning.
> > AFAICT, meaning bpf_sk_lookup_tcp() will return the listening sk
> > even if there is a request_sock. Does it make sense to check
> > syncookie if there is already a request_sock?
>
> No, that doesn't make a lot of sense. I hadn't realised that
> sk_lookup_tcp only returns full sockets.
> This means we need a way to detect that there is a request sock for a
> given tuple.
>
> * adding a reqsk_exists(tuple) helper means we have to pay the lookup cost twice
> * drop the sk argument and do the necessary lookups in the helper
> itself, but that also
> wastes a call to __inet_lookup_listener
> * skip sk_to_full_sk() in a helper and return RET_PTR_TO_SOCK_COMMON,
> but that violates a bunch of assumptions (e.g. calling bpf_sk_release on them)
How about creating a new lookup helper, bpf_sk"c"_lookup_tcp,
that does not call sk_to_full_sk() before returning.
Its ".ret_type" will be RET_PTR_TO_SOCK_COMMON_OR_NULL which its
reference(-counting) state has to be tracked in the verifier also.
Mainly in check_helper_call(), iirc.
The bpf_prog can then check bpf_sock->state for TCP_LISTEN,
call bpf_tcp_sock() to get the TCP listener sock and pass to
the bpf_tcp_check_syncookie()
>
> For context: ultimately we want use this to answer the question: does
> this (encapsulated)
> packet contain a payload destined to a local socket? Amongst the edge
> cases we need to
> handle are ICMP Packet Too Big messages and SYN cookies. A solution
> would be to hide
> all this in an "uber" helper that takes pointers to the L3 / L4
> headers and returns a verdict,
> but that seems a bit gross.
Please include this use case in the commit message.
It is useful.
>
> >
> > > + return -EINVAL;
> > > +
> > > + if (!sock_net(sk)->ipv4.sysctl_tcp_syncookies)
> > Should tcp_synq_no_recent_overflow(tp) be checked also?
> >
>
> Yes, not sure how that slipped out.
>
> > > + return -EINVAL;
> > > +
> > > + if (!th->ack || th->rst)
> > How about th->syn?
> >
>
> Yes, I missed the fact that the callers in tcp_ipv{4,6}.c check this.
>
> > > + return -ENOENT;
> > > +
> > > + cookie = ntohl(th->ack_seq) - 1;
> > > +
> > > + switch (sk->sk_family) {
> > > + case AF_INET:
> > > + if (unlikely(iph_len < sizeof(struct iphdr)))
> > > + return -EINVAL;
> > > +
> > > + ret = __cookie_v4_check((struct iphdr *)iph, th, cookie);
> > > + break;
> > > +
> > > +#if IS_ENABLED(CONFIG_IPV6)
> > > + case AF_INET6:
> > > + if (unlikely(iph_len < sizeof(struct ipv6hdr)))
> > > + return -EINVAL;
> > > +
> > > + ret = __cookie_v6_check((struct ipv6hdr *)iph, th, cookie);
> > > + break;
> > > +#endif /* CONFIG_IPV6 */
> > > +
> > > + default:
> > > + return -EPROTONOSUPPORT;
> > > + }
> > > +
> > > + if (ret > 0)
> > > + return 0;
> > > +
> > > + return -ENOENT;
> > > +#else
> > > + return -ENOTSUP;
> > > +#endif
> > > +}
> > > +
> > > +static const struct bpf_func_proto bpf_sk_check_syncookie_proto = {
> > > + .func = bpf_sk_check_syncookie,
> > > + .gpl_only = true,
> > > + .pkt_access = true,
> > > + .ret_type = RET_INTEGER,
> > > + .arg1_type = ARG_PTR_TO_SOCKET,
> > I think it should be ARG_PTR_TO_TCP_SOCK
> >
> > > + .arg2_type = ARG_PTR_TO_MEM,
> > > + .arg3_type = ARG_CONST_SIZE,
> > > + .arg4_type = ARG_PTR_TO_MEM,
> > > + .arg5_type = ARG_CONST_SIZE,
> > > +};
> > > +
> > > #endif /* CONFIG_INET */
>
>
>
> --
> Lorenz Bauer | Systems Engineer
> 25 Lavington St., London SE1 0NZ
>
> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.cloudflare.com&d=DwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=VQnoQ7LvghIj0gVEaiQSUw&m=xhDwvX3iD-mbqSrx-L8XQNaZiYFZzMWNo_2Y38Z9j34&s=I4Ag3HflabFppFv7UtMp8WnMVSqCDW0W28ziWIvuwDE&e=
^ permalink raw reply
* [PATCH net-next 0/2] vxlan: create and changelink extack support
From: Roopa Prabhu @ 2019-02-26 6:03 UTC (permalink / raw)
To: davem; +Cc: netdev, dsa, sd, johannes
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This series adds extack support to changelink paths.
In the process re-factors flag sets to a separate helper.
Also adds some changelink testcases to rtnetlink.sh
(This series was initially part of another series that
tried to support changelink for more attributes.
But after some feedback from sabrina, i have dropped the
'support changelink for more attributes' part because some
of them cannot be supported today or may require additional
use-case handling code. These can be done separately
as and when we see the need for it.)
Roopa Prabhu (2):
vxlan: add extack support for create and changelink
tools: selftests: rtnetlink: add testcases for vxlan flag sets
drivers/net/vxlan.c | 208 +++++++++++++++++++++----------
include/net/vxlan.h | 31 +++++
tools/testing/selftests/net/rtnetlink.sh | 52 ++++++++
3 files changed, 224 insertions(+), 67 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH net-next 1/2] vxlan: add extack support for create and changelink
From: Roopa Prabhu @ 2019-02-26 6:03 UTC (permalink / raw)
To: davem; +Cc: netdev, dsa, sd, johannes
In-Reply-To: <1551160982-32685-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch adds extack coverage in vxlan link
create and changelink paths. Introduces a new helper
vxlan_nl2flags to consolidate flag attribute validation.
thanks to Johannes Berg for some tips to construct the
generic vxlan flag extack strings.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
drivers/net/vxlan.c | 208 +++++++++++++++++++++++++++++++++++-----------------
include/net/vxlan.h | 31 ++++++++
2 files changed, 172 insertions(+), 67 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 577201c..a3c46d7 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -3583,11 +3583,40 @@ static int __vxlan_dev_create(struct net *net, struct net_device *dev,
return err;
}
+/* Set/clear flags based on attribute */
+static int vxlan_nl2flag(struct vxlan_config *conf, struct nlattr *tb[],
+ int attrtype, unsigned long mask, bool changelink,
+ bool changelink_supported,
+ struct netlink_ext_ack *extack)
+{
+ unsigned long flags;
+
+ if (!tb[attrtype])
+ return 0;
+
+ if (changelink && !changelink_supported) {
+ vxlan_flag_attr_error(attrtype, extack);
+ return -EOPNOTSUPP;
+ }
+
+ if (vxlan_policy[attrtype].type == NLA_FLAG)
+ flags = conf->flags | mask;
+ else if (nla_get_u8(tb[attrtype]))
+ flags = conf->flags | mask;
+ else
+ flags = conf->flags & ~mask;
+
+ conf->flags = flags;
+
+ return 0;
+}
+
static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
struct net_device *dev, struct vxlan_config *conf,
- bool changelink)
+ bool changelink, struct netlink_ext_ack *extack)
{
struct vxlan_dev *vxlan = netdev_priv(dev);
+ int err = 0;
memset(conf, 0, sizeof(*conf));
@@ -3598,40 +3627,54 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
if (data[IFLA_VXLAN_ID]) {
__be32 vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
- if (changelink && (vni != conf->vni))
+ if (changelink && (vni != conf->vni)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_ID], "Cannot change VNI");
return -EOPNOTSUPP;
+ }
conf->vni = cpu_to_be32(nla_get_u32(data[IFLA_VXLAN_ID]));
}
if (data[IFLA_VXLAN_GROUP]) {
- if (changelink && (conf->remote_ip.sa.sa_family != AF_INET))
+ if (changelink && (conf->remote_ip.sa.sa_family != AF_INET)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP], "New group address family does not match old group");
return -EOPNOTSUPP;
+ }
conf->remote_ip.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_GROUP]);
conf->remote_ip.sa.sa_family = AF_INET;
} else if (data[IFLA_VXLAN_GROUP6]) {
- if (!IS_ENABLED(CONFIG_IPV6))
+ if (!IS_ENABLED(CONFIG_IPV6)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "IPv6 support not enabled in the kernel");
return -EPFNOSUPPORT;
+ }
- if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6))
+ if (changelink && (conf->remote_ip.sa.sa_family != AF_INET6)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_GROUP6], "New group address family does not match old group");
return -EOPNOTSUPP;
+ }
conf->remote_ip.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_GROUP6]);
conf->remote_ip.sa.sa_family = AF_INET6;
}
if (data[IFLA_VXLAN_LOCAL]) {
- if (changelink && (conf->saddr.sa.sa_family != AF_INET))
+ if (changelink && (conf->saddr.sa.sa_family != AF_INET)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL], "New local address family does not match old");
return -EOPNOTSUPP;
+ }
conf->saddr.sin.sin_addr.s_addr = nla_get_in_addr(data[IFLA_VXLAN_LOCAL]);
conf->saddr.sa.sa_family = AF_INET;
} else if (data[IFLA_VXLAN_LOCAL6]) {
- if (!IS_ENABLED(CONFIG_IPV6))
+ if (!IS_ENABLED(CONFIG_IPV6)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "IPv6 support not enabled in the kernel");
return -EPFNOSUPPORT;
+ }
- if (changelink && (conf->saddr.sa.sa_family != AF_INET6))
+ if (changelink && (conf->saddr.sa.sa_family != AF_INET6)) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LOCAL6], "New local address family does not match old");
return -EOPNOTSUPP;
+ }
/* TODO: respect scope id */
conf->saddr.sin6.sin6_addr = nla_get_in6_addr(data[IFLA_VXLAN_LOCAL6]);
@@ -3648,9 +3691,12 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
conf->ttl = nla_get_u8(data[IFLA_VXLAN_TTL]);
if (data[IFLA_VXLAN_TTL_INHERIT]) {
- if (changelink)
- return -EOPNOTSUPP;
- conf->flags |= VXLAN_F_TTL_INHERIT;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_TTL_INHERIT,
+ VXLAN_F_TTL_INHERIT, changelink, false,
+ extack);
+ if (err)
+ return err;
+
}
if (data[IFLA_VXLAN_LABEL])
@@ -3658,10 +3704,11 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
IPV6_FLOWLABEL_MASK;
if (data[IFLA_VXLAN_LEARNING]) {
- if (nla_get_u8(data[IFLA_VXLAN_LEARNING]))
- conf->flags |= VXLAN_F_LEARN;
- else
- conf->flags &= ~VXLAN_F_LEARN;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_LEARNING,
+ VXLAN_F_LEARN, changelink, true,
+ extack);
+ if (err)
+ return err;
} else if (!changelink) {
/* default to learn on a new device */
conf->flags |= VXLAN_F_LEARN;
@@ -3671,44 +3718,52 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
conf->age_interval = nla_get_u32(data[IFLA_VXLAN_AGEING]);
if (data[IFLA_VXLAN_PROXY]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_PROXY]))
- conf->flags |= VXLAN_F_PROXY;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_PROXY,
+ VXLAN_F_PROXY, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_RSC]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_RSC]))
- conf->flags |= VXLAN_F_RSC;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_RSC,
+ VXLAN_F_RSC, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_L2MISS]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_L2MISS]))
- conf->flags |= VXLAN_F_L2MISS;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L2MISS,
+ VXLAN_F_L2MISS, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_L3MISS]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_L3MISS]))
- conf->flags |= VXLAN_F_L3MISS;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_L3MISS,
+ VXLAN_F_L3MISS, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_LIMIT]) {
- if (changelink)
+ if (changelink) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_LIMIT],
+ "Cannot change limit");
return -EOPNOTSUPP;
+ }
conf->addrmax = nla_get_u32(data[IFLA_VXLAN_LIMIT]);
}
if (data[IFLA_VXLAN_COLLECT_METADATA]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_COLLECT_METADATA]))
- conf->flags |= VXLAN_F_COLLECT_METADATA;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_COLLECT_METADATA,
+ VXLAN_F_COLLECT_METADATA, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_PORT_RANGE]) {
@@ -3718,72 +3773,92 @@ static int vxlan_nl2conf(struct nlattr *tb[], struct nlattr *data[],
conf->port_min = ntohs(p->low);
conf->port_max = ntohs(p->high);
} else {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT_RANGE],
+ "Cannot change port range");
return -EOPNOTSUPP;
}
}
if (data[IFLA_VXLAN_PORT]) {
- if (changelink)
+ if (changelink) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_PORT],
+ "Cannot change port");
return -EOPNOTSUPP;
+ }
conf->dst_port = nla_get_be16(data[IFLA_VXLAN_PORT]);
}
if (data[IFLA_VXLAN_UDP_CSUM]) {
- if (changelink)
+ if (changelink) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_VXLAN_UDP_CSUM],
+ "Cannot change UDP_CSUM flag");
return -EOPNOTSUPP;
+ }
if (!nla_get_u8(data[IFLA_VXLAN_UDP_CSUM]))
conf->flags |= VXLAN_F_UDP_ZERO_CSUM_TX;
}
if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_TX]))
- conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_TX;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_TX,
+ VXLAN_F_UDP_ZERO_CSUM6_TX, changelink,
+ false, extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_UDP_ZERO_CSUM6_RX]))
- conf->flags |= VXLAN_F_UDP_ZERO_CSUM6_RX;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_UDP_ZERO_CSUM6_RX,
+ VXLAN_F_UDP_ZERO_CSUM6_RX, changelink,
+ false, extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_REMCSUM_TX]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_TX]))
- conf->flags |= VXLAN_F_REMCSUM_TX;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_TX,
+ VXLAN_F_REMCSUM_TX, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_REMCSUM_RX]) {
- if (changelink)
- return -EOPNOTSUPP;
- if (nla_get_u8(data[IFLA_VXLAN_REMCSUM_RX]))
- conf->flags |= VXLAN_F_REMCSUM_RX;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_RX,
+ VXLAN_F_REMCSUM_RX, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_GBP]) {
- if (changelink)
- return -EOPNOTSUPP;
- conf->flags |= VXLAN_F_GBP;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GBP,
+ VXLAN_F_GBP, changelink, false, extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_GPE]) {
- if (changelink)
- return -EOPNOTSUPP;
- conf->flags |= VXLAN_F_GPE;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_GPE,
+ VXLAN_F_GPE, changelink, false,
+ extack);
+ if (err)
+ return err;
}
if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL]) {
- if (changelink)
- return -EOPNOTSUPP;
- conf->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
+ err = vxlan_nl2flag(conf, data, IFLA_VXLAN_REMCSUM_NOPARTIAL,
+ VXLAN_F_REMCSUM_NOPARTIAL, changelink,
+ false, extack);
+ if (err)
+ return err;
}
if (tb[IFLA_MTU]) {
- if (changelink)
+ if (changelink) {
+ NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU],
+ "Cannot change mtu");
return -EOPNOTSUPP;
+ }
conf->mtu = nla_get_u32(tb[IFLA_MTU]);
}
@@ -3800,7 +3875,7 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
struct vxlan_config conf;
int err;
- err = vxlan_nl2conf(tb, data, dev, &conf, false);
+ err = vxlan_nl2conf(tb, data, dev, &conf, false, extack);
if (err)
return err;
@@ -3817,8 +3892,7 @@ static int vxlan_changelink(struct net_device *dev, struct nlattr *tb[],
struct vxlan_config conf;
int err;
- err = vxlan_nl2conf(tb, data,
- dev, &conf, true);
+ err = vxlan_nl2conf(tb, data, dev, &conf, true, extack);
if (err)
return err;
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 0976781..00254a5 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -453,4 +453,35 @@ vxlan_fdb_clear_offload(const struct net_device *dev, __be32 vni)
}
#endif
+static inline void vxlan_flag_attr_error(int attrtype,
+ struct netlink_ext_ack *extack)
+{
+#define VXLAN_FLAG(flg) \
+ case IFLA_VXLAN_##flg: \
+ NL_SET_ERR_MSG_MOD(extack, \
+ "cannot change " #flg " flag"); \
+ break
+ switch (attrtype) {
+ VXLAN_FLAG(TTL_INHERIT);
+ VXLAN_FLAG(LEARNING);
+ VXLAN_FLAG(PROXY);
+ VXLAN_FLAG(RSC);
+ VXLAN_FLAG(L2MISS);
+ VXLAN_FLAG(L3MISS);
+ VXLAN_FLAG(COLLECT_METADATA);
+ VXLAN_FLAG(UDP_ZERO_CSUM6_TX);
+ VXLAN_FLAG(UDP_ZERO_CSUM6_RX);
+ VXLAN_FLAG(REMCSUM_TX);
+ VXLAN_FLAG(REMCSUM_RX);
+ VXLAN_FLAG(GBP);
+ VXLAN_FLAG(GPE);
+ VXLAN_FLAG(REMCSUM_NOPARTIAL);
+ default:
+ NL_SET_ERR_MSG_MOD(extack, \
+ "cannot change flag");
+ break;
+ }
+#undef VXLAN_FLAG
+}
+
#endif
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 2/2] tools: selftests: rtnetlink: add testcases for vxlan flag sets
From: Roopa Prabhu @ 2019-02-26 6:03 UTC (permalink / raw)
To: davem; +Cc: netdev, dsa, sd, johannes
In-Reply-To: <1551160982-32685-1-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
This patch extends rtnetlink.sh to cover some vxlan flag
netlink attribute sets.
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
tools/testing/selftests/net/rtnetlink.sh | 52 ++++++++++++++++++++++++++++++++
1 file changed, 52 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 78fc593..5baf35a 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -408,6 +408,58 @@ kci_test_encap_vxlan()
ip netns exec "$testns" ip link add link "$vxlan" name "$vlan" type vlan id 1
check_err $?
+ # changelink testcases
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan vni 43 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan group ffe5::5 dev "$devdummy" 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan ttl inherit 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan ttl 64
+ check_err $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan nolearning
+ check_err $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan proxy 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan norsc 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan l2miss 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan l3miss 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan external 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan udpcsum 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan udp6zerocsumtx 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan udp6zerocsumrx 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan remcsumtx 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan remcsumrx 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan gbp 2>/dev/null
+ check_fail $?
+
+ ip netns exec "$testns" ip link set dev "$vxlan" type vxlan gpe 2>/dev/null
+ check_fail $?
+
ip netns exec "$testns" ip link del "$vxlan"
check_err $?
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 0/2] bpf: context casting for tail call and gtrace prog type
From: Alexei Starovoitov @ 2019-02-26 6:18 UTC (permalink / raw)
To: Kris Van Hees; +Cc: netdev, bpf, daniel
In-Reply-To: <201902251554.x1PFsD3w017626@userv0121.oracle.com>
On Mon, Feb 25, 2019 at 07:54:13AM -0800, Kris Van Hees wrote:
>
> The goal is to further extend the BPF_PROG_TYPE_GTRACE implementation to
> support what tracers commonly need, and I am also looking at ways to further
> extend this model to allow more tracer-specific features as well without the
> need for adding a BPF program types for every tracer.
It seems by themselves the patches don't provide any new functionality,
but instead look like plumbing to call external code.
This is no-go.
There were several attempts to do so in the past, so we documented it here:
Documentation/bpf/bpf_design_QA.rst
Q: New functionality via kernel modules?
----------------------------------------
Q: Can BPF functionality such as new program or map types, new
helpers, etc be added out of kernel module code?
A: NO.
The answer is still the same.
^ permalink raw reply
* Re: [RFC] nasty corner case in unix_dgram_sendmsg()
From: Al Viro @ 2019-02-26 6:28 UTC (permalink / raw)
To: Rainer Weikusat, Jason Baron; +Cc: netdev
In-Reply-To: <20190225035121.GH2217@ZenIV.linux.org.uk>
On Mon, Feb 25, 2019 at 03:51:21AM +0000, Al Viro wrote:
> PS: unix_dgram_sendmsg() is really much too subtle for its own good -
> AFAICS, it *does* avoid blocking operations under sk->lock, but proof
> is considerably more complex than one would like it to be...
Several questions about the whole peer_wake mechanism:
1) why do we even bother with that for SOCK_SEQPACKET? AFAICS, there
we do *not* hit unix_wait_for_peer() on send - it's conditional upon
if (other != sk &&
unlikely(unix_peer(other) != sk && unix_recvq_full(other))) {
after we'd checked that other is non-NULL and not dead. For seqpacket
we start with both ends of connection having unix_peer() pointing to
each other and only changed to NULL when one of the ends gets closed.
The socket we'd passed to sendmsg is obviously not dead yet, and we'd
verified that other isn't dead either. So unix_peer(other) must be
equal to sk and we don't go into that if.
In unix_dgram_poll() we might get to unix_dgram_peer_wake_me() for
seqpacket, but only in case when other is dead. In that case
unix_dgram_peer_wake_me() will insert our peer_wake into queue,
see SOCK_DEAD, remove peer_wake from queue and return false.
AFAICS, that serves no purpose whatsoever...
2) the logics in sendmsg is very odd:
* if we'd detected n:1 send *and* found that we need to
wait, do so (not using the peer_wake - other's peer_wait is not
going away). No questions there.
* if we'd detected n:1 send *and* found that we need to
wait, but don't have any remaining timeout, we lock both ends
and check if unix_peer(sk) is (now) not equal to other, either
because it never had or because we'd been hit by connect(2) while
we'd been doing the locking. In that case we fail with -EAGAIN.
Fair enough, but
* if after relocking we see that unix_peer(sk) now
is equal to other, we arrange for wakeup forwarding from other's
peer_wait *and* if that has (likely) succeeded we fail with -EAGAIN.
Huh? What's the point? The only thing that depends upon that
wakeup forwarding is poll, and _that_ will set the forwarding up
on its own.
* if we'd failed (either because other is dead now or
because its queue is not full), we go back to restart_locked.
If it's dead, we'll sod off with ECONNREFUSED, if it's not
full anymore, we'll send the damn thing.
All of that comes at the cost of considerable headache in
unix_dgram_sendmsg() - flag-conditional locking, etc. Why do
we bother? What's wrong with simple "n:1 case detected, queue
full, no timeout left, return -EAGAIN and be done with that"?
IDGI... Am I missing something subtle going on here?
I understand what peer_wake is for, and the poll side of things
is fine; it's sendmsg one that looks weird. What's going on
there?
^ permalink raw reply
* Re: [RFC] nasty corner case in unix_dgram_sendmsg()
From: Al Viro @ 2019-02-26 6:38 UTC (permalink / raw)
To: Rainer Weikusat, Jason Baron; +Cc: netdev
In-Reply-To: <20190226062817.GA17962@ZenIV.linux.org.uk>
On Tue, Feb 26, 2019 at 06:28:17AM +0000, Al Viro wrote:
> 2) the logics in sendmsg is very odd:
> * if we'd detected n:1 send *and* found that we need to
> wait, do so (not using the peer_wake - other's peer_wait is not
> going away). No questions there.
> * if we'd detected n:1 send *and* found that we need to
> wait, but don't have any remaining timeout, we lock both ends
> and check if unix_peer(sk) is (now) not equal to other, either
> because it never had or because we'd been hit by connect(2) while
> we'd been doing the locking. In that case we fail with -EAGAIN.
> Fair enough, but
> * if after relocking we see that unix_peer(sk) now
> is equal to other, we arrange for wakeup forwarding from other's
> peer_wait *and* if that has (likely) succeeded we fail with -EAGAIN.
> Huh? What's the point? The only thing that depends upon that
> wakeup forwarding is poll, and _that_ will set the forwarding up
> on its own.
> * if we'd failed (either because other is dead now or
> because its queue is not full), we go back to restart_locked.
> If it's dead, we'll sod off with ECONNREFUSED, if it's not
> full anymore, we'll send the damn thing.
>
> All of that comes at the cost of considerable headache in
> unix_dgram_sendmsg() - flag-conditional locking, etc. Why do
> we bother? What's wrong with simple "n:1 case detected, queue
> full, no timeout left, return -EAGAIN and be done with that"?
>
> IDGI... Am I missing something subtle going on here?
>
> I understand what peer_wake is for, and the poll side of things
> is fine; it's sendmsg one that looks weird. What's going on
> there?
What I have in mind is something like this (for that part of the
issues):
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 74d1eed7cbd4..85d72ea79924 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1635,7 +1635,6 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
long timeo;
struct scm_cookie scm;
int data_len = 0;
- int sk_locked;
wait_for_unix_gc();
err = scm_send(sock, msg, &scm, false);
@@ -1713,9 +1712,8 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
goto out_free;
}
- sk_locked = 0;
unix_state_lock(other);
-restart_locked:
+
err = -EPERM;
if (!unix_may_send(sk, other))
goto out_unlock;
@@ -1728,8 +1726,7 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
unix_state_unlock(other);
sock_put(other);
- if (!sk_locked)
- unix_state_lock(sk);
+ unix_state_lock(sk);
err = 0;
if (unix_peer(sk) == other) {
@@ -1767,36 +1764,19 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
*/
if (other != sk &&
unlikely(unix_peer(other) != sk && unix_recvq_full(other))) {
- if (timeo) {
- timeo = unix_wait_for_peer(other, timeo);
-
- err = sock_intr_errno(timeo);
- if (signal_pending(current))
- goto out_free;
-
- goto restart;
- }
-
- if (!sk_locked) {
- unix_state_unlock(other);
- unix_state_double_lock(sk, other);
- }
-
- if (unix_peer(sk) != other ||
- unix_dgram_peer_wake_me(sk, other)) {
+ if (!timeo) {
err = -EAGAIN;
- sk_locked = 1;
goto out_unlock;
}
- if (!sk_locked) {
- sk_locked = 1;
- goto restart_locked;
- }
- }
+ timeo = unix_wait_for_peer(other, timeo);
- if (unlikely(sk_locked))
- unix_state_unlock(sk);
+ err = sock_intr_errno(timeo);
+ if (signal_pending(current))
+ goto out_free;
+
+ goto restart;
+ }
if (sock_flag(other, SOCK_RCVTSTAMP))
__net_timestamp(skb);
@@ -1809,8 +1789,6 @@ static int unix_dgram_sendmsg(struct socket *sock, struct msghdr *msg,
return len;
out_unlock:
- if (sk_locked)
- unix_state_unlock(sk);
unix_state_unlock(other);
out_free:
kfree_skb(skb);
^ permalink raw reply related
* [BUG] net/sched : qlen can not really be per cpu ?
From: Eric Dumazet @ 2019-02-26 6:42 UTC (permalink / raw)
To: John Fastabend, Networking, Jamal Hadi Salim, Cong Wang,
Jiri Pirko
HTB + pfifo_fast as a leaf qdisc hits badly the following warning in htb_activate() :
WARN_ON(cl->level || !cl->leaf.q || !cl->leaf.q->q.qlen);
This is because pfifo_fast does not update sch->q.qlen, but per cpu counters.
So cl->leaf.q->q.qlen is zero.
HFSC, CBQ, DRR, QFQ have the same problem.
Any ideas how we can fix this ?
Thanks !
^ permalink raw reply
* Re: [PATCH 0/2] bpf: context casting for tail call and gtrace prog type
From: Kris Van Hees @ 2019-02-26 6:46 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: Kris Van Hees, netdev, bpf, daniel
In-Reply-To: <20190226061823.f7hfs7yojfuycqmg@ast-mbp.dhcp.thefacebook.com>
On Mon, Feb 25, 2019 at 10:18:25PM -0800, Alexei Starovoitov wrote:
> On Mon, Feb 25, 2019 at 07:54:13AM -0800, Kris Van Hees wrote:
> >
> > The goal is to further extend the BPF_PROG_TYPE_GTRACE implementation to
> > support what tracers commonly need, and I am also looking at ways to further
> > extend this model to allow more tracer-specific features as well without the
> > need for adding a BPF program types for every tracer.
>
> It seems by themselves the patches don't provide any new functionality,
> but instead look like plumbing to call external code.
The patches are definitely not plumbing to call external code, and if I gave
that impression I apologise. I overlooked the information you quote below on
allowing new functionality through modules when I wrote the comment above but
please note that it was a forward-looking comment in terms of what could be
done - not a reason for the patches that I submitted.
The patches accomplish something that is totally independent from that: they
make it possible for existing events that execute BPF programs when triggered
to transfer control to a BPF program with a more rich context. The first
patch makes such a transfer possible (using tail-call combined with converting
the context to the new program type), and the second patch provides one such
program type (generic trace). The new functionality provided by the program
type is direct access to task information that previously could only be
obtained through helper calls. E.g. the new program type allows programs to
access the task state, prio, ppid, euid, and egid. None of those pieces of
information can currently be obtained unless you start poking around in
memory using bpf_probe_read() helper calls.
> This is no-go.
> There were several attempts to do so in the past, so we documented it here:
> Documentation/bpf/bpf_design_QA.rst
> Q: New functionality via kernel modules?
> ----------------------------------------
> Q: Can BPF functionality such as new program or map types, new
> helpers, etc be added out of kernel module code?
>
> A: NO.
>
> The answer is still the same.
Thanks for pointing this out - but again, my reference to modules was merely
musing about the possibilities. This information clearly closes the door on
that train of thought, but that is not directly related to what I am doing
with the patches I submitted.
Kris
^ permalink raw reply
* Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta
From: kbuild test robot @ 2019-02-26 6:48 UTC (permalink / raw)
To: Leslie Monis
Cc: kbuild-all, jhs, netdev, dave, Mohit P. Tahiliani, Dhaval Khandla,
Hrishikesh Hiraskar, Manish Kumar B, Sachin D . Patil,
Leslie Monis
In-Reply-To: <20190225102051.12268-6-lesliemonis@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 851 bytes --]
Hi Mohit,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Leslie-Monis/net-sched-pie-align-PIE-implementation-with-RFC-8033/20190226-041701
config: mips-allmodconfig (attached as .config)
compiler: mips-linux-gnu-gcc (Debian 8.2.0-11) 8.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.2.0 make.cross ARCH=mips
All errors (new ones prefixed by >>):
>> ERROR: "__udivdi3" [net/sched/sch_pie.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 58625 bytes --]
^ permalink raw reply
* Re: [PATCH net-next v3 5/6] devlink: hold a reference to the netdevice around ethtool compat
From: Jiri Pirko @ 2019-02-26 6:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190225103127.4f17f900@cakuba.netronome.com>
Mon, Feb 25, 2019 at 07:31:27PM CET, jakub.kicinski@netronome.com wrote:
>On Sun, 24 Feb 2019 12:00:03 +0100, Jiri Pirko wrote:
>> Fri, Feb 22, 2019 at 11:07:56PM CET, jakub.kicinski@netronome.com wrote:
>> >When ethtool is calling into devlink compat code make sure we have
>> >a reference on the netdevice on which the operation was invoked.
>> >
>> >v3: move the hold/lock logic into devlink_compat_* functions (Florian)
>> >
>> >Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>> >---
>> > net/core/devlink.c | 34 +++++++++++++++++++++++-----------
>> > net/core/ethtool.c | 13 ++-----------
>> > 2 files changed, 25 insertions(+), 22 deletions(-)
>> >
>> >diff --git a/net/core/devlink.c b/net/core/devlink.c
>> >index a13055160be0..78c6ea1870ca 100644
>> >--- a/net/core/devlink.c
>> >+++ b/net/core/devlink.c
>> >@@ -6430,27 +6430,39 @@ void devlink_compat_running_version(struct net_device *dev,
>> > {
>> > struct devlink *devlink;
>> >
>> >+ dev_hold(dev);
>> >+ rtnl_unlock();
>>
>> Ha, I got it now. You rely on dev_hold to make sure the
>> devlink instance does not dissappear. But until this patch, that is not
>> guaranteed (or I'm missing it).
>
>Yup, I think the expectation that drivers should free netdevs before
>unregistering devlink holds today. But it may be a source of bugs :S
Sure.
>
>I can add take devlink_mutex, and check the devlink instance is
>registered. Do you prefer an explicit ->registered field like port has,
>or can I do this:
Yeah, let's add WARN_ON.
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index cefcc0f45d44..be39ad6a4e2e 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -5275,6 +5275,7 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
> return NULL;
> devlink->ops = ops;
> devlink_net_set(devlink, &init_net);
>+ INIT_LIST_HEAD(&devlink->list);
> INIT_LIST_HEAD(&devlink->port_list);
> INIT_LIST_HEAD(&devlink->sb_list);
> INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
>@@ -5303,6 +5304,11 @@ int devlink_register(struct devlink *devlink, struct device *dev)
> }
> EXPORT_SYMBOL_GPL(devlink_register);
>
>+static bool devlink_is_registered(struct devlink *devlink)
>+{
>+ return list_empty(&devlink->list);
>+}
>+
> /**
> * devlink_unregister - Unregister devlink instance
> *
>@@ -5312,7 +5318,7 @@ void devlink_unregister(struct devlink *devlink)
> {
> mutex_lock(&devlink_mutex);
> devlink_notify(devlink, DEVLINK_CMD_DEL);
>- list_del(&devlink->list);
>+ list_del_init(&devlink->list);
> mutex_unlock(&devlink_mutex);
> }
> EXPORT_SYMBOL_GPL(devlink_unregister);
>
>?
^ permalink raw reply
* Re: [PATCH net-next v3 6/6] devlink: add missing NULL checks for devlink ops
From: Jiri Pirko @ 2019-02-26 6:50 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: davem, mkubecek, andrew, f.fainelli, netdev, oss-drivers
In-Reply-To: <20190225103249.15d26b32@cakuba.netronome.com>
Mon, Feb 25, 2019 at 07:32:49PM CET, jakub.kicinski@netronome.com wrote:
>On Sun, 24 Feb 2019 12:03:19 +0100, Jiri Pirko wrote:
>> Fri, Feb 22, 2019 at 11:07:57PM CET, jakub.kicinski@netronome.com wrote:
>>> Commit 76726ccb7f46 ("devlink: add flash update command") and
>>> commit 2d8dc5bbf4e7 ("devlink: Add support for reload")
>>> access devlink ops without NULL-checking. Add the missing checks.
>>> Note that all drivers currently implementing devlink pass non-NULL
>>> ops, so this is not a problem.
>>
>> Wouldn't it be better to rather put WARN_ON&fail when driver calls
>> devlink_alloc() with NULL ops and avoid these checks in the whole code?
>
>Sounds good! Should I remove the existing ones?
Yes please.
^ permalink raw reply
* [PATCH net] net: phy: phylink: fix uninitialized variable in phy_resolve
From: Heiner Kallweit @ 2019-02-26 7:23 UTC (permalink / raw)
To: Russell King - ARM Linux, David Miller, Andrew Lunn,
Florian Fainelli
Cc: netdev@vger.kernel.org
When debugging an issue I found implausible values in state->pause.
Reason in that state->pause isn't initialized and later only single
bits are changed. Also the struct itself isn't initialized in
phylink_resolve(). So better initialize state->pause.
Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phylink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 59d175a5b..a9954c205 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -324,6 +324,7 @@ static int phylink_get_mac_state(struct phylink *pl, struct phylink_link_state *
linkmode_zero(state->lp_advertising);
state->interface = pl->link_config.interface;
state->an_enabled = pl->link_config.an_enabled;
+ state->pause = MLO_PAUSE_NONE;
state->link = 1;
return pl->ops->mac_link_state(ndev, state);
--
2.20.1
^ permalink raw reply related
* [PATCH v2 net] net: phy: phylink: fix uninitialized variable in phylink_get_mac_state
From: Heiner Kallweit @ 2019-02-26 7:25 UTC (permalink / raw)
To: Russell King - ARM Linux, David Miller, Andrew Lunn,
Florian Fainelli
Cc: netdev@vger.kernel.org
When debugging an issue I found implausible values in state->pause.
Reason in that state->pause isn't initialized and later only single
bits are changed. Also the struct itself isn't initialized in
phylink_resolve(). So better initialize state->pause.
v2:
- use right function name in subject
Fixes: 9525ae83959b ("phylink: add phylink infrastructure")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phylink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 59d175a5b..a9954c205 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -324,6 +324,7 @@ static int phylink_get_mac_state(struct phylink *pl, struct phylink_link_state *
linkmode_zero(state->lp_advertising);
state->interface = pl->link_config.interface;
state->an_enabled = pl->link_config.an_enabled;
+ state->pause = MLO_PAUSE_NONE;
state->link = 1;
return pl->ops->mac_link_state(ndev, state);
--
2.20.1
^ permalink raw reply related
* Re: kernel BUG at include/linux/mm.h:LINE! (2)
From: Eric Biggers @ 2019-02-26 7:27 UTC (permalink / raw)
To: syzbot
Cc: davem, edumazet, kuznet, linux-kernel, netdev, syzkaller-bugs,
yoshfuji
In-Reply-To: <000000000000b5f145056e2121a4@google.com>
On Fri, Jun 08, 2018 at 06:11:02AM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 7170e6045a6a strparser: Add __strp_unpause and use it in k..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=114236af800000
> kernel config: https://syzkaller.appspot.com/x/.config?x=a601a80fec461d44
> dashboard link: https://syzkaller.appspot.com/bug?extid=3225ce21c0e9929bb9cf
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=10f44fdf800000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=110f636f800000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3225ce21c0e9929bb9cf@syzkaller.appspotmail.com
>
> flags: 0x2fffc0000000000()
> raw: 02fffc0000000000 0000000000000000 0000000000000000 00000000ffffff80
> raw: ffffea0006b29220 ffff88021fffac18 0000000000000003 0000000000000000
> page dumped because: VM_BUG_ON_PAGE(page_ref_count(page) <= 0)
> ------------[ cut here ]------------
> kernel BUG at include/linux/mm.h:853!
> invalid opcode: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 1 PID: 4545 Comm: syz-executor492 Not tainted 4.17.0-rc7+ #82
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:get_page include/linux/mm.h:853 [inline]
> RIP: 0010:do_tcp_sendpages+0x1879/0x1e60 net/ipv4/tcp.c:1002
> RSP: 0018:ffff8801c2a06f88 EFLAGS: 00010203
> RAX: 0000000000000000 RBX: ffff8801d972d580 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff81a66c25 RDI: ffffed0038540de0
> RBP: ffff8801c2a071e8 R08: ffff8801b11d2480 R09: 0000000000000006
> R10: ffff8801b11d2480 R11: 0000000000000000 R12: 000000000000301d
> R13: ffffea0006b2621c R14: ffff8801ae5a6040 R15: dffffc0000000000
> FS: 0000000000000000(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020008000 CR3: 0000000008c6a000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> tls_push_sg+0x25b/0x860 net/tls/tls_main.c:126
> tls_push_record+0xae5/0x13e0 net/tls/tls_sw.c:266
> tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
> tls_handle_open_record net/tls/tls_main.c:164 [inline]
> tls_sk_proto_close+0x734/0xad0 net/tls/tls_main.c:264
> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
> sock_release+0x96/0x1b0 net/socket.c:594
> sock_close+0x16/0x20 net/socket.c:1149
> __fput+0x34d/0x890 fs/file_table.c:209
> ____fput+0x15/0x20 fs/file_table.c:243
> task_work_run+0x1e4/0x290 kernel/task_work.c:113
> exit_task_work include/linux/task_work.h:22 [inline]
> do_exit+0x1aee/0x2730 kernel/exit.c:865
> do_group_exit+0x16f/0x430 kernel/exit.c:968
> __do_sys_exit_group kernel/exit.c:979 [inline]
> __se_sys_exit_group kernel/exit.c:977 [inline]
> __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
> do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x43f368
> RSP: 002b:00007ffd03500578 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043f368
> RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
> RBP: 00000000004bf448 R08: 00000000000000e7 R09: ffffffffffffffd0
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> R13: 00000000006d1180 R14: 0000000000000000 R15: 0000000000000000
> Code: ff ff 41 89 86 cc 08 00 00 e8 e4 07 05 00 e9 2c eb ff ff e8 ca 4b 27
> fb 48 8b bd b8 fd ff ff 48 c7 c6 40 0c 54 88 e8 77 72 54 fb <0f> 0b 48 89 85
> b8 fd ff ff e8 a9 4b 27 fb 48 8b 85 b8 fd ff ff
> RIP: get_page include/linux/mm.h:853 [inline] RSP: ffff8801c2a06f88
> RIP: do_tcp_sendpages+0x1879/0x1e60 net/ipv4/tcp.c:1002 RSP:
> ffff8801c2a06f88
> ---[ end trace 500a6e4fab99629c ]---
>
>
> ---
> 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
>
AFAICS this was fixed by this commit:
commit d829e9c4112b52f4f00195900fd4c685f61365ab
Author: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat Oct 13 02:45:59 2018 +0200
tls: convert to generic sk_msg interface
So telling syzbot:
#syz fix: tls: convert to generic sk_msg interface
The issue was that described in this comment in tls_sw_sendmsg():
/* Open records defined only if successfully copied, otherwise
* we would trim the sg but not reset the open record frags.
*/
tls_ctx->pending_open_record_frags = true;
Basically, on sendmsg() to a TLS socket, if the message buffer was partially
unmapped, a TLS record would be marked as pending (and then tried to be sent at
sock_release() time) even though it had actually been discarded.
- Eric
^ permalink raw reply
* Re: KASAN: use-after-free Write in tls_push_record (2)
From: Eric Biggers @ 2019-02-26 7:31 UTC (permalink / raw)
To: Boris Pismenny
Cc: syzbot, aviadye, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <bb81a2ef-7908-844d-3161-213973342ddb@mellanox.com>
On Thu, Jul 12, 2018 at 06:44:55AM -0400, Boris Pismenny wrote:
> It seems to me that the crash here is due to write_space being called after
> the close system call. Maybe the correct solution is to move the TX software
> state to be released in sk_destruct. As we already do for the device state
> (see tls_device.c).
>
> Is anyone looking into this one?
>
> On 7/11/2018 8:49 PM, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 1e09177acae3 Merge tag 'mips_fixes_4.18_3' of
> > git://git.ke..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=128903b2400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=25856fac4e580aa7
> > dashboard link:
> > https://syzkaller.appspot.com/bug?extid=6c4e6ecbf9a2797be67c
> > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=12312678400000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13ef76c2400000
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+6c4e6ecbf9a2797be67c@syzkaller.appspotmail.com
> >
> > RDX: 00000000fffffdef RSI: 00000000200005c0 RDI: 0000000000000003
> > RBP: 00000000006cb018 R08: 0000000020000000 R09: 000000000000001c
> > R10: 0000000000000040 R11: 0000000000000212 R12: 0000000000000005
> > R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> > ==================================================================
> > BUG: KASAN: use-after-free in tls_fill_prepend include/net/tls.h:339
> > [inline]
> > BUG: KASAN: use-after-free in tls_push_record+0x1091/0x1400
> > net/tls/tls_sw.c:239
> > Write of size 1 at addr ffff8801ae430000 by task syz-executor589/4567
> >
> > CPU: 0 PID: 4567 Comm: syz-executor589 Not tainted 4.18.0-rc4+ #141
> > 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+0x1c9/0x2b4 lib/dump_stack.c:113
> > print_address_description+0x6c/0x20b mm/kasan/report.c:256
> > kasan_report_error mm/kasan/report.c:354 [inline]
> > kasan_report.cold.7+0x242/0x2fe mm/kasan/report.c:412
> > __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
> > tls_fill_prepend include/net/tls.h:339 [inline]
> > tls_push_record+0x1091/0x1400 net/tls/tls_sw.c:239
> > tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
> > tls_handle_open_record net/tls/tls_main.c:164 [inline]
> > tls_sk_proto_close+0x74c/0xae0 net/tls/tls_main.c:264
> > inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
> > inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
> > __sock_release+0xd7/0x260 net/socket.c:599
> > sock_close+0x19/0x20 net/socket.c:1150
> > __fput+0x355/0x8b0 fs/file_table.c:209
> > ____fput+0x15/0x20 fs/file_table.c:243
> > task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
> > exit_task_work include/linux/task_work.h:22 [inline]
> > do_exit+0x1b08/0x2750 kernel/exit.c:865
> > do_group_exit+0x177/0x440 kernel/exit.c:968
> > __do_sys_exit_group kernel/exit.c:979 [inline]
> > __se_sys_exit_group kernel/exit.c:977 [inline]
> > __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:977
> > do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x43f358
> > Code: Bad RIP value.
> > RSP: 002b:00007fff51750198 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> > RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043f358
> > RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
> > RBP: 00000000004bf448 R08: 00000000000000e7 R09: ffffffffffffffd0
> > R10: 0000000000000040 R11: 0000000000000246 R12: 0000000000000001
> > R13: 00000000006d1180 R14: 0000000000000000 R15: 0000000000000000
> >
> > The buggy address belongs to the page:
> > page:ffffea0006b90c00 count:0 mapcount:-128 mapping:0000000000000000
> > index:0x0
> > flags: 0x2fffc0000000000()
> > raw: 02fffc0000000000 ffffea0006b96408 ffff88021fffac18 0000000000000000
> > raw: 0000000000000000 0000000000000003 00000000ffffff7f 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffff8801ae42ff00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801ae42ff80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > > ffff8801ae430000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ^
> > ffff8801ae430080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ffff8801ae430100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ==================================================================
> >
> >
> > ---
> > 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
>
AFAICS this was fixed by this commit:
commit d829e9c4112b52f4f00195900fd4c685f61365ab
Author: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat Oct 13 02:45:59 2018 +0200
tls: convert to generic sk_msg interface
So telling syzbot:
#syz fix: tls: convert to generic sk_msg interface
The issue was that described in this comment in tls_sw_sendmsg():
/* Open records defined only if successfully copied, otherwise
* we would trim the sg but not reset the open record frags.
*/
tls_ctx->pending_open_record_frags = true;
Basically, on sendmsg() to a TLS socket, if the message buffer was partially
unmapped, a TLS record would be marked as pending (and then tried to be sent at
sock_release() time) even though it had actually been discarded.
- Eric
^ permalink raw reply
* Re: [RFC] net: dsa: qca8k: CPU port broken with commit 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
From: Heiner Kallweit @ 2019-02-26 7:33 UTC (permalink / raw)
To: Michal Vokáč, Andrew Lunn, Vinod Koul
Cc: David S. Miller, Florian Fainelli, netdev
In-Reply-To: <d127618b-873e-e4aa-ea75-f747da5574ee@ysoft.com>
On 20.02.2019 16:02, Michal Vokáč wrote:
> Hi,
>
> Another issue in a row with networking on imx6dl-yapp4 platform [1]
> that uses QCA8334 Ethernet switch.
>
> Very recently, with Vinod and Andrew, we solved an issue with
> RGMII_ID mode by patch[2][3]. I tested those with next-20190215
> and it worked just fine.
>
> The patch[2] was merged into next-20190220 so I tested the latest version.
> Now the cpu port does not work again. I tracked it down to this commit
> 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in
> genphy_read_status") [4]
>
> If I revert the offending commit, cpu port works fine. I suspect the
> problem is on the qca8k driver side but I am not really sure.
> AFAICT autonegotiation is not available on the QCA833x cpu port (MAC0).
>
> Any ideas what may be the root cause of the problem?
>
Hi Michal,
I faced the same issue on a system with Marvell switch.
Should be fixed with 0f3b1cf23f0e ("net: phy: fix reading fixed phy status").
This patch is in net-next currently and should show up in linux-next today.
> Thank you,
> Michal
>
Heiner
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=87489ec3a77f3e01bcf0d46e353ae7112ec8c4f0
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a968b5e9d5879f9535d6099505f9e14abcafb623
> [3] https://lore.kernel.org/patchwork/patch/1043817/
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=5502b218e001
>
^ permalink raw reply
* Re: general protection fault in tls_push_sg
From: Eric Biggers @ 2019-02-26 7:40 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <0000000000006a3ec5056f0c25cb@google.com>
On Tue, Jun 19, 2018 at 10:34:01PM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: ba4dbdedd3ed Merge tag 'jfs-4.18' of git://github.com/klei..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=112e9ce4400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=f390986c4f7cd566
> dashboard link: https://syzkaller.appspot.com/bug?extid=54bcc120da8da091d609
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+54bcc120da8da091d609@syzkaller.appspotmail.com
>
> netlink: 8 bytes leftover after parsing attributes in process
> `syz-executor0'.
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN
> CPU: 1 PID: 27979 Comm: syz-executor6 Not tainted 4.18.0-rc1+ #109
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
> RIP: 0010:compound_head include/linux/page-flags.h:142 [inline]
> RIP: 0010:put_page include/linux/mm.h:911 [inline]
> RIP: 0010:tls_push_sg+0x2a3/0x880 net/tls/tls_main.c:142
> Code: fa 4d 39 e5 75 a2 e8 bc 50 f1 fa 48 8b 85 08 ff ff ff 49 8d 7f 08 48
> b9 00 00 00 00 00 fc ff df c6 00 00 48 89 f8 48 c1 e8 03 <80> 3c 08 00 0f 85
> 50 05 00 00 48 8b 85 08 ff ff ff 49 8b 5f 08 80
> RSP: 0018:ffff8801c5776d90 EFLAGS: 00010202
> RAX: 0000000000000001 RBX: 0000000000000000 RCX: dffffc0000000000
> RDX: 0000000000000000 RSI: ffffffff868a59e4 RDI: 0000000000000008
> RBP: ffff8801c5776eb0 R08: ffff88018e4fc6c0 R09: ffff8801c5776668
> R10: 0000000000000003 R11: 0000000000000002 R12: 0000000000000000
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> FS: 00007f2d08c17700(0000) GS:ffff8801daf00000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 000000001ffffcc0 CR3: 0000000188ce8000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> tls_push_record+0xaec/0x1400 net/tls/tls_sw.c:264
> tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
> tls_handle_open_record net/tls/tls_main.c:164 [inline]
> tls_sk_proto_close+0x74c/0xae0 net/tls/tls_main.c:264
> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
> __sock_release+0xd7/0x260 net/socket.c:603
> sock_close+0x19/0x20 net/socket.c:1186
> __fput+0x35b/0x8b0 fs/file_table.c:209
> ____fput+0x15/0x20 fs/file_table.c:243
> task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
> exit_task_work include/linux/task_work.h:22 [inline]
> do_exit+0x1b08/0x2750 kernel/exit.c:865
> do_group_exit+0x177/0x440 kernel/exit.c:968
> get_signal+0x88e/0x1970 kernel/signal.c:2468
> do_signal+0x9c/0x21c0 arch/x86/kernel/signal.c:816
> exit_to_usermode_loop+0x2de/0x370 arch/x86/entry/common.c:162
> prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
> do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x455b29
> Code: 1d ba fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 eb b9 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f2d08c16ce8 EFLAGS: 00000246 ORIG_RAX: 00000000000000ca
> RAX: fffffffffffffe00 RBX: 000000000072bec8 RCX: 0000000000455b29
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 000000000072bec8
> RBP: 000000000072bec8 R08: 0000000000000033 R09: 000000000072bea0
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> R13: 0000000000a3e81f R14: 00007f2d08c179c0 R15: 0000000000000000
> Modules linked in:
> Dumping ftrace buffer:
> (ftrace buffer empty)
> ---[ end trace d9dfd7279b1a9c99 ]---
> RIP: 0010:__read_once_size include/linux/compiler.h:188 [inline]
> RIP: 0010:compound_head include/linux/page-flags.h:142 [inline]
> RIP: 0010:put_page include/linux/mm.h:911 [inline]
> RIP: 0010:tls_push_sg+0x2a3/0x880 net/tls/tls_main.c:142
>
>
> ---
> 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.
>
(As with the other reports of this...)
AFAICS this was fixed by this commit:
commit d829e9c4112b52f4f00195900fd4c685f61365ab
Author: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat Oct 13 02:45:59 2018 +0200
tls: convert to generic sk_msg interface
So telling syzbot:
#syz fix: tls: convert to generic sk_msg interface
The issue was that described in this comment in tls_sw_sendmsg():
/* Open records defined only if successfully copied, otherwise
* we would trim the sg but not reset the open record frags.
*/
tls_ctx->pending_open_record_frags = true;
Basically, on sendmsg() to a TLS socket, if the message buffer was partially
unmapped, a TLS record would be marked as pending (and then tried to be sent at
sock_release() time) even though it had actually been discarded.
- Eric
^ permalink raw reply
* Re: [PATCH] net: dsa: read mac address from DT for slave device
From: xiaofeis @ 2019-02-26 7:45 UTC (permalink / raw)
To: Florian Fainelli
Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
Andrew Lunn, Vivien Didelot, Niklas Cassel, netdev
In-Reply-To: <d741691d-cc04-442f-8027-9466a331a529@gmail.com>
On 2019-02-26 01:27, Florian Fainelli wrote:
> On 2/25/19 5:28 AM, xiaofeis@codeaurora.org wrote:
>> Hi Florian
>>
>> We have two slave DSA interfaces, wan0 and lan0, one is for wan port,
>> and the other is for lan port. Customer has it's mac address pool,
>> they
>> want
>> to assign the mac address from the pool on wan0, lan0, and other
>> interfaces like
>> wifi, bt. Coreboot/uboot will populate it to the DTS node, so the
>> driver
>> can
>> get it from it's node. For DSA slave interface, it already has it's
>> own
>> DTS node, it's
>> easy to just add one porperty "local-mac-address" there for the usage
>> in
>> DSA driver.
>>
>> If not use DSA framework, normally we will use eth0.x and eth0.y for
>> wan
>> and lan.
>> On this case, customer usually also assign the MAC address on these
>> logical interface
>> from it's pool.
>
> OK, but this is not necessary per my previous explanation: the CPU <=>
> WAN pipe is a separate broadcast domain (otherwise it is a security
> hole
> since you exposing LAN machines to the outside world), and so there is
> no need for a separate MAC address. It might be convenient to have one,
> especially for the provider, if they run a management software (e.g.:
> TR69), but it is not required per-se.
>
> Let me ask a secondary question here, how many Ethernet MACs connect to
> the switch in this configuration? Is there one that is supposed to be
> assigned all LAN traffic and one that is supposed to be assigned all
> WAN
> traffic? If so, then what you are doing makes even less
>
Only one MAC connected to switch cpu port, both lan0 and wan0 are on the
top of
same interface(eth0).
>>
>> On 2019-02-22 23:43, Florian Fainelli wrote:
>>> On 2/22/19 4:58 AM, Vinod Koul wrote:
>>>> From: Xiaofei Shen <xiaofeis@codeaurora.org>
>>>>
>>>> Before creating a slave netdevice, get the mac address from DTS and
>>>> apply in case it is valid.
>>>
>>> Can you explain your use case in details?
>>>
>>> Assigning a MAC address to a network device that represents a switch
>>> port does not quite make sense in general. The switch port is really
>>> representing one end of a pipe, so one side you have stations and on
>>> the
>>> other side, you have the CPU/management Ethernet MAC controller's MAC
>>> address which constitutes a station as well. The DSA slave network
>>> devices are just software constructs meant to steer traffic towards
>>> specific ports of the switch, but they are all from the perpsective
>>> of
>>> traffic reaching the CPU Port in the first place, therefore traffic
>>> that
>>> is generally a known unicast Ethernet frame with the CPU's MAC
>>> address
>>> as MAC DA (and of course all types of unknown MC, management traffic
>>> etc.)
>>>
>>> By default, DSA switch need to come up in a configuration where all
>>> ports (except CPU/management) must be strictly separate from every
>>> other
>>> port such that we can achieve what a standalone Ethernet NIC would
>>> do.
>>> This works because all ports are isolated from one another, so there
>>> is
>>> no cross talk and so having the same MAC address (the one from the
>>> CPU)
>>> on the DSA slave network devices just works, each port is a separate
>>> broadcast domain.
>>>
>>> Once you start bridging one or ore ports, the bridge root port will
>>> have
>>> a MAC address, most likely the one the CPU/management Ethernet MAC,
>>> but
>>> similarly, this is not an issue and that's exactly how a software
>>> bridge
>>> would work as well.
>>>
>>>>
>>>> Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
>>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>>> ---
>>>> include/net/dsa.h | 1 +
>>>> net/dsa/dsa2.c | 1 +
>>>> net/dsa/slave.c | 5 ++++-
>>>> 3 files changed, 6 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>>>> index b3eefe8e18fd..aa24ce756679 100644
>>>> --- a/include/net/dsa.h
>>>> +++ b/include/net/dsa.h
>>>> @@ -198,6 +198,7 @@ struct dsa_port {
>>>> unsigned int index;
>>>> const char *name;
>>>> const struct dsa_port *cpu_dp;
>>>> + const char *mac;
>>>> struct device_node *dn;
>>>> unsigned int ageing_time;
>>>> u8 stp_state;
>>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>>> index a1917025e155..afb7d9fa42f6 100644
>>>> --- a/net/dsa/dsa2.c
>>>> +++ b/net/dsa/dsa2.c
>>>> @@ -261,6 +261,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>>>> int err = 0;
>>>>
>>>> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>>>> + dp->mac = of_get_mac_address(dp->dn);
>>>>
>>>> if (dp->type != DSA_PORT_TYPE_UNUSED)
>>>> err = devlink_port_register(ds->devlink, &dp->devlink_port,
>>>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>>>> index a3fcc1d01615..8e64c4e947c6 100644
>>>> --- a/net/dsa/slave.c
>>>> +++ b/net/dsa/slave.c
>>>> @@ -1308,7 +1308,10 @@ int dsa_slave_create(struct dsa_port *port)
>>>> slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
>>>> slave_dev->hw_features |= NETIF_F_HW_TC;
>>>> slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>>>> - eth_hw_addr_inherit(slave_dev, master);
>>>> + if (port->mac && is_valid_ether_addr(port->mac))
>>>> + ether_addr_copy(slave_dev->dev_addr, port->mac);
>>>> + else
>>>> + eth_hw_addr_inherit(slave_dev, master);
>>>> slave_dev->priv_flags |= IFF_NO_QUEUE;
>>>> slave_dev->netdev_ops = &dsa_slave_netdev_ops;
>>>> slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
>>>>
^ permalink raw reply
* Re: [PATCH net-next] can: kvaser_usb: Use struct_size() in alloc_candev()
From: Marc Kleine-Budde @ 2019-02-26 7:52 UTC (permalink / raw)
To: Gustavo A. R. Silva, Wolfgang Grandegger, David S. Miller
Cc: linux-can, netdev, linux-kernel
In-Reply-To: <20190208031035.GA2665@embeddedor>
On 2/8/19 4:10 AM, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
>
> struct foo {
> int stuff;
> void *entry[];
> };
>
> instance = alloc(sizeof(struct foo) + count * sizeof(void *));
>
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
>
> instance = alloc(struct_size(instance, entry, count));
>
> This code was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Applied to linux-can-next.
Tnx,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
^ permalink raw reply
* Re: KASAN: use-after-free Read in tls_tx_records
From: Eric Biggers @ 2019-02-26 7:55 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <000000000000dfb5720576ee2873@google.com>
On Fri, Sep 28, 2018 at 06:09:03AM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 1042caa79e93 net-ipv4: remove 2 always zero parameters fro..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=13fff711400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=6da69433212d7e87
> dashboard link: https://syzkaller.appspot.com/bug?extid=c45f79b4e5e940da28a9
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+c45f79b4e5e940da28a9@syzkaller.appspotmail.com
>
> EXT4-fs (sda1): resizing filesystem from 524032 to 6 blocks
> EXT4-fs warning (device sda1): ext4_resize_fs:1930: can't shrink FS - resize
> aborted
> EXT4-fs (sda1): resizing filesystem from 524032 to 6 blocks
> EXT4-fs warning (device sda1): ext4_resize_fs:1930: can't shrink FS - resize
> aborted
> ==================================================================
> BUG: KASAN: use-after-free in tls_tx_records+0x8b0/0x980
> net/tls/tls_sw.c:365
> Read of size 8 at addr ffff8801ce46e040 by task syz-executor3/28575
>
> CPU: 0 PID: 28575 Comm: syz-executor3 Not tainted 4.19.0-rc5+ #235
> 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+0x1c4/0x2b4 lib/dump_stack.c:113
> print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
> __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
> tls_tx_records+0x8b0/0x980 net/tls/tls_sw.c:365
> tls_sw_free_resources_tx+0x1ec/0xd20 net/tls/tls_sw.c:1552
> tls_sk_proto_close+0x605/0x750 net/tls/tls_main.c:278
> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:428
> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:458
> __sock_release+0xd7/0x250 net/socket.c:579
> sock_close+0x19/0x20 net/socket.c:1141
> __fput+0x385/0xa30 fs/file_table.c:278
> ____fput+0x15/0x20 fs/file_table.c:309
> task_work_run+0x1e8/0x2a0 kernel/task_work.c:113
> tracehook_notify_resume include/linux/tracehook.h:193 [inline]
> exit_to_usermode_loop+0x318/0x380 arch/x86/entry/common.c:166
> prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
> syscall_return_slowpath arch/x86/entry/common.c:268 [inline]
> do_syscall_64+0x6be/0x820 arch/x86/entry/common.c:293
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x457579
> Code: 1d b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 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 eb b3 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> RSP: 002b:00007f2ccaa3bc78 EFLAGS: 00000246 ORIG_RAX: 0000000000000003
> RAX: 0000000000000000 RBX: 0000000000000001 RCX: 0000000000457579
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> RBP: 000000000072bf00 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007f2ccaa3c6d4
> R13: 00000000004ef912 R14: 00000000004cc460 R15: 00000000ffffffff
>
> Allocated by task 28575:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
> __do_kmalloc mm/slab.c:3718 [inline]
> __kmalloc+0x14e/0x760 mm/slab.c:3727
> kmalloc include/linux/slab.h:518 [inline]
> kzalloc include/linux/slab.h:707 [inline]
> get_rec+0x147/0x630 net/tls/tls_sw.c:653
> tls_sw_sendmsg+0x47e/0x17a0 net/tls/tls_sw.c:727
> inet_sendmsg+0x1a1/0x690 net/ipv4/af_inet.c:798
> sock_sendmsg_nosec net/socket.c:621 [inline]
> sock_sendmsg+0xd5/0x120 net/socket.c:631
> __sys_sendto+0x3d7/0x670 net/socket.c:1788
> __do_sys_sendto net/socket.c:1800 [inline]
> __se_sys_sendto net/socket.c:1796 [inline]
> __x64_sys_sendto+0xe1/0x1a0 net/socket.c:1796
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> Freed by task 23411:
> save_stack+0x43/0xd0 mm/kasan/kasan.c:448
> set_track mm/kasan/kasan.c:460 [inline]
> __kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
> kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
> __cache_free mm/slab.c:3498 [inline]
> kfree+0xcf/0x230 mm/slab.c:3813
> tls_encrypt_done+0x221/0x610 net/tls/tls_sw.c:417
> aead_request_complete include/crypto/internal/aead.h:75 [inline]
> pcrypt_aead_serial+0x7b/0xb0 crypto/pcrypt.c:123
> padata_serial_worker+0x4c6/0x760 kernel/padata.c:349
> process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153
> worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
> kthread+0x35a/0x420 kernel/kthread.c:246
> ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
>
> The buggy address belongs to the object at ffff8801ce46e040
> which belongs to the cache kmalloc-2048 of size 2048
> The buggy address is located 0 bytes inside of
> 2048-byte region [ffff8801ce46e040, ffff8801ce46e840)
> The buggy address belongs to the page:
> page:ffffea0007391b80 count:1 mapcount:0 mapping:ffff8801da800c40 index:0x0
> compound_mapcount: 0
> flags: 0x2fffc0000008100(slab|head)
> raw: 02fffc0000008100 ffffea00071b4f88 ffffea0007344f88 ffff8801da800c40
> raw: 0000000000000000 ffff8801ce46e040 0000000100000003 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8801ce46df00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8801ce46df80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801ce46e000: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
> ^
> ffff8801ce46e080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8801ce46e100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ==================================================================
>
>
> ---
> 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.
>
(As with the other reports of this...)
AFAICS this was fixed by this commit:
commit d829e9c4112b52f4f00195900fd4c685f61365ab
Author: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat Oct 13 02:45:59 2018 +0200
tls: convert to generic sk_msg interface
So telling syzbot:
#syz fix: tls: convert to generic sk_msg interface
The issue was that described in this comment in tls_sw_sendmsg():
/* Open records defined only if successfully copied, otherwise
* we would trim the sg but not reset the open record frags.
*/
tls_ctx->pending_open_record_frags = true;
Basically, on sendmsg() to a TLS socket, if the message buffer was partially
unmapped, a TLS record would be marked as pending (and then tried to be sent at
sock_release() time) even though it had actually been discarded.
- Eric
^ permalink raw reply
* Re: KASAN: out-of-bounds Write in tls_push_record
From: Eric Biggers @ 2019-02-26 7:59 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <0000000000007567f0057072d2e7@google.com>
On Sat, Jul 07, 2018 at 06:29:03PM -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 526674536360 Add linux-next specific files for 20180706
> git tree: linux-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17e63968400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=c8d1cfc0cb798e48
> dashboard link: https://syzkaller.appspot.com/bug?extid=43358359519ad16cf05e
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15790594400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=12b53f48400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+43358359519ad16cf05e@syzkaller.appspotmail.com
>
> RDX: 00000000fffffdef RSI: 00000000200005c0 RDI: 0000000000000004
> RBP: 00000000006cb018 R08: 0000000020000000 R09: 000000000000001c
> R10: 0000000000000040 R11: 0000000000000216 R12: 0000000000000005
> R13: ffffffffffffffff R14: 0000000000000000 R15: 0000000000000000
> ==================================================================
> BUG: KASAN: out-of-bounds in tls_fill_prepend include/net/tls.h:339 [inline]
> BUG: KASAN: out-of-bounds in tls_push_record+0x1091/0x1400
> net/tls/tls_sw.c:239
> Write of size 1 at addr ffff8801c07b8000 by task syz-executor985/4467
>
> CPU: 0 PID: 4467 Comm: syz-executor985 Not tainted 4.18.0-rc3-next-20180706+
> #1
> 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+0x1c9/0x2b4 lib/dump_stack.c:113
> print_address_description+0x6c/0x20b mm/kasan/report.c:256
> kasan_report_error mm/kasan/report.c:354 [inline]
> kasan_report.cold.7+0x242/0x30d mm/kasan/report.c:412
> __asan_report_store1_noabort+0x17/0x20 mm/kasan/report.c:435
> tls_fill_prepend include/net/tls.h:339 [inline]
> tls_push_record+0x1091/0x1400 net/tls/tls_sw.c:239
> tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:276
> tls_handle_open_record net/tls/tls_main.c:164 [inline]
> tls_sk_proto_close+0x74c/0xae0 net/tls/tls_main.c:264
> inet_release+0x104/0x1f0 net/ipv4/af_inet.c:427
> inet6_release+0x50/0x70 net/ipv6/af_inet6.c:459
> __sock_release+0xd7/0x260 net/socket.c:600
> sock_close+0x19/0x20 net/socket.c:1151
> __fput+0x35d/0x930 fs/file_table.c:215
> ____fput+0x15/0x20 fs/file_table.c:251
> task_work_run+0x1ec/0x2a0 kernel/task_work.c:113
> exit_task_work include/linux/task_work.h:22 [inline]
> do_exit+0x1b08/0x2750 kernel/exit.c:869
> do_group_exit+0x177/0x440 kernel/exit.c:972
> __do_sys_exit_group kernel/exit.c:983 [inline]
> __se_sys_exit_group kernel/exit.c:981 [inline]
> __x64_sys_exit_group+0x3e/0x50 kernel/exit.c:981
> do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> entry_SYSCALL_64_after_hwframe+0x49/0xbe
> RIP: 0033:0x43f358
> Code: Bad RIP value.
> RSP: 002b:00007ffd4c2414b8 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 000000000043f358
> RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
> RBP: 00000000004bf448 R08: 00000000000000e7 R09: ffffffffffffffd0
> R10: 0000000000000040 R11: 0000000000000246 R12: 0000000000000001
> R13: 00000000006d1180 R14: 0000000000000000 R15: 0000000000000000
>
> The buggy address belongs to the page:
> page:ffffea000701ee00 count:0 mapcount:-128 mapping:0000000000000000
> index:0x0
> flags: 0x2fffc0000000000()
> raw: 02fffc0000000000 ffffea0006b7be08 ffff88021fffac18 0000000000000000
> raw: 0000000000000000 0000000000000003 00000000ffffff7f 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8801c07b7f00: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff8801c07b7f80: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> > ffff8801c07b8000: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ^
> ffff8801c07b8080: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ffff8801c07b8100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
> ==================================================================
>
>
> ---
> 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
>
(As with the other reports of this...)
AFAICS this was fixed by this commit:
commit d829e9c4112b52f4f00195900fd4c685f61365ab
Author: Daniel Borkmann <daniel@iogearbox.net>
Date: Sat Oct 13 02:45:59 2018 +0200
tls: convert to generic sk_msg interface
So telling syzbot:
#syz fix: tls: convert to generic sk_msg interface
The issue was that described in this comment in tls_sw_sendmsg():
/* Open records defined only if successfully copied, otherwise
* we would trim the sg but not reset the open record frags.
*/
tls_ctx->pending_open_record_frags = true;
Basically, on sendmsg() to a TLS socket, if the message buffer was partially
unmapped, a TLS record would be marked as pending (and then tried to be sent at
sock_release() time) even though it had actually been discarded.
- Eric
^ 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