* [PATCH net] net/sched: pfifo_fast: fix wrong dereference in pfifo_fast_enqueue
@ 2019-08-27 21:18 Davide Caratti
2019-08-28 6:31 ` Paolo Abeni
2019-08-28 22:58 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Davide Caratti @ 2019-08-27 21:18 UTC (permalink / raw)
To: Cong Wang, Jamal Hadi Salim, Jiri Pirko, David S. Miller, netdev
Cc: Paolo Abeni, Stefano Brivio, Li Shuang
Now that 'TCQ_F_CPUSTATS' bit can be cleared, depending on the value of
'TCQ_F_NOLOCK' bit in the parent qdisc, we can't assume anymore that
per-cpu counters are there in the error path of skb_array_produce().
Otherwise, the following splat can be seen:
Unable to handle kernel paging request at virtual address 0000600dea430008
Mem abort info:
ESR = 0x96000005
Exception class = DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
Data abort info:
ISV = 0, ISS = 0x00000005
CM = 0, WnR = 0
user pgtable: 64k pages, 48-bit VAs, pgdp = 000000007b97530e
[0000600dea430008] pgd=0000000000000000, pud=0000000000000000
Internal error: Oops: 96000005 [#1] SMP
[...]
pstate: 10000005 (nzcV daif -PAN -UAO)
pc : pfifo_fast_enqueue+0x524/0x6e8
lr : pfifo_fast_enqueue+0x46c/0x6e8
sp : ffff800d39376fe0
x29: ffff800d39376fe0 x28: 1ffff001a07d1e40
x27: ffff800d03e8f188 x26: ffff800d03e8f200
x25: 0000000000000062 x24: ffff800d393772f0
x23: 0000000000000000 x22: 0000000000000403
x21: ffff800cca569a00 x20: ffff800d03e8ee00
x19: ffff800cca569a10 x18: 00000000000000bf
x17: 0000000000000000 x16: 0000000000000000
x15: 0000000000000000 x14: ffff1001a726edd0
x13: 1fffe4000276a9a4 x12: 0000000000000000
x11: dfff200000000000 x10: ffff800d03e8f1a0
x9 : 0000000000000003 x8 : 0000000000000000
x7 : 00000000f1f1f1f1 x6 : ffff1001a726edea
x5 : ffff800cca56a53c x4 : 1ffff001bf9a8003
x3 : 1ffff001bf9a8003 x2 : 1ffff001a07d1dcb
x1 : 0000600dea430000 x0 : 0000600dea430008
Process ping (pid: 6067, stack limit = 0x00000000dc0aa557)
Call trace:
pfifo_fast_enqueue+0x524/0x6e8
htb_enqueue+0x660/0x10e0 [sch_htb]
__dev_queue_xmit+0x123c/0x2de0
dev_queue_xmit+0x24/0x30
ip_finish_output2+0xc48/0x1720
ip_finish_output+0x548/0x9d8
ip_output+0x334/0x788
ip_local_out+0x90/0x138
ip_send_skb+0x44/0x1d0
ip_push_pending_frames+0x5c/0x78
raw_sendmsg+0xed8/0x28d0
inet_sendmsg+0xc4/0x5c0
sock_sendmsg+0xac/0x108
__sys_sendto+0x1ac/0x2a0
__arm64_sys_sendto+0xc4/0x138
el0_svc_handler+0x13c/0x298
el0_svc+0x8/0xc
Code: f9402e80 d538d081 91002000 8b010000 (885f7c03)
Fix this by testing the value of 'TCQ_F_CPUSTATS' bit in 'qdisc->flags',
before dereferencing 'qdisc->cpu_qstats'.
Fixes: 8a53e616de29 ("net: sched: when clearing NOLOCK, clear TCQ_F_CPUSTATS, too")
CC: Paolo Abeni <pabeni@redhat.com>
CC: Stefano Brivio <sbrivio@redhat.com>
Reported-by: Li Shuang <shuali@redhat.com>
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/sch_generic.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 099797e5409d..137db1cbde85 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -624,8 +624,12 @@ static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
err = skb_array_produce(q, skb);
- if (unlikely(err))
- return qdisc_drop_cpu(skb, qdisc, to_free);
+ if (unlikely(err)) {
+ if (qdisc_is_percpu_stats(qdisc))
+ return qdisc_drop_cpu(skb, qdisc, to_free);
+ else
+ return qdisc_drop(skb, qdisc, to_free);
+ }
qdisc_update_stats_at_enqueue(qdisc, pkt_len);
return NET_XMIT_SUCCESS;
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH net] net/sched: pfifo_fast: fix wrong dereference in pfifo_fast_enqueue
2019-08-27 21:18 [PATCH net] net/sched: pfifo_fast: fix wrong dereference in pfifo_fast_enqueue Davide Caratti
@ 2019-08-28 6:31 ` Paolo Abeni
2019-08-28 22:58 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Abeni @ 2019-08-28 6:31 UTC (permalink / raw)
To: Davide Caratti, Cong Wang, Jamal Hadi Salim, Jiri Pirko,
David S. Miller, netdev
Cc: Stefano Brivio, Li Shuang
On Tue, 2019-08-27 at 23:18 +0200, Davide Caratti wrote:
> Now that 'TCQ_F_CPUSTATS' bit can be cleared, depending on the value of
> 'TCQ_F_NOLOCK' bit in the parent qdisc, we can't assume anymore that
> per-cpu counters are there in the error path of skb_array_produce().
> Otherwise, the following splat can be seen:
>
> Unable to handle kernel paging request at virtual address 0000600dea430008
> Mem abort info:
> ESR = 0x96000005
> Exception class = DABT (current EL), IL = 32 bits
> SET = 0, FnV = 0
> EA = 0, S1PTW = 0
> Data abort info:
> ISV = 0, ISS = 0x00000005
> CM = 0, WnR = 0
> user pgtable: 64k pages, 48-bit VAs, pgdp = 000000007b97530e
> [0000600dea430008] pgd=0000000000000000, pud=0000000000000000
> Internal error: Oops: 96000005 [#1] SMP
> [...]
> pstate: 10000005 (nzcV daif -PAN -UAO)
> pc : pfifo_fast_enqueue+0x524/0x6e8
> lr : pfifo_fast_enqueue+0x46c/0x6e8
> sp : ffff800d39376fe0
> x29: ffff800d39376fe0 x28: 1ffff001a07d1e40
> x27: ffff800d03e8f188 x26: ffff800d03e8f200
> x25: 0000000000000062 x24: ffff800d393772f0
> x23: 0000000000000000 x22: 0000000000000403
> x21: ffff800cca569a00 x20: ffff800d03e8ee00
> x19: ffff800cca569a10 x18: 00000000000000bf
> x17: 0000000000000000 x16: 0000000000000000
> x15: 0000000000000000 x14: ffff1001a726edd0
> x13: 1fffe4000276a9a4 x12: 0000000000000000
> x11: dfff200000000000 x10: ffff800d03e8f1a0
> x9 : 0000000000000003 x8 : 0000000000000000
> x7 : 00000000f1f1f1f1 x6 : ffff1001a726edea
> x5 : ffff800cca56a53c x4 : 1ffff001bf9a8003
> x3 : 1ffff001bf9a8003 x2 : 1ffff001a07d1dcb
> x1 : 0000600dea430000 x0 : 0000600dea430008
> Process ping (pid: 6067, stack limit = 0x00000000dc0aa557)
> Call trace:
> pfifo_fast_enqueue+0x524/0x6e8
> htb_enqueue+0x660/0x10e0 [sch_htb]
> __dev_queue_xmit+0x123c/0x2de0
> dev_queue_xmit+0x24/0x30
> ip_finish_output2+0xc48/0x1720
> ip_finish_output+0x548/0x9d8
> ip_output+0x334/0x788
> ip_local_out+0x90/0x138
> ip_send_skb+0x44/0x1d0
> ip_push_pending_frames+0x5c/0x78
> raw_sendmsg+0xed8/0x28d0
> inet_sendmsg+0xc4/0x5c0
> sock_sendmsg+0xac/0x108
> __sys_sendto+0x1ac/0x2a0
> __arm64_sys_sendto+0xc4/0x138
> el0_svc_handler+0x13c/0x298
> el0_svc+0x8/0xc
> Code: f9402e80 d538d081 91002000 8b010000 (885f7c03)
>
> Fix this by testing the value of 'TCQ_F_CPUSTATS' bit in 'qdisc->flags',
> before dereferencing 'qdisc->cpu_qstats'.
>
> Fixes: 8a53e616de29 ("net: sched: when clearing NOLOCK, clear TCQ_F_CPUSTATS, too")
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Stefano Brivio <sbrivio@redhat.com>
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
> net/sched/sch_generic.c | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
> index 099797e5409d..137db1cbde85 100644
> --- a/net/sched/sch_generic.c
> +++ b/net/sched/sch_generic.c
> @@ -624,8 +624,12 @@ static int pfifo_fast_enqueue(struct sk_buff *skb, struct Qdisc *qdisc,
>
> err = skb_array_produce(q, skb);
>
> - if (unlikely(err))
> - return qdisc_drop_cpu(skb, qdisc, to_free);
> + if (unlikely(err)) {
> + if (qdisc_is_percpu_stats(qdisc))
> + return qdisc_drop_cpu(skb, qdisc, to_free);
> + else
> + return qdisc_drop(skb, qdisc, to_free);
> + }
>
> qdisc_update_stats_at_enqueue(qdisc, pkt_len);
> return NET_XMIT_SUCCESS;
LGTM, thanks Davide!
I just did a code audit of the others pfifo_fast callbacks, I think
this is the last spot in need of such fix.
Acked-by: Paolo Abeni <pabeni@redhat.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH net] net/sched: pfifo_fast: fix wrong dereference in pfifo_fast_enqueue
2019-08-27 21:18 [PATCH net] net/sched: pfifo_fast: fix wrong dereference in pfifo_fast_enqueue Davide Caratti
2019-08-28 6:31 ` Paolo Abeni
@ 2019-08-28 22:58 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2019-08-28 22:58 UTC (permalink / raw)
To: dcaratti; +Cc: xiyou.wangcong, jhs, jiri, netdev, pabeni, sbrivio, shuali
From: Davide Caratti <dcaratti@redhat.com>
Date: Tue, 27 Aug 2019 23:18:53 +0200
> Now that 'TCQ_F_CPUSTATS' bit can be cleared, depending on the value of
> 'TCQ_F_NOLOCK' bit in the parent qdisc, we can't assume anymore that
> per-cpu counters are there in the error path of skb_array_produce().
> Otherwise, the following splat can be seen:
...
> Fix this by testing the value of 'TCQ_F_CPUSTATS' bit in 'qdisc->flags',
> before dereferencing 'qdisc->cpu_qstats'.
>
> Fixes: 8a53e616de29 ("net: sched: when clearing NOLOCK, clear TCQ_F_CPUSTATS, too")
> CC: Paolo Abeni <pabeni@redhat.com>
> CC: Stefano Brivio <sbrivio@redhat.com>
> Reported-by: Li Shuang <shuali@redhat.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied and queued up for v5.2 -stable, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-28 22:58 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-27 21:18 [PATCH net] net/sched: pfifo_fast: fix wrong dereference in pfifo_fast_enqueue Davide Caratti
2019-08-28 6:31 ` Paolo Abeni
2019-08-28 22:58 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).