* Re: [PATCH net-next v3 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Vivien Didelot @ 2019-02-20 17:26 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <E1gwR7O-0003ig-0P@rmk-PC.armlinux.org.uk>
Hi Russell,
On Wed, 20 Feb 2019 12:36:54 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
> the multicast and unicast flood properties to be controlled. These
> can be controlled on a per-port basis via commands such as:
>
> bridge link set dev lan1 flood on|off
> bridge link set dev lan1 mcast_flood on|off
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 32e7af5caa69..937370639fe4 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4692,6 +4692,24 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
> return err;
> }
>
> +static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
> + bool unicast, bool multicast)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int ret = -EOPNOTSUPP;
I'd prefer "err" like in the rest of the driver.
> +
> + WARN_ON_ONCE(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port));
No need to warn, the driver does not need to care about which port is targeted
by a call.
> +
> + mutex_lock(&chip->reg_lock);
> + if (chip->info->ops->port_set_egress_floods)
> + ret = chip->info->ops->port_set_egress_floods(chip, port,
> + unicast,
> + multicast);
> + mutex_unlock(&chip->reg_lock);
> +
> + return ret;
> +}
> +
> static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
> #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
> .probe = mv88e6xxx_drv_probe,
> @@ -4719,6 +4737,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
> .set_ageing_time = mv88e6xxx_set_ageing_time,
> .port_bridge_join = mv88e6xxx_port_bridge_join,
> .port_bridge_leave = mv88e6xxx_port_bridge_leave,
> + .port_egress_floods = mv88e6xxx_port_egress_floods,
> .port_stp_state_set = mv88e6xxx_port_stp_state_set,
> .port_fast_age = mv88e6xxx_port_fast_age,
> .port_vlan_filtering = mv88e6xxx_port_vlan_filtering,
Otherwise it looks good to me.
^ permalink raw reply
* Re: [PATCH net-next v3 3/3] net: dsa: enable flooding for bridge ports
From: Vivien Didelot @ 2019-02-20 17:23 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <E1gwR7T-0003iw-Cq@rmk-PC.armlinux.org.uk>
Hi Russell,
On Wed, 20 Feb 2019 12:36:59 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Switches work by learning the MAC address for each attached station by
> monitoring traffic from each station. When a station sends a packet,
> the switch records which port the MAC address is connected to.
>
> With IPv4 networking, before communication commences with a neighbour,
> an ARP packet is broadcasted to all stations asking for the MAC address
> corresponding with the IPv4. The desired station responds with an ARP
> reply, and the ARP reply causes the switch to learn which port the
> station is connected to.
>
> With IPv6 networking, the situation is rather different. Rather than
> broadcasting ARP packets, a "neighbour solicitation" is multicasted
> rather than broadcasted. This multicast needs to reach the intended
> station in order for the neighbour to be discovered.
>
> Once a neighbour has been discovered, and entered into the sending
> stations neighbour cache, communication can restart at a point later
> without sending a new neighbour solicitation, even if the entry in
> the neighbour cache is marked as stale. This can be after the MAC
> address has expired from the forwarding cache of the DSA switch -
> when that occurs, there is a long pause in communication.
>
> Our DSA implementation for mv88e6xxx switches disables flooding of
> multicast and unicast frames for bridged ports. As per the above
> description, this is fine for IPv4 networking, since the broadcasted
> ARP queries will be sent to and received by all stations on the same
> network. However, this breaks IPv6 very badly - blocking neighbour
> solicitations and later causing connections to stall.
>
> The defaults that the Linux bridge code expect from bridges are for
> unknown unicast and unknown multicast frames to be flooded to all ports
> on the bridge, which is at odds to the defaults adopted by our DSA
> implementation for mv88e6xxx switches.
>
> This commit enables by default flooding of both unknown unicast and
> unknown multicast frames whenever a port is added to a bridge, and
> disables the flooding when a port leaves the bridge. This means that
> mv88e6xxx DSA switches now behave as per the bridge(8) man page, and
> IPv6 works flawlessly through such a switch.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> net/dsa/port.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index b84d010fb165..9e7aab13957e 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -105,6 +105,11 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
> };
> int err;
>
> + /* Set the flooding mode before joining */
Note that as stated by the comment just below, the port has already joined
the bridge here.
> + err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD, NULL);
> + if (err)
> + return err;
> +
> /* Here the port is already bridged. Reflect the current configuration
> * so that drivers can program their chips accordingly.
> */
> @@ -113,8 +118,10 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
> err = dsa_port_notify(dp, DSA_NOTIFIER_BRIDGE_JOIN, &info);
>
> /* The bridging is rolled back on error */
> - if (err)
> + if (err) {
> + dsa_port_bridge_flags(dp, 0, NULL);
> dp->bridge_dev = NULL;
> + }
>
> return err;
> }
> @@ -137,6 +144,9 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
> if (err)
> pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");
>
> + /* Port is leaving the bridge, disable flooding */
> + dsa_port_bridge_flags(dp, BR_LEARNING, NULL);
> +
> /* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
> * so allow it to be in BR_STATE_FORWARDING to be kept functional
> */
This makes it clear that we must add this logic which sets the expected
default flags into the bridge code itself. But this can be done later.
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH v2 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Florian Fainelli @ 2019-02-20 17:22 UTC (permalink / raw)
To: Andrew Lunn, Tristram.Ha
Cc: Sergio Paracuellos, Pavel Machek, UNGLinuxDriver, netdev
In-Reply-To: <20190220150826.GI13075@lunn.ch>
On 2/20/19 7:08 AM, Andrew Lunn wrote:
>> +static void mib_monitor(struct timer_list *t)
>> +{
>> + struct ksz_device *dev = from_timer(dev, t, mib_read_timer);
>> + const struct dsa_port *dp;
>> + struct net_device *netdev;
>> + struct ksz_port_mib *mib;
>> + struct ksz_port *p;
>> + int i;
>> +
>> + mod_timer(&dev->mib_read_timer, jiffies + dev->mib_read_interval);
>> +
>> + /* Check which port needs to read MIB counters. */
>> + for (i = 0; i < dev->mib_port_cnt; i++) {
>> + p = &dev->ports[i];
>> + if (!p->on)
>> + continue;
>> + dp = dsa_to_port(dev->ds, i);
>> + netdev = dp->slave;
>> +
>> + mib = &p->mib;
>> + mutex_lock(&mib->cnt_mutex);
>> +
>> + /* Read only dropped counters when link is not up. */
>> + if (netdev && netdev->phydev && !netdev->phydev->link)
>> + mib->cnt_ptr = dev->reg_mib_cnt;
>> + mutex_unlock(&mib->cnt_mutex);
>> + p->read = true;
>> + }
>> + schedule_work(&dev->mib_read);
>> +}
>
> Hi Tristram
>
> This is much easier to understand. Thanks for making the change.
>
> However, i suspect Florian was suggesting you use
> netif_carrier_ok(netdev), not poke around inside the phydev structure.
Yes indeed.
--
Florian
^ permalink raw reply
* Re: [PATCH net 1/2] ipv6: route: enforce RCU protection in rt6_update_exception_stamp_rt()
From: Paolo Abeni @ 2019-02-20 17:22 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
In-Reply-To: <1ac484e7626a5d5e7c22de13485575de30ad7fe0.1550682516.git.pabeni@redhat.com>
On Wed, 2019-02-20 at 18:10 +0100, Paolo Abeni wrote:
> We must access rt6_info->from under RCU read lock: move the
> dereference under such lock, with proper annotation, and use
> rcu_access_pointer() to check for null value outside the lock.
>
> Fixes: a68886a69180 ("net/ipv6: Make from in rt6_info rcu protected")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/ipv6/route.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index bd09abd1fb22..cbaa8745d9ff 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1610,15 +1610,15 @@ static int rt6_remove_exception_rt(struct rt6_info *rt)
> static void rt6_update_exception_stamp_rt(struct rt6_info *rt)
> {
> struct rt6_exception_bucket *bucket;
> - struct fib6_info *from = rt->from;
> struct in6_addr *src_key = NULL;
> struct rt6_exception *rt6_ex;
> + struct fib6_info *from;
>
> - if (!from ||
> - !(rt->rt6i_flags & RTF_CACHE))
> + if (!rcu_access_pointer(rt->from) || !(rt->rt6i_flags & RTF_CACHE))
> return;
>
> rcu_read_lock();
> + from = rcu_dereference(rt->from);
-ELOWONCOFFEE: even this one is racy, as rt->from can go away due to
underlying device removal between the two fetch operation.
I'll send a v2.
Again, I'm sorry for the noise,
Paolo
^ permalink raw reply
* Re: [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: Florian Fainelli @ 2019-02-20 17:22 UTC (permalink / raw)
To: Russell King, Andrew Lunn, Vivien Didelot; +Cc: David S. Miller, netdev
In-Reply-To: <E1gwPBM-0008SQ-CM@rmk-PC.armlinux.org.uk>
On 2/20/19 2:32 AM, Russell King wrote:
> When a DSA port is added to a bridge and brought up, the resulting STP
> state programmed into the hardware depends on the order that these
> operations are performed. However, the Linux bridge code believes that
> the port is in disabled mode.
>
> If the DSA port is first added to a bridge and then brought up, it will
> be in blocking mode. If it is brought up and then added to the bridge,
> it will be in disabled mode.
>
> This difference is caused by DSA always setting the STP mode in
> dsa_port_enable() whether or not this port is part of a bridge. Since
> bridge always sets the STP state when the port is added, brought up or
> taken down, it is unnecessary for us to manipulate the STP state.
>
> Apparently, this code was copied from Rocker, and the very next day a
> similar fix for Rocker was merged but was not propagated to DSA. See
> e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
>
> Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Nice example of cargo cult programming, thanks for fixing this!
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
--
Florian
^ permalink raw reply
* [PATCH net v2] ipv6: route: purge exception on removal
From: Paolo Abeni @ 2019-02-20 17:18 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
When a netdevice is unregistered, we flush the relevant exception
via rt6_sync_down_dev() -> fib6_ifdown() -> fib6_del() -> fib6_del_route().
Finally, we end-up calling rt6_remove_exception(), where we release
the relevant dst, while we keep the references to the related fib6_info and
dev. Such references should be released later when the dst will be
destroyed.
There are a number of caches that can keep the exception around for an
unlimited amount of time - namely dst_cache, possibly even socket cache.
As a result device registration may hang, as demonstrated by this script:
ip netns add cl
ip netns add rt
ip netns add srv
ip netns exec rt sysctl -w net.ipv6.conf.all.forwarding=1
ip link add name cl_veth type veth peer name cl_rt_veth
ip link set dev cl_veth netns cl
ip -n cl link set dev cl_veth up
ip -n cl addr add dev cl_veth 2001::2/64
ip -n cl route add default via 2001::1
ip -n cl link add tunv6 type ip6tnl mode ip6ip6 local 2001::2 remote 2002::1 hoplimit 64 dev cl_veth
ip -n cl link set tunv6 up
ip -n cl addr add 2013::2/64 dev tunv6
ip link set dev cl_rt_veth netns rt
ip -n rt link set dev cl_rt_veth up
ip -n rt addr add dev cl_rt_veth 2001::1/64
ip link add name rt_srv_veth type veth peer name srv_veth
ip link set dev srv_veth netns srv
ip -n srv link set dev srv_veth up
ip -n srv addr add dev srv_veth 2002::1/64
ip -n srv route add default via 2002::2
ip -n srv link add tunv6 type ip6tnl mode ip6ip6 local 2002::1 remote 2001::2 hoplimit 64 dev srv_veth
ip -n srv link set tunv6 up
ip -n srv addr add 2013::1/64 dev tunv6
ip link set dev rt_srv_veth netns rt
ip -n rt link set dev rt_srv_veth up
ip -n rt addr add dev rt_srv_veth 2002::2/64
ip netns exec srv netserver & sleep 0.1
ip netns exec cl ping6 -c 4 2013::1
ip netns exec cl netperf -H 2013::1 -t TCP_STREAM -l 3 & sleep 1
ip -n rt link set dev rt_srv_veth mtu 1400
wait %2
ip -n cl link del cl_veth
This commit addresses the issue purging all the references held by the
exception at time, as we currently do for e.g. ipv6 pcpu dst entries.
v1 -> v2:
- re-order the code to avoid accessing dst and net after dst_dev_put()
Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/route.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 964491cf3672..bd09abd1fb22 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1274,18 +1274,29 @@ static DEFINE_SPINLOCK(rt6_exception_lock);
static void rt6_remove_exception(struct rt6_exception_bucket *bucket,
struct rt6_exception *rt6_ex)
{
+ struct fib6_info *from;
struct net *net;
if (!bucket || !rt6_ex)
return;
net = dev_net(rt6_ex->rt6i->dst.dev);
+ net->ipv6.rt6_stats->fib_rt_cache--;
+
+ /* purge completely the exception to allow releasing the held resources:
+ * some [sk] cache may keep the dst around for unlimited time
+ */
+ from = rcu_dereference_protected(rt6_ex->rt6i->from,
+ lockdep_is_held(&rt6_exception_lock));
+ rcu_assign_pointer(rt6_ex->rt6i->from, NULL);
+ fib6_info_release(from);
+ dst_dev_put(&rt6_ex->rt6i->dst);
+
hlist_del_rcu(&rt6_ex->hlist);
dst_release(&rt6_ex->rt6i->dst);
kfree_rcu(rt6_ex, rcu);
WARN_ON_ONCE(!bucket->depth);
bucket->depth--;
- net->ipv6.rt6_stats->fib_rt_cache--;
}
/* Remove oldest rt6_ex in bucket and free the memory
--
2.20.1
^ permalink raw reply related
* [PATCH net 2/2] ipv6: route: enforce RCU protection in ip6_route_check_nh_onlink()
From: Paolo Abeni @ 2019-02-20 17:10 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
In-Reply-To: <cover.1550682516.git.pabeni@redhat.com>
We need a RCU critical section around rt6_info->from deference, and
proper annotation.
Fixes: 4ed591c8ab44 ("net/ipv6: Allow onlink routes to have a device mismatch if it is the default route")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/route.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index cbaa8745d9ff..3b526a070299 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2753,20 +2753,24 @@ static int ip6_route_check_nh_onlink(struct net *net,
u32 tbid = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
const struct in6_addr *gw_addr = &cfg->fc_gateway;
u32 flags = RTF_LOCAL | RTF_ANYCAST | RTF_REJECT;
+ struct fib6_info *from;
struct rt6_info *grt;
int err;
err = 0;
grt = ip6_nh_lookup_table(net, cfg, gw_addr, tbid, 0);
if (grt) {
+ rcu_read_lock();
+ from = rcu_dereference(grt->from);
if (!grt->dst.error &&
/* ignore match if it is the default route */
- grt->from && !ipv6_addr_any(&grt->from->fib6_dst.addr) &&
+ from && !ipv6_addr_any(&from->fib6_dst.addr) &&
(grt->rt6i_flags & flags || dev != grt->dst.dev)) {
NL_SET_ERR_MSG(extack,
"Nexthop has invalid gateway or device mismatch");
err = -EINVAL;
}
+ rcu_read_unlock();
ip6_rt_put(grt);
}
--
2.20.1
^ permalink raw reply related
* [PATCH net 1/2] ipv6: route: enforce RCU protection in rt6_update_exception_stamp_rt()
From: Paolo Abeni @ 2019-02-20 17:10 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
In-Reply-To: <cover.1550682516.git.pabeni@redhat.com>
We must access rt6_info->from under RCU read lock: move the
dereference under such lock, with proper annotation, and use
rcu_access_pointer() to check for null value outside the lock.
Fixes: a68886a69180 ("net/ipv6: Make from in rt6_info rcu protected")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
net/ipv6/route.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index bd09abd1fb22..cbaa8745d9ff 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1610,15 +1610,15 @@ static int rt6_remove_exception_rt(struct rt6_info *rt)
static void rt6_update_exception_stamp_rt(struct rt6_info *rt)
{
struct rt6_exception_bucket *bucket;
- struct fib6_info *from = rt->from;
struct in6_addr *src_key = NULL;
struct rt6_exception *rt6_ex;
+ struct fib6_info *from;
- if (!from ||
- !(rt->rt6i_flags & RTF_CACHE))
+ if (!rcu_access_pointer(rt->from) || !(rt->rt6i_flags & RTF_CACHE))
return;
rcu_read_lock();
+ from = rcu_dereference(rt->from);
bucket = rcu_dereference(from->rt6i_exception_bucket);
#ifdef CONFIG_IPV6_SUBTREES
--
2.20.1
^ permalink raw reply related
* [PATCH net 0/2] ipv6: route: enforce RCU protection for fib6_info->from
From: Paolo Abeni @ 2019-02-20 17:10 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
This series addresses a couple of RCU left-over dating back to rt6_info->from
conversion to RCU
Paolo Abeni (2):
ipv6: route: enforce RCU protection in rt6_update_exception_stamp_rt()
ipv6: route: enforce RCU protection in ip6_route_check_nh_onlink()
net/ipv6/route.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Alexei Starovoitov @ 2019-02-20 17:07 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: ast, netdev
In-Reply-To: <20190220110629.21646-1-daniel@iogearbox.net>
On Wed, Feb 20, 2019 at 12:06:29PM +0100, Daniel Borkmann wrote:
> In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> a check was added for BPF_PROG_RUN() that for every invocation preemption has
> to be disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this
> does not count for seccomp because only cBPF -> eBPF is loaded here and it does
> not make use of any functionality that would require this assertion. Fix this
> false positive by adding and using __BPF_PROG_RUN() variant that does not have
> the cant_sleep(); check.
>
> Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> include/linux/filter.h | 9 ++++++++-
> kernel/seccomp.c | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index f32b3ec..2648fd7 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -533,7 +533,14 @@ struct sk_filter {
> struct bpf_prog *prog;
> };
>
> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> +#define bpf_prog_run__non_preempt(prog, ctx) \
> + ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
> +/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
> +#define BPF_PROG_RUN(prog, ctx) \
> + bpf_prog_run__non_preempt(prog, ctx)
> +/* Direct use for cBPF -> eBPF only, but not for native eBPF. */
I think the comment is too abstract.
May be it should say that this is seccomp cBPF only ?
And macro name should be explicit as well ?
> +#define __BPF_PROG_RUN(prog, ctx) \
> + (*(prog)->bpf_func)(ctx, (prog)->insnsi)
>
> #define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e815781..826d4e4 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
> * value always takes priority (ignoring the DATA).
> */
> for (; f; f = f->prev) {
> - u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
> + u32 cur_ret = __BPF_PROG_RUN(f->prog, sd);
>
> if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
> ret = cur_ret;
> --
> 2.9.5
>
^ permalink raw reply
* Re: [PATCH iproute2] ss: Render buffer to output every time a number of chunks are allocated
From: Stefano Brivio @ 2019-02-20 16:58 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Eric Dumazet, Phil Sutter, David Ahern, Sabrina Dubroca, netdev
In-Reply-To: <03dd56e5161a3c1270a21c4ba3f6e695793dbb74.1550105375.git.sbrivio@redhat.com>
Hi Stephen,
I think this patch is reasonably tested now. Eric, who reported the
original issue, is also satisfied with it. Is there any issue with it?
--
Stefano
On Thu, 14 Feb 2019 01:58:32 +0100
Stefano Brivio <sbrivio@redhat.com> wrote:
> Eric reported that, with 10 million sockets, ss -emoi (about 1000 bytes
> output per socket) can easily lead to OOM (buffer would grow to 10GB of
> memory).
>
> Limit the maximum size of the buffer to five chunks, 1M each. Render and
> flush buffers whenever we reach that.
>
> This might make the resulting blocks slightly unaligned between them, with
> occasional loss of readability on lines occurring every 5k to 50k sockets
> approximately. Something like (from ss -tu):
>
> [...]
> CLOSE-WAIT 32 0 192.168.1.50:35232 10.0.0.1:https
> ESTAB 0 0 192.168.1.50:53820 10.0.0.1:https
> ESTAB 0 0 192.168.1.50:46924 10.0.0.1:https
> CLOSE-WAIT 32 0 192.168.1.50:35228 10.0.0.1:https
> [...]
>
> However, I don't actually expect any human user to scroll through that
> amount of sockets, so readability should be preserved when it matters.
>
> The bulk of the diffstat comes from moving field_next() around, as we now
> call render() from it. Functionally, this is implemented by six lines of
> code, most of them in field_next().
>
> Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
> Fixes: 691bd854bf4a ("ss: Buffer raw fields first, then render them as a table")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
> Eric, it would be nice if you could test this with your bazillion sockets,
> I checked this with -emoi and "only" 500,000 sockets.
>
> misc/ss.c | 68 ++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 40 insertions(+), 28 deletions(-)
>
> diff --git a/misc/ss.c b/misc/ss.c
> index 9e821faf0d31..28bdcba72d73 100644
> --- a/misc/ss.c
> +++ b/misc/ss.c
> @@ -52,7 +52,8 @@
> #include <linux/tipc_sockets_diag.h>
>
> #define MAGIC_SEQ 123456
> -#define BUF_CHUNK (1024 * 1024)
> +#define BUF_CHUNK (1024 * 1024) /* Buffer chunk allocation size */
> +#define BUF_CHUNKS_MAX 5 /* Maximum number of allocated buffer chunks */
> #define LEN_ALIGN(x) (((x) + 1) & ~1)
>
> #define DIAG_REQUEST(_req, _r) \
> @@ -176,6 +177,7 @@ static struct {
> struct buf_token *cur; /* Position of current token in chunk */
> struct buf_chunk *head; /* First chunk */
> struct buf_chunk *tail; /* Current chunk */
> + int chunks; /* Number of allocated chunks */
> } buffer;
>
> static const char *TCP_PROTO = "tcp";
> @@ -936,6 +938,8 @@ static struct buf_chunk *buf_chunk_new(void)
>
> new->end = buffer.cur->data;
>
> + buffer.chunks++;
> +
> return new;
> }
>
> @@ -1080,33 +1084,6 @@ static int field_is_last(struct column *f)
> return f - columns == COL_MAX - 1;
> }
>
> -static void field_next(void)
> -{
> - field_flush(current_field);
> -
> - if (field_is_last(current_field))
> - current_field = columns;
> - else
> - current_field++;
> -}
> -
> -/* Walk through fields and flush them until we reach the desired one */
> -static void field_set(enum col_id id)
> -{
> - while (id != current_field - columns)
> - field_next();
> -}
> -
> -/* Print header for all non-empty columns */
> -static void print_header(void)
> -{
> - while (!field_is_last(current_field)) {
> - if (!current_field->disabled)
> - out("%s", current_field->header);
> - field_next();
> - }
> -}
> -
> /* Get the next available token in the buffer starting from the current token */
> static struct buf_token *buf_token_next(struct buf_token *cur)
> {
> @@ -1132,6 +1109,7 @@ static void buf_free_all(void)
> free(tmp);
> }
> buffer.head = NULL;
> + buffer.chunks = 0;
> }
>
> /* Get current screen width, default to 80 columns if TIOCGWINSZ fails */
> @@ -1294,6 +1272,40 @@ static void render(void)
> current_field = columns;
> }
>
> +/* Move to next field, and render buffer if we reached the maximum number of
> + * chunks, at the last field in a line.
> + */
> +static void field_next(void)
> +{
> + if (field_is_last(current_field) && buffer.chunks >= BUF_CHUNKS_MAX) {
> + render();
> + return;
> + }
> +
> + field_flush(current_field);
> + if (field_is_last(current_field))
> + current_field = columns;
> + else
> + current_field++;
> +}
> +
> +/* Walk through fields and flush them until we reach the desired one */
> +static void field_set(enum col_id id)
> +{
> + while (id != current_field - columns)
> + field_next();
> +}
> +
> +/* Print header for all non-empty columns */
> +static void print_header(void)
> +{
> + while (!field_is_last(current_field)) {
> + if (!current_field->disabled)
> + out("%s", current_field->header);
> + field_next();
> + }
> +}
> +
> static void sock_state_print(struct sockstat *s)
> {
> const char *sock_name;
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Read in tls_push_record
From: Dmitry Vyukov @ 2019-02-20 16:55 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, davejwatson, David Miller, LKML, netdev,
syzkaller-bugs
In-Reply-To: <000000000000ccb25d0576c17446@google.com>
On Wed, Sep 26, 2018 at 9:49 AM syzbot
<syzbot+942a1909765f0413329b@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 739d0def85ca Merge branch 'hv_netvsc-Support-LRO-RSC-in-th..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=13aa179e400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=e79b9299baeb2298
> dashboard link: https://syzkaller.appspot.com/bug?extid=942a1909765f0413329b
> 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+942a1909765f0413329b@syzkaller.appspotmail.com
>
> ==================================================================
> BUG: KASAN: slab-out-of-bounds in sg_mark_end
> include/linux/scatterlist.h:195 [inline]
> BUG: KASAN: slab-out-of-bounds in tls_push_record+0x1243/0x1330
> net/tls/tls_sw.c:521
> Read of size 8 at addr ffff8801a4d235f8 by task syz-executor1/7668
This is probably:
#syz dup: KASAN: out-of-bounds Write in tls_push_record
> CPU: 1 PID: 7668 Comm: syz-executor1 Not tainted 4.19.0-rc4+ #228
> 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
> sg_mark_end include/linux/scatterlist.h:195 [inline]
> tls_push_record+0x1243/0x1330 net/tls/tls_sw.c:521
> tls_sw_push_pending_record+0x22/0x30 net/tls/tls_sw.c:558
> tls_handle_open_record net/tls/tls_main.c:155 [inline]
> tls_sk_proto_close+0x439/0x750 net/tls/tls_main.c:272
> 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:0x411151
> Code: 75 14 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 0f 83 34 19 00 00 c3 48
> 83 ec 08 e8 0a fc ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48
> 89 c2 e8 53 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
> RSP: 002b:00007fff8ba9e200 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
> RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000411151
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000004
> RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000001
> R10: 00007fff8ba9e130 R11: 0000000000000293 R12: 000000000000000f
> R13: 0000000000086e3d R14: 0000000000000412 R15: badc0ffeebadface
>
> Allocated by task 0:
> (stack is not available)
>
> Freed by task 0:
> (stack is not available)
>
> The buggy address belongs to the object at ffff8801a4d22d80
> which belongs to the cache kmalloc-2048 of size 2048
> The buggy address is located 120 bytes to the right of
> 2048-byte region [ffff8801a4d22d80, ffff8801a4d23580)
> The buggy address belongs to the page:
> page:ffffea0006934880 count:1 mapcount:0 mapping:ffff8801da800c40 index:0x0
> compound_mapcount: 0
> flags: 0x2fffc0000008100(slab|head)
> raw: 02fffc0000008100 ffffea000717fd08 ffff8801da801948 ffff8801da800c40
> raw: 0000000000000000 ffff8801a4d22500 0000000100000003 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffff8801a4d23480: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ffff8801a4d23500: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > ffff8801a4d23580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> ^
> ffff8801a4d23600: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8801a4d23680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ==================================================================
>
>
> ---
> 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.
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/000000000000ccb25d0576c17446%40google.com.
> For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [RFC v1 15/19] RDMA/irdma: Add miscellaneous utility definitions
From: Jason Gunthorpe @ 2019-02-20 16:53 UTC (permalink / raw)
To: Saleem, Shiraz
Cc: dledford@redhat.com, davem@davemloft.net,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
Ismail, Mustafa, Kirsher, Jeffrey T
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A5A460B3@fmsmsx124.amr.corp.intel.com>
On Wed, Feb 20, 2019 at 02:53:18PM +0000, Saleem, Shiraz wrote:
> >Subject: Re: [RFC v1 15/19] RDMA/irdma: Add miscellaneous utility definitions
> >
> >On Fri, Feb 15, 2019 at 11:11:02AM -0600, Shiraz Saleem wrote:
> >> From: Mustafa Ismail <mustafa.ismail@intel.com>
> >>
> >> Add miscellaneous utility functions and headers.
> >>
>
> [....]
>
> >
> >> +#define to_device(ptr) \
> >> + (&((struct pci_dev *)((ptr)->hw->dev_context))->dev)
> >
> >?? Seems like this wants to be container_of??
>
> Yes.
>
> >
> >> +/**
> >> + * irdma_insert_wqe_hdr - write wqe header
> >> + * @wqe: cqp wqe for header
> >> + * @header: header for the cqp wqe
> >> + */
> >> +static inline void irdma_insert_wqe_hdr(__le64 *wqe, u64 hdr) {
> >> + wmb(); /* make sure WQE is populated before polarity is set */
> >> + set_64bit_val(wqe, 24, hdr);
> >
> >Generally don't like seeing wmbs in drivers.. Are you sure this isn't supposed to
> >be smp_store_release(), or dma_wmb() perhaps?
>
> Why is wmb() an issue in drivers?
There are many ways to get a barrier that are much clearer and
maintainable then some random wmb. Rarely do drivers actually need
wmb.
> >> +/**
> >> + * irdma_allocate_dma_mem - Memory alloc helper fn
> >> + * @hw: pointer to the HW structure
> >> + * @mem: ptr to mem struct to fill out
> >> + * @size: size of memory requested
> >> + * @alignment: what to align the allocation to */ enum
> >> +irdma_status_code irdma_allocate_dma_mem(struct irdma_hw *hw,
> >> + struct irdma_dma_mem *mem,
> >> + u64 size,
> >> + u32 alignment)
> >> +{
> >> + struct pci_dev *pcidev = (struct pci_dev *)hw->dev_context;
> >> +
> >> + if (!mem)
> >> + return IRDMA_ERR_PARAM;
> >> +
> >> + mem->size = ALIGN(size, alignment);
> >> + mem->va = dma_alloc_coherent(&pcidev->dev, mem->size,
> >> + (dma_addr_t *)&mem->pa, GFP_KERNEL);
> >> + if (!mem->va)
> >> + return IRDMA_ERR_NO_MEMORY;
> >> +
> >> + return 0;
> >> +}
> >
> >More wrappers? Why?
>
> I agree on your previous comment on wrapper for usecnt tracking but this does
> consolidate some common code and avoid duplication.
IRDMA_ERR_PARAM seems like an nonsense thing to do, and why does this
driver have its own custom error codes anyhow? Don't like it, it is
very stange. Once you strip that away there is no code to share.
Jason
^ permalink raw reply
* Re: [RFC v1 12/19] RDMA/irdma: Implement device supported verb APIs
From: Jason Gunthorpe @ 2019-02-20 16:51 UTC (permalink / raw)
To: Saleem, Shiraz
Cc: dledford@redhat.com, davem@davemloft.net,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
Ismail, Mustafa, Kirsher, Jeffrey T
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A5A460A0@fmsmsx124.amr.corp.intel.com>
On Wed, Feb 20, 2019 at 02:52:31PM +0000, Saleem, Shiraz wrote:
> >All lists of things should be sorted. I saw many examples of unsorted lists.
>
> OK. We weren't aware of this rule in kernel drivers. Is this subsystem specific?
It is a general kernel preference - it helps avoid unnecessary merge
conflicts. Lists in kconfig, makefiles, etc should all be
sorted. Other order-independent lists, like ops, and what not should
be sorted for the same reasons.
> >> + iwibdev->ibdev.iwcm->add_ref = irdma_add_ref;
> >> + iwibdev->ibdev.iwcm->rem_ref = irdma_rem_ref;
> >> + iwibdev->ibdev.iwcm->get_qp = irdma_get_qp;
> >> + iwibdev->ibdev.iwcm->connect = irdma_connect;
> >> + iwibdev->ibdev.iwcm->accept = irdma_accept;
> >> + iwibdev->ibdev.iwcm->reject = irdma_reject;
> >> + iwibdev->ibdev.iwcm->create_listen = irdma_create_listen;
> >> + iwibdev->ibdev.iwcm->destroy_listen = irdma_destroy_listen;
> >
> >Huh. These should probably be moved into the ops structure too.
>
> Not sure. It looks cleaner this way. These are iWARP CM
> specific. Why allocate them for all devices?
Not sure a few bytes really matter.
Jason
^ permalink raw reply
* Re: [RFC v1 17/19] RDMA/irdma: Add ABI definitions
From: Jason Gunthorpe @ 2019-02-20 16:50 UTC (permalink / raw)
To: Saleem, Shiraz
Cc: dledford@redhat.com, davem@davemloft.net,
linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
Ismail, Mustafa, Kirsher, Jeffrey T
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A5A46077@fmsmsx124.amr.corp.intel.com>
On Wed, Feb 20, 2019 at 02:52:03PM +0000, Saleem, Shiraz wrote:
> >Subject: Re: [RFC v1 17/19] RDMA/irdma: Add ABI definitions
> >
> >On Fri, Feb 15, 2019 at 11:11:04AM -0600, Shiraz Saleem wrote:
> >> From: Mustafa Ismail <mustafa.ismail@intel.com>
> >>
> >> Add ABI definitions for irdma.
>
> [....]
> >>
> >> +
> >> +#include <linux/types.h>
> >> +
> >> +#define IRDMA_ABI_VER 6
> >
> >Starting with high numbers?
>
> It's a bump on the current i40iw ABI ver. of 5 since we
> want to be compatible and support current rdma-core's libi40iw
> for Gen1 (X722) device.
i40iw is one of the drivers that doesn't do ABI versions right, so
this is all meaningless. You should probably fix it:
static const struct verbs_device_ops i40iw_udev_ops = {
.name = "i40iw",
.match_min_abi_version = 0,
.match_max_abi_version = INT_MAX,
0 and INT_MAX need to be something sensible.
> >This won't even compile like this - don't forget you have to send the rdma-core
> >PR along with the kernel patches. You should already be running the travis
> >checks yourself. rdma-core should detect malformed user space headers..
>
> Yes. We will be sending the rdma-core patches soon.
> Maybe we are missing something here, but this did compile with libirdma
> in rdma-core-v22, but we havent run travis checks yet.
travis does the tests.
Jason
^ permalink raw reply
* PROBLEM: dev_hard_start_xmit general protection fault on 4.19.18
From: Jesse Hathaway @ 2019-02-20 16:48 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller
After an uptime of 2-4 days our routers are hitting a general protection fault
in dev_hard_start_xmit. We are going to try the latest 4.19.24 release to see
if the bug has been resolved, but I didn't see any obvious commits in the logs.
We are also going to test with a much older 4.9.159 kernel as starting point to
finding when this problem was introduced. Please let me know if there is any
additional information I can provide or any test patches you would like me to
try. Thanks, Jesse Hathaway
Decoded stacktrace:
decode_stacktrace.sh was unable to decode the RIP line, but gdb was able to, if
someone knows why that failed I would love to know.
(gdb) l *dev_hard_start_xmit+0x38
0xffffffff815e7488 is in dev_hard_start_xmit (net/core/dev.c:3256).
3251 {
3252 struct sk_buff *skb = first;
3253 int rc = NETDEV_TX_OK;
3254
3255 while (skb) {
3256 struct sk_buff *next = skb->next;
3257
3258 skb->next = NULL;
3259 rc = xmit_one(skb, dev, txq, next != NULL);
3260 if (unlikely(!dev_xmit_complete(rc))) {
(gdb)
[423866.182835] general protection fault: 0000 [#1] SMP PTI
[423866.188774] CPU: 4 PID: 0 Comm: swapper/4 Tainted: G E
4.19.18-bt7u1-amd64 #1
[423866.198308] Hardware name: Dell Inc. PowerEdge R730/0599V5, BIOS
2.8.0 005/17/2018
[423866.206874] RIP: 0010:dev_hard_start_xmit (??:?)
[423866.212522] Code: 53 48 83 ec 28 48 85 ff 48 89 54 24 08 48 89 4c
24 18 0f 84 b9 01 00 00 48 8d 86 90 00 00 00 48 89 f5 48 89 fb 48 89
44 24 10 <4c> 8b 33 48 c7 03 00 00 00 00 48 8b 05 77 46 b4 00 4d 85 f6
0f 95
All code
========
0: 53 push %rbx
1: 48 83 ec 28 sub $0x28,%rsp
5: 48 85 ff test %rdi,%rdi
8: 48 89 54 24 08 mov %rdx,0x8(%rsp)
d: 48 89 4c 24 18 mov %rcx,0x18(%rsp)
12: 0f 84 b9 01 00 00 je 0x1d1
18: 48 8d 86 90 00 00 00 lea 0x90(%rsi),%rax
1f: 48 89 f5 mov %rsi,%rbp
22: 48 89 fb mov %rdi,%rbx
25: 48 89 44 24 10 mov %rax,0x10(%rsp)
2a:* 4c 8b 33 mov (%rbx),%r14 <-- trapping instruction
2d: 48 c7 03 00 00 00 00 movq $0x0,(%rbx)
34: 48 8b 05 77 46 b4 00 mov 0xb44677(%rip),%rax # 0xb446b2
3b: 4d 85 f6 test %r14,%r14
3e: 0f .byte 0xf
3f: 95 xchg %eax,%ebp
Code starting with the faulting instruction
===========================================
0: 4c 8b 33 mov (%rbx),%r14
3: 48 c7 03 00 00 00 00 movq $0x0,(%rbx)
a: 48 8b 05 77 46 b4 00 mov 0xb44677(%rip),%rax # 0xb44688
11: 4d 85 f6 test %r14,%r14
14: 0f .byte 0xf
15: 95 xchg %eax,%ebp
[423866.233612] RSP: 0018:ffff96f4af483b18 EFLAGS: 00010202
[423866.239550] RAX: ffff96f3f72b6600 RBX: 2e5903fe657c2d03 RCX:
0000000000000003
[423866.247627] RDX: ffffcc02bf687600 RSI: 00000000fffffe01 RDI:
ffffffffb69e864d
[423866.255703] RBP: ffff96f4a802a000 R08: 0000000000000001 R09:
00000000000003e8
[423866.263779] R10: 00000000000002f5 R11: ffff96f4a86ff940 R12:
ffff96f4a802a000
[423866.271854] R13: 0000000000000032 R14: 2e5903fe657c2d03 R15:
0000000000000000
[423866.279931] FS: 0000000000000000(0000) GS:ffff96f4af480000(0000)
knlGS:0000000000000000
[423866.289075] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[423866.295596] CR2: 00007fb3c351f000 CR3: 000000074080a001 CR4:
00000000001606e0
[423866.303671] Call Trace:
[423866.306501] <IRQ>
[423866.308847] __dev_queue_xmit (/source/linux-4.19.18/net/core/dev.c:3830)
[423866.313427] ip_finish_output2
(/source/linux-4.19.18/./include/net/neighbour.h:501
/source/linux-4.19.18/net/ipv4/ip_output.c:229)
[423866.318105] ip_output (/source/linux-4.19.18/net/ipv4/ip_output.c:409)
[423866.321810] ? ip_fragment.constprop.49
(/source/linux-4.19.18/net/ipv4/ip_output.c:293)
[423866.327166] ip_forward (/source/linux-4.19.18/net/ipv4/ip_forward.c:150)
[423866.331161] ? ip_check_defrag
(/source/linux-4.19.18/net/ipv4/ip_forward.c:66)
[423866.335837] ip_rcv (/source/linux-4.19.18/net/ipv4/ip_input.c:527)
[423866.339250] ? ip_rcv_core.isra.15
(/source/linux-4.19.18/net/ipv4/ip_input.c:403)
[423866.344314] __netif_receive_skb_one_core
(/source/linux-4.19.18/net/core/dev.c:4920)
[423866.349866] netif_receive_skb_internal
(/source/linux-4.19.18/net/core/dev.c:5134)
[423866.355222] napi_gro_receive
(/source/linux-4.19.18/net/core/dev.c:5591
/source/linux-4.19.18/net/core/dev.c:5622)
[423866.359615] ixgbe_poll
(/source/linux-4.19.18/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:2404
/source/linux-4.19.18/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c:3186)
ixgbe
[423866.364488] ? load_balance (/source/linux-4.19.18/kernel/sched/fair.c:8578)
[423866.368875] net_rx_action
(/source/linux-4.19.18/net/core/dev.c:6262
/source/linux-4.19.18/net/core/dev.c:6328)
[423866.373164] __do_softirq
(/source/linux-4.19.18/kernel/softirq.c:292
/source/linux-4.19.18/./include/linux/jump_label.h:142
/source/linux-4.19.18/./include/trace/events/irq.h:142
/source/linux-4.19.18/kernel/softirq.c:293)
[423866.377260] irq_exit (/source/linux-4.19.18/kernel/softirq.c:372
/source/linux-4.19.18/kernel/softirq.c:412)
[423866.380867] do_IRQ
(/source/linux-4.19.18/./arch/x86/include/asm/irq_regs.h:19
/source/linux-4.19.18/./arch/x86/include/asm/irq_regs.h:26
/source/linux-4.19.18/arch/x86/kernel/irq.c:260)
[423866.384378] common_interrupt
(/source/linux-4.19.18/arch/x86/entry/entry_64.S:646)
[423866.388567] </IRQ>
[423866.391587] RIP: 0010:mwait_idle (??:?)
[423866.396925] Code: 01 00 0f ae 38 0f ae f0 31 d2 65 48 8b 04 25 40
5c 01 00 48 89 d1 0f 01 c8 48 8b 00 a8 08 0f 85 30 01 00 00 31 c0 fb
0f 01 c9 <65> 8b 2d a3 13 53 49 0f 1f 44 00 00 eb 07 fb 66 0f 1f 44 00
00 65
All code
========
0: 01 00 add %eax,(%rax)
2: 0f ae 38 clflush (%rax)
5: 0f ae f0 mfence
8: 31 d2 xor %edx,%edx
a: 65 48 8b 04 25 40 5c mov %gs:0x15c40,%rax
11: 01 00
13: 48 89 d1 mov %rdx,%rcx
16: 0f 01 c8 monitor %rax,%rcx,%rdx
19: 48 8b 00 mov (%rax),%rax
1c: a8 08 test $0x8,%al
1e: 0f 85 30 01 00 00 jne 0x154
24: 31 c0 xor %eax,%eax
26: fb sti
27: 0f 01 c9 mwait %rax,%rcx
2a: 65 8b 2d a3 13 53 49 mov %gs:*0x495313a3(%rip),%ebp #
0x495313d4 <-- trapping instruction
31: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
36: eb 07 jmp 0x3f
38: fb sti
39: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
3f: 65 gs
Code starting with the faulting instruction
===========================================
0: 65 8b 2d a3 13 53 49 mov %gs:0x495313a3(%rip),%ebp # 0x495313aa
7: 0f 1f 44 00 00 nopl 0x0(%rax,%rax,1)
c: eb 07 jmp 0x15
e: fb sti
f: 66 0f 1f 44 00 00 nopw 0x0(%rax,%rax,1)
15: 65 gs
[423866.419176] RSP: 0018:ffffac06c01ebe98 EFLAGS: 00000246 ORIG_RAX:
ffffffffffffffdb
[423866.428304] RAX: 0000000000000000 RBX: 0000000000000004 RCX:
0000000000000000
[423866.436944] RDX: 0000000000000000 RSI: ffff96f4af49a760 RDI:
0000000000000004
[423866.445569] RBP: 0000000000000004 R08: 0000000000000000 R09:
0000000000000000
[423866.454198] R10: 0000000000000000 R11: 0000000000000000 R12:
0000000000000000
[423866.462825] R13: ffff96f4ad1aac40 R14: ffff96f4ad1aac40 R15:
ffff96f4ad1aac40
[423866.471446] do_idle (/source/linux-4.19.18/kernel/sched/idle.c:153
/source/linux-4.19.18/kernel/sched/idle.c:262)
[423866.475681] cpu_startup_entry
(/source/linux-4.19.18/kernel/sched/idle.c:368 (discriminator 1))
[423866.480690] start_secondary
(/source/linux-4.19.18/arch/x86/kernel/smpboot.c:272)
[423866.485693] secondary_startup_64
(/source/linux-4.19.18/arch/x86/kernel/head_64.S:243)
[423866.490976] Modules linked in: drbg(E) ansi_cprng(E) echainiv(E)
esp4(E) xfrm4_mode_transport(E) tcp_diag(E) inet_diag(E)
nf_conntrack_netlink(E) xt_nat(E) xt_policy(E) nfnetlink_log(E)
xt_NFLOG(E) xt_limit(E) ipt_REJECT(E) nf_)
[423866.575127] serpent_avx2(E) serpent_avx_x86_64(E)
serpent_sse2_x86_64(E) serpent_generic(E) glue_helper(E)
blowfish_generic(E) blowfish_x86_64(E) blowfish_common(E)
cast5_avx_x86_64(E) cast5_generic(E) cast_common(E) crypto_si)
[423866.658693] crc32c_generic(E) crc32c_intel(E) ext4(E) crc16(E)
mbcache(E) jbd2(E) fscrypto(E) sg(E) sd_mod(E) ehci_pci(E) ahci(E)
ehci_hcd(E) libahci(E) ixgbe(E) libata(E) megaraid_sas(E) dca(E)
usbcore(E) mdio(E) i40e(E) scsi)
[423866.683437] ---[ end trace e0abd70b6f85b1fd ]---
# awk -f scripts/ver_linux
Linux rtr1 4.19.18-bt7u1-amd64 #1 SMP Mon Feb 11 20:09:59 UTC 2019
x86_64 GNU/Linux
GNU C 4.7
GNU Make 3.81
Binutils 2.22
Util-linux 2.20.1
Mount 2.20.1
E2fsprogs 1.42.5
Linux C Library 2.13
Dynamic linker (ldd) 2.13
Linux C++ Library 6.0.17
Procps 3.3.3
Net-tools 1.60
Sh-utils 8.13
Udev 175
Modules Loaded 8021q acpi_power_meter aesni_intel aes_x86_64
af_key ahci ansi_cprng authenc blowfish_common blowfish_generic
blowfish_x86_64 bonding button camellia_aesni_avx2
camellia_aesni_avx_x86_64 camellia_generic camellia_x86_64
cast5_avx_x86_64 cast5_generic cast_common cbc cmac crc16
crc32c_generic crc32c_intel cryptd crypto_simd ctr dca dcdbas
des_generic drbg drm drm_kms_helper dummy echainiv ehci_hcd ehci_pci
esp4 evdev ext4 fscrypto garp glue_helper gre i2c_algo_bit i2c_dev
i2c_i801 i40e inet_diag ioatdma ip_gre ipmi_devintf ipmi_msghandler
ipmi_si ip_set ip_set_hash_net ip_set_hash_netiface
ip_set_hash_netport iptable_filter iptable_mangle iptable_nat
iptable_raw ip_tables ipt_REJECT ip_tunnel iTCO_vendor_support
iTCO_wdt ixgbe jbd2 libahci libata libcrc32c llc loop lpc_ich mbcache
mdio megaraid_sas mei mei_me mgag200 mrp mxm_wmi nf_conntrack
nf_conntrack_netlink nf_conntrack_proto_gre nf_defrag_ipv4 nf_nat
nf_nat_ipv4 nfnetlink nfnetlink_log nf_reject_ipv4 pcbc pcrypt pcspkr
rmd160 scsi_mod sd_mod serpent_avx2 serpent_avx_x86_64 serpent_generic
serpent_sse2_x86_64 sg sha512_generic sha512_ssse3 snd snd_pcm
snd_timer soundcore stp tcp_diag ttm twofish_avx_x86_64 twofish_common
twofish_generic twofish_x86_64 twofish_x86_64_3way usbcore wmi xcbc
xfrm4_mode_transport xfrm_algo x_tables xt_addrtype xt_connmark
xt_conntrack xt_CT xt_limit xt_mark xt_nat xt_NFLOG xt_policy xt_set
xt_tcpudp
^ permalink raw reply
* Re: [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: Vivien Didelot @ 2019-02-20 16:38 UTC (permalink / raw)
To: Russell King; +Cc: Andrew Lunn, Florian Fainelli, David S. Miller, netdev
In-Reply-To: <E1gwPBM-0008SQ-CM@rmk-PC.armlinux.org.uk>
On Wed, 20 Feb 2019 10:32:52 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> When a DSA port is added to a bridge and brought up, the resulting STP
> state programmed into the hardware depends on the order that these
> operations are performed. However, the Linux bridge code believes that
> the port is in disabled mode.
>
> If the DSA port is first added to a bridge and then brought up, it will
> be in blocking mode. If it is brought up and then added to the bridge,
> it will be in disabled mode.
>
> This difference is caused by DSA always setting the STP mode in
> dsa_port_enable() whether or not this port is part of a bridge. Since
> bridge always sets the STP state when the port is added, brought up or
> taken down, it is unnecessary for us to manipulate the STP state.
>
> Apparently, this code was copied from Rocker, and the very next day a
> similar fix for Rocker was merged but was not propagated to DSA. See
> e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
>
> Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH iproute2] bridge: make mcast_flood description consistent
From: Vivien Didelot @ 2019-02-20 16:35 UTC (permalink / raw)
To: Russell King - ARM Linux admin; +Cc: netdev, stephen
In-Reply-To: <20190220000528.pl6uc2w3hcxiwb7v@shell.armlinux.org.uk>
Hi Russell,
On Wed, 20 Feb 2019 00:05:29 +0000, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> I'm not sure if it's in the current iproute2, but there is a
> discrepency between the arguments for 'bridge' stated in the man page
> and the description thereof:
>
> bridge link set dev DEV [ cost COST ] [ priority PRIO ] [ state STATE
> ...
> } ] [ learning_sync { on | off } ] [ flood { on | off } ] [
> ^^^^^^^^^^^^^^^^^^
>
> vs
>
> flooding on or flooding off
> Controls whether a given port will flood unicast traffic for
> which there is no FDB entry. By default this flag is on.
>
> vs the command actually accepting "flood" not "flooding". I spotted
> that in iproute2-4.20.0. I haven't had a chance to generate a patch
> that yet and work out how to submit it, but thanks for leading the
> way!
You are correct! I've sent a v2.
Thank you,
Vivien
^ permalink raw reply
* Re: skb_can_coalesce() merges tcp frags with XFS-related slab objects
From: Eric Dumazet @ 2019-02-20 16:35 UTC (permalink / raw)
To: Vasily Averin, Eric Dumazet, David S. Miller
Cc: Linux Kernel Network Developers, Ilya Dryomov
In-Reply-To: <ed413bee-796e-1c3f-4829-4871c032e1ff@virtuozzo.com>
On 02/20/2019 08:19 AM, Vasily Averin wrote:
> Thank you for explanation,
> though this happen in real life and triggers BUG_ON only if receiving side is located on the same host.
> Is it probably makes sense to add WARN_ON into skb_can_coalesce to detect such cases?
Yes, but please do it only in the sendpage() path, or only in CONFIG_DEBUG_PAGEALLOC / CONFIG_DEBUG_VM cases.
tcp_sendmsg() uses a per task page (look at sk_page_frag()), and it seems
strange to recheck what we already know (it is a page not backed/used by SLAB)
^ permalink raw reply
* [PATCH iproute2 v2] bridge: make mcast_flood description consistent
From: Vivien Didelot @ 2019-02-20 16:33 UTC (permalink / raw)
To: netdev; +Cc: stephen, Russell King, Vivien Didelot
This patch simply changes the description of the mcast_flood flag
with "flood" instead of "be flooded with" to avoid confusion, and be
consistent with the description of the flooding flag, which "Controls
whether a given port will *flood* unicast traffic for which there is
no FDB entry."
At the same time, fix the documentation for the "flood" flag which
is incorrectly described as "flooding on" or "flooding off".
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
man/man8/bridge.8 | 4 ++--
man/man8/ip-link.8.in | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/man/man8/bridge.8 b/man/man8/bridge.8
index 72210f62..13c46386 100644
--- a/man/man8/bridge.8
+++ b/man/man8/bridge.8
@@ -344,7 +344,7 @@ Controls whether a given port will sync MAC addresses learned on device port to
bridge FDB.
.TP
-.BR "flooding on " or " flooding off "
+.BR "flood on " or " flood off "
Controls whether a given port will flood unicast traffic for which there is no FDB entry. By default this flag is on.
.TP
@@ -361,7 +361,7 @@ switch.
.TP
.BR "mcast_flood on " or " mcast_flood off "
-Controls whether a given port will be flooded with multicast traffic for which there is no MDB entry. By default this flag is on.
+Controls whether a given port will flood multicast traffic for which there is no MDB entry. By default this flag is on.
.TP
.BR "neigh_suppress on " or " neigh_suppress off "
diff --git a/man/man8/ip-link.8.in b/man/man8/ip-link.8.in
index 5132f514..cef489a4 100644
--- a/man/man8/ip-link.8.in
+++ b/man/man8/ip-link.8.in
@@ -2155,7 +2155,7 @@ queries.
option above.
.BR mcast_flood " { " on " | " off " }"
-- controls whether a given port will be flooded with multicast traffic for which there is no MDB entry.
+- controls whether a given port will flood multicast traffic for which there is no MDB entry.
.BI group_fwd_mask " MASK "
- set the group forward mask. This is the bitmask that is applied to decide whether to forward incoming frames destined to link-local addresses, ie addresses of the form 01:80:C2:00:00:0X (defaults to 0, ie the bridge does not forward any link-local frames coming on this port).
--
2.20.1
^ permalink raw reply related
* KASAN: use-after-free Read in icmp_sk_exit
From: syzbot @ 2019-02-20 16:23 UTC (permalink / raw)
To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 1f43f400a2cb net: netcp: Fix ethss driver probe issue
git tree: net
console output: https://syzkaller.appspot.com/x/log.txt?x=1276fea2c00000
kernel config: https://syzkaller.appspot.com/x/.config?x=ee434566c893c7b1
dashboard link: https://syzkaller.appspot.com/bug?extid=3d7fa0f0de0f86d0eb4f
compiler: gcc (GCC) 9.0.0 20181231 (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+3d7fa0f0de0f86d0eb4f@syzkaller.appspotmail.com
IPVS: set_ctl: invalid protocol: 63 172.20.20.187:20002
kernel msg: ebtables bug: please report to author: entries_size too small
kernel msg: ebtables bug: please report to author: entries_size too small
==================================================================
BUG: KASAN: use-after-free in inet_ctl_sock_destroy
include/net/inet_common.h:56 [inline]
BUG: KASAN: use-after-free in icmp_sk_exit+0x1ce/0x1f0 net/ipv4/icmp.c:1187
Read of size 8 at addr ffff8880847adc50 by task kworker/u4:1/21
CPU: 0 PID: 21 Comm: kworker/u4:1 Not tainted 5.0.0-rc6+ #85
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: netns cleanup_net
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
__asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:135
inet_ctl_sock_destroy include/net/inet_common.h:56 [inline]
icmp_sk_exit+0x1ce/0x1f0 net/ipv4/icmp.c:1187
ops_exit_list.isra.0+0xb0/0x160 net/core/net_namespace.c:153
cleanup_net+0x3fb/0x960 net/core/net_namespace.c:551
process_one_work+0x98e/0x1790 kernel/workqueue.c:2173
worker_thread+0x98/0xe40 kernel/workqueue.c:2319
kthread+0x357/0x430 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
Allocated by task 25570:
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 mm/kasan/common.c:504 [inline]
kasan_slab_alloc+0xf/0x20 mm/kasan/common.c:411
kmem_cache_alloc+0x12d/0x710 mm/slab.c:3543
sk_prot_alloc+0x67/0x2e0 net/core/sock.c:1471
sk_alloc+0x39/0xf70 net/core/sock.c:1531
inet_create net/ipv4/af_inet.c:321 [inline]
inet_create+0x36a/0xe10 net/ipv4/af_inet.c:247
__sock_create+0x3e6/0x750 net/socket.c:1275
sock_create_kern+0x3b/0x50 net/socket.c:1321
inet_ctl_sock_create+0x9d/0x1f0 net/ipv4/af_inet.c:1614
icmp_sk_init net/ipv4/icmp.c:1203 [inline]
icmp_sk_init+0x120/0x680 net/ipv4/icmp.c:1192
ops_init+0xb6/0x410 net/core/net_namespace.c:129
setup_net+0x2c5/0x730 net/core/net_namespace.c:314
copy_net_ns+0x1d9/0x340 net/core/net_namespace.c:437
create_new_namespaces+0x400/0x7b0 kernel/nsproxy.c:107
unshare_nsproxy_namespaces+0xc2/0x200 kernel/nsproxy.c:206
ksys_unshare+0x440/0x980 kernel/fork.c:2550
__do_sys_unshare kernel/fork.c:2618 [inline]
__se_sys_unshare kernel/fork.c:2616 [inline]
__x64_sys_unshare+0x31/0x40 kernel/fork.c:2616
do_syscall_64+0x103/0x610 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 21:
save_stack+0x45/0xd0 mm/kasan/common.c:73
set_track mm/kasan/common.c:85 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/common.c:458
kasan_slab_free+0xe/0x10 mm/kasan/common.c:466
__cache_free mm/slab.c:3487 [inline]
kmem_cache_free+0x86/0x260 mm/slab.c:3749
sk_prot_free net/core/sock.c:1512 [inline]
__sk_destruct+0x4b6/0x6d0 net/core/sock.c:1596
sk_destruct+0x7b/0x90 net/core/sock.c:1604
__sk_free+0xce/0x300 net/core/sock.c:1615
sk_free+0x42/0x50 net/core/sock.c:1626
sock_put include/net/sock.h:1707 [inline]
sk_common_release+0x224/0x330 net/core/sock.c:3042
raw_close+0x22/0x30 net/ipv4/raw.c:708
inet_release+0x105/0x1f0 net/ipv4/af_inet.c:428
__sock_release+0x1d3/0x250 net/socket.c:579
sock_release+0x18/0x20 net/socket.c:598
inet_ctl_sock_destroy include/net/inet_common.h:56 [inline]
icmp_sk_exit+0x11f/0x1f0 net/ipv4/icmp.c:1187
ops_exit_list.isra.0+0xb0/0x160 net/core/net_namespace.c:153
cleanup_net+0x3fb/0x960 net/core/net_namespace.c:551
process_one_work+0x98e/0x1790 kernel/workqueue.c:2173
worker_thread+0x98/0xe40 kernel/workqueue.c:2319
kthread+0x357/0x430 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:352
The buggy address belongs to the object at ffff8880847ad840
which belongs to the cache RAW(97:syz4) of size 1320
The buggy address is located 1040 bytes inside of
1320-byte region [ffff8880847ad840, ffff8880847add68)
The buggy address belongs to the page:
page:ffffea000211eb00 count:1 mapcount:0 mapping:ffff8880a84bf1c0
index:0xffff8880847ac140 compound_mapcount: 0
flags: 0x1fffc0000010200(slab|head)
raw: 01fffc0000010200 ffff8880a4cd5b38 ffff8880a4cd5b38 ffff8880a84bf1c0
raw: ffff8880847ac140 ffff8880847ac140 0000000100000003 ffff888067b1a1c0
page dumped because: kasan: bad access detected
page->mem_cgroup:ffff888067b1a1c0
Memory state around the buggy address:
ffff8880847adb00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8880847adb80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> ffff8880847adc00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8880847adc80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8880847add00: fb fb fb fb fb fb fb fb fb fb fb fb fb fc fc fc
==================================================================
---
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.
^ permalink raw reply
* Re: skb_can_coalesce() merges tcp frags with XFS-related slab objects
From: Vasily Averin @ 2019-02-20 16:19 UTC (permalink / raw)
To: Eric Dumazet, David S. Miller
Cc: Linux Kernel Network Developers, Ilya Dryomov
In-Reply-To: <2dc88af7-3d29-58e1-00e0-127d1f798c28@gmail.com>
On 2/20/19 6:53 PM, Eric Dumazet wrote:
> On 02/20/2019 05:34 AM, Vasily Averin wrote:
>> Dear David,
>>
>> currently do_tcp_sendpages() calls skb_can_coalesce() to merge proper tcp fragments.
>> If these fragments are slab objects and the data is not transferred out of the local host
>> then tcp_recvmsg() can crash host on BUG_ON (see [2] below).
>>
>> There is known usecase when slab objects are provided to tcp_sendpage:
>> XFS over locally landed network blockdevice.
>>
>> I found few such cases:
>> - _drbd_send_page() had PageSlab() check log time ago.
>> - recently Ilya Dryomov fixed it in ceph
>> by commit 7e241f647dc7 "libceph: fall back to sendmsg for slab pages"
>>
>> Recently OpenVZ team noticed this problem during experiments with
>> XFS over locally-landed iscsi target.
>>
>> I would note: triggered BUG is not a real problem but false alert,
>> that though crashes host.
>>
>> I can fix last problem by adding PageSlab() into iscsi_tcp_segment_map(),
>> however it does not fix the problem completely,
>> there are chances that the problem will be reproduced again with some other filesystems
>> or with some other kind of network blockdevice.
>>
>> David, what do you think, is it probably better to add PageSlab() check
>> directly into skb_can_coalesce()? (see [1] below)
>>
>
> No, this would be wrong.
>
> There is no way a page fragment can be backed by slab object,
> since a page fragment can be shared (the page refcount needs to be manipulated, without slab/slub
> being aware of this)
Thank you for explanation,
though this happen in real life and triggers BUG_ON only if receiving side is located on the same host.
Is it probably makes sense to add WARN_ON into skb_can_coalesce to detect such cases?
> Please fix the callers.
Ok, will do it.
^ permalink raw reply
* Re: [PATCH v2] kcm: remove any offset before parsing messages
From: Tom Herbert @ 2019-02-20 16:18 UTC (permalink / raw)
To: Dominique Martinet
Cc: Tom Herbert, David Miller, Doron Roberts-Kedes, Dave Watson,
Linux Kernel Network Developers, LKML
In-Reply-To: <20190220041151.GA13520@nautica>
On Tue, Feb 19, 2019 at 8:12 PM Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> Dominique Martinet wrote on Fri, Feb 15, 2019:
> > With all that said I guess my patch should work correctly then, I'll try
> > to find some time to check the error does come back up the tcp socket in
> > my reproducer but I have no reason to believe it doesn't.
>
> Ok, so I can confirm this part - the 'csock' does come back up with
> POLLERR if the parse function returns ENOMEM in the current code.
>
Good.
> It also comes back up with POLLERR when the remote side closes the
> connection, which is expected, but I'm having a very hard time
> understanding how an application is supposed to deal with these
> POLLERR after having read the documentation and a bit of
> experimentation.
> I'm not sure how much it would matter for real life (if the other end
> closes the connection most servers would not care about what they said
> just before closing, but I can imagine some clients doing that in real
> life e.g. a POST request they don't care if it succeeds or not)...
> My test program works like this:
> - client connects, sends 10k messages and close()s the socket
> - server loops recving and close()s after 10k messages; it used to be
> recvmsg() directly but it's now using poll then recvmsg.
>
>
> When the client closes the socket, some messages are obviously still "in
> flight", and the server will recv a POLLERR notification on the csock at
> some point with many messages left.
> The documentation says to unattach the csock when you get POLLER. If I
> do that, the kcm socket will no longer give me any message, so all the
> messages still in flight at the time are lost.
So basically it sounds like you're interested in supporting TCP
connections that are half closed. I believe that the error in half
closed is EPIPE, so if the TCP socket returns that it can be ignored
and the socket can continue being attached and used to send data.
Another possibility is to add some linger semantics to an attached
socket. For instance, a large message might be sent so that part of
the messge is queued in TCP and part is queued in the KCM socket.
Unattach would probably break that message. We probably want to linger
option similar to SO_LINGER (or maybe just use the option on the TCP
socket) that means don't complete the detach until any message being
transmitted on the lower socket has been queued.
>
> If I just ignore the csock like I used to, all the messages do come just
> fine, but as said previously on a real error this will just make recvmsg
> or the polling hang forever and I see no way to distinguish a "real"
> error vs. a connection shut down from the remote side with data left in
> the pipe.
> I thought of checking POLLERR on csock and POLLIN not set on kcmsock,
> but even that seems to happen fairly regularily - the kcm sock hasn't
> been filled up, it's still reading from the csock.
>
>
> On the other hand, checking POLLIN on the csock does say there is still
> data left, so I know there is data left on the csock, but this is also
> the case on a real error (e.g. if parser returns -ENOMEM)
> ... And this made me try to read from the csock after detaching it and I
> can resume manual tcp parsing for a few messages until read() fails with
> EPROTO ?! and I cannot seem to be able to get anything out of attaching
> it back to kcm (for e.g. an ENOMEM error that was transient)...
>
>
>
> I'm honestly not sure how the POLLERR notification mechanism works but I
> think it would be much easier to use KCM if we could somehow delay that
> error until KCM is done feeding from the csock (when netparser really
> stops reading from it like on real error, e.g. abort callback maybe?)
> I think it's fine if the csock is closed before the kcm sock message is
> read, but we should not lose messages like this.
Sounds like linger semantics is needed then.
>
>
>
> > I'd like to see some retry on ENOMEM before this is merged though, so
> > while I'm there I'll resend this with a second patch doing that
> > retry,.. I think just not setting strp->interrupted and not reporting
> > the error up might be enough? Will have to try either way.
>
> I also tried playing with that without much success.
> I had assumed just not calling strp_parser_err() (which calls the
> abort_parser cb) would be enough, eventually calling strp_start_timer()
> like the !len case, but no can do.
I think you need to ignore the ENOMEM and have a timer or other
callback to retry the operation in the future.
> With that said, returning 0 from the parse function also raises POLLERR
> on the csock and hangs netparser, so things aren't that simple...
Can you point to where this is happening. If the parse_msg callback
returns 0 that is suppose to indicate that more bytes are needed.
>
>
> I could use a bit of help again.
>
> Thanks,
> --
> Dominique
^ permalink raw reply
* Re: [PATCH 6/6] net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver
From: Tony Lindgren @ 2019-02-20 16:18 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, Kishon Vijay Abraham I, Rob Herring, netdev,
Sekhar Nori, linux-kernel, linux-omap, devicetree,
linux-arm-kernel
In-Reply-To: <1550676319-6440-7-git-send-email-grygorii.strashko@ti.com>
Hi,
* Grygorii Strashko <grygorii.strashko@ti.com> [190220 15:26]:
> Deprecate cpsw-phy-sel driver as it's been replaced with new
> TI phy-gmii-sel PHY driver.
I'm not going to pick up this one, seems that Dave can merge
this later on? That is unless Dave wants to ack this one.
Regards,
Tony
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/ti/Kconfig | 6 +++---
> drivers/net/ethernet/ti/cpsw.h | 6 ++++++
> 2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
> index bb126be1eb72..8b21b40a9fe5 100644
> --- a/drivers/net/ethernet/ti/Kconfig
> +++ b/drivers/net/ethernet/ti/Kconfig
> @@ -49,10 +49,11 @@ config TI_DAVINCI_CPDMA
> will be called davinci_cpdma. This is recommended.
>
> config TI_CPSW_PHY_SEL
> - bool
> + bool "TI CPSW Phy mode Selection (DEPRECATED)"
> + default n
> ---help---
> This driver supports configuring of the phy mode connected to
> - the CPSW.
> + the CPSW. DEPRECATED: use PHY_TI_GMII_SEL.
>
> config TI_CPSW_ALE
> tristate "TI CPSW ALE Support"
> @@ -64,7 +65,6 @@ config TI_CPSW
> depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
> select TI_DAVINCI_CPDMA
> select TI_DAVINCI_MDIO
> - select TI_CPSW_PHY_SEL
> select TI_CPSW_ALE
> select MFD_SYSCON
> select REGMAP
> diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
> index cf111db3dc27..907e05fc22e4 100644
> --- a/drivers/net/ethernet/ti/cpsw.h
> +++ b/drivers/net/ethernet/ti/cpsw.h
> @@ -21,7 +21,13 @@
> ((mac)[2] << 16) | ((mac)[3] << 24))
> #define mac_lo(mac) (((mac)[4] << 0) | ((mac)[5] << 8))
>
> +#if IS_ENABLED(CONFIG_TI_CPSW_PHY_SEL)
> void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave);
> +#else
> +static inline
> +void cpsw_phy_sel(struct device *dev, phy_interface_t phy_mode, int slave)
> +{}
> +#endif
> int ti_cm_get_macid(struct device *dev, int slave, u8 *mac_addr);
>
> #endif /* __CPSW_H__ */
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH net] ipv6: route: purge exception on removal
From: Paolo Abeni @ 2019-02-20 16:00 UTC (permalink / raw)
To: netdev; +Cc: David Ahern, David S. Miller
In-Reply-To: <460ef555bb02ec46d8a6108aa0d7b0095e11afd1.1550662037.git.pabeni@redhat.com>
On Wed, 2019-02-20 at 15:08 +0100, Paolo Abeni wrote:
> When a netdevice is unregistered, we flush the relevant exception
> via rt6_sync_down_dev() -> fib6_ifdown() -> fib6_del() -> fib6_del_route().
>
> Finally, we end-up calling rt6_remove_exception(), where we release
> the relevant dst, while we keep the references to the related fib6_info and
> dev. Such references should be released later when the dst will be
> destroyed.
>
> There are a number of caches that can keep the exception around for an
> unlimited amount of time - namely dst_cache, possibly even socket cache.
> As a result device registration may hang, as demonstrated by this script:
>
> ip netns add cl
> ip netns add rt
> ip netns add srv
> ip netns exec rt sysctl -w net.ipv6.conf.all.forwarding=1
>
> ip link add name cl_veth type veth peer name cl_rt_veth
> ip link set dev cl_veth netns cl
> ip -n cl link set dev cl_veth up
> ip -n cl addr add dev cl_veth 2001::2/64
> ip -n cl route add default via 2001::1
>
> ip -n cl link add tunv6 type ip6tnl mode ip6ip6 local 2001::2 remote 2002::1 hoplimit 64 dev cl_veth
> ip -n cl link set tunv6 up
> ip -n cl addr add 2013::2/64 dev tunv6
>
> ip link set dev cl_rt_veth netns rt
> ip -n rt link set dev cl_rt_veth up
> ip -n rt addr add dev cl_rt_veth 2001::1/64
>
> ip link add name rt_srv_veth type veth peer name srv_veth
> ip link set dev srv_veth netns srv
> ip -n srv link set dev srv_veth up
> ip -n srv addr add dev srv_veth 2002::1/64
> ip -n srv route add default via 2002::2
>
> ip -n srv link add tunv6 type ip6tnl mode ip6ip6 local 2002::1 remote 2001::2 hoplimit 64 dev srv_veth
> ip -n srv link set tunv6 up
> ip -n srv addr add 2013::1/64 dev tunv6
>
> ip link set dev rt_srv_veth netns rt
> ip -n rt link set dev rt_srv_veth up
> ip -n rt addr add dev rt_srv_veth 2002::2/64
>
> ip netns exec srv netserver & sleep 0.1
> ip netns exec cl ping6 -c 4 2013::1
> ip netns exec cl netperf -H 2013::1 -t TCP_STREAM -l 3 & sleep 1
> ip -n rt link set dev rt_srv_veth mtu 1400
> wait %2
>
> ip -n cl link del cl_veth
>
> This commit addresses the issue purging all the references held by the
> exception at time, as we currently do for e.g. ipv6 pcpu dst entries.
>
> Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/ipv6/route.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 964491cf3672..43a819ae46c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -1274,11 +1274,21 @@ static DEFINE_SPINLOCK(rt6_exception_lock);
> static void rt6_remove_exception(struct rt6_exception_bucket *bucket,
> struct rt6_exception *rt6_ex)
> {
> + struct fib6_info *from;
> struct net *net;
>
> if (!bucket || !rt6_ex)
> return;
>
> + /* purge completely the exception to allow releasing the held resources:
> + * some [sk] cache may keep the dst around for unlimited time
> + */
> + from = rcu_dereference_protected(rt6_ex->rt6i->from,
> + lockdep_is_held(&rt6_exception_lock));
> + rcu_assign_pointer(rt6_ex->rt6i->from, NULL);
> + fib6_info_release(from);
> + dst_dev_put(&rt6_ex->rt6i->dst);
> +
> net = dev_net(rt6_ex->rt6i->dst.dev);
uhm... This is possibly racy, if dev goes away on dst_dev_put(). I'll
send a v2 reordering the operation so that we don't access dev (and net
) after dst_dev_put().
Sorry for the noise,
Paolo
^ 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