netdev - Re: [PATCH] net: Revert "net_sched: no need to free qdisc in RCU callback"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Dec 2017 13:59:59 -0800
From:   Jakub Kicinski <kubakici@...pl>
To:     John Fastabend <john.fastabend@...il.com>
Cc:     xiyou.wangcong@...il.com, jiri@...nulli.us, davem@...emloft.net,
        netdev@...r.kernel.org, eric.dumazet@...il.com
Subject: Re: [PATCH] net: Revert "net_sched: no need to free qdisc in RCU
 callback"

On Wed, 20 Dec 2017 12:09:19 -0800, John Fastabend wrote:
> RCU grace period is needed for lockless qdiscs added in the commit
> c5ad119fb6c09 ("net: sched: pfifo_fast use skb_array").
> 
> It is needed now that qdiscs may be lockless otherwise we risk
> free'ing a qdisc that is still in use from datapath. Additionally,
> push list cleanup into RCU callback. Otherwise we risk the datapath
> adding skbs during removal.
> 
> Fixes: c5ad119fb6c09 ("net: sched: pfifo_fast use skb_array")
> Signed-off-by: John Fastabend <john.fastabend@...il.com>

Seems like this revert may be too heavy handed:

# ./tools/testing/selftests/bpf/test_offload.py --log /tmp/log
Test destruction of generic XDP...
Test TC non-offloaded...
Test TC non-offloaded isn't getting bound...
Test TC offloads are off by default...
Test TC offload by default...
Test TC cBPF bytcode tries offload by default...
Test TC cBPF unbound bytecode doesn't offload...
Test TC offloads work...
FAIL: TC filter did not load with TC offloads enabled

And it's triggering:

WARNING: CPU: 15 PID: 1853 at ../drivers/net/netdevsim/bpf.c:372 nsim_bpf_uninit+0x2e/0x41 [netdevsim]

Which is:

   368	void nsim_bpf_uninit(struct netdevsim *ns)
   369	{
   370		WARN_ON(!list_empty(&ns->bpf_bound_progs));
   371		WARN_ON(ns->xdp_prog);
>> 372		WARN_ON(ns->bpf_offloaded);
   373	}

(Meaning the offload was not stopped by the stack before ndo_uninit.)