* crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()
@ 2018-10-16 4:13 Maciej Żenczykowski
2018-10-16 8:11 ` Florian Westphal
2018-10-23 14:47 ` [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments Florian Westphal
0 siblings, 2 replies; 10+ messages in thread
From: Maciej Żenczykowski @ 2018-10-16 4:13 UTC (permalink / raw)
To: Lorenzo Colitti, Eric Dumazet, Florian Westphal, Linux NetDev,
Maciej Zenczykowski, Maciej Żenczykowski
I believe that:
commit ad8b1ffc3efae2f65080bdb11145c87d299b8f9a
Author: Florian Westphal <fw@strlen.de>
netfilter: ipv6: nf_defrag: drop skb dst before queueing
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -618,6 +618,8 @@ int nf_ct_frag6_gather(struct net *net, struct
sk_buff *skb, u32 user)
fq->q.meat == fq->q.len &&
nf_ct_frag6_reasm(fq, skb, dev))
ret = 0;
+ else
+ skb_dst_drop(skb);
out_unlock:
spin_unlock_bh(&fq->q.lock);
Is causing a crash on android after upgrading from 4.9.96 to 4.9.119
This is because clatd ipv4 to ipv6 translation user space daemon is
functionally equivalent to the syzkaller reproducer.
It will convert ipv4 frags it receives via tap into ipv6 frags which
it will write out via rawv6 sendmsg.
However we are also using xt_policy, after stripping cruft this is basically:
ip6tables -A OUTPUT -m policy --dir out --pol ipsec
Crash is:
match_policy_out()
const struct dst_entry *dst = skb_dst(skb); // returns NULL
if (dst->xfrm == NULL) <-- dst == NULL -> panic
[ 1136.606948] c1 2675 [<ffffff9ec38b4098>] policy_mt+0x34/0x18c
[ 1136.606954] c1 2675 [<ffffff9ec39e6af8>] ip6t_do_table+0x280/0x684
[ 1136.606961] c1 2675 [<ffffff9ec39e7250>] ip6table_filter_hook+0x20/0x28
[ 1136.606969] c1 2675 [<ffffff9ec386ecc8>] nf_hook_slow+0x98/0x154
[ 1136.606977] c1 2675 [<ffffff9ec39b9b10>] rawv6_sendmsg+0xd14/0x1520
[ 1136.606985] c1 2675 [<ffffff9ec39191fc>] inet_sendmsg+0x100/0x1b0
[ 1136.606993] c1 2675 [<ffffff9ec37d3720>] ___sys_sendmsg+0x2a0/0x414
[ 1136.606999] c1 2675 [<ffffff9ec37d3d48>] SyS_sendmsg+0x94/0xe4
Just checking for NULL in xt_policy.c:match_policy_out() and returning
0 or 1 unconditionally seems to be the wrong thing to do,
since after all prior to skb_dst_drop() the skb->dst->xfrm might not
have been NULL.
Maciej Żenczykowski, Kernel Networking Developer @ Google
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()
2018-10-16 4:13 crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather() Maciej Żenczykowski
@ 2018-10-16 8:11 ` Florian Westphal
2018-10-16 9:40 ` Maciej Żenczykowski
2018-10-23 14:47 ` [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments Florian Westphal
1 sibling, 1 reply; 10+ messages in thread
From: Florian Westphal @ 2018-10-16 8:11 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: Lorenzo Colitti, Eric Dumazet, Florian Westphal, Linux NetDev,
Maciej Zenczykowski
Maciej Żenczykowski <zenczykowski@gmail.com> wrote:
I am currently travelling and not able to investigate
until next week.
> commit ad8b1ffc3efae2f65080bdb11145c87d299b8f9a
> Author: Florian Westphal <fw@strlen.de>
> netfilter: ipv6: nf_defrag: drop skb dst before queueing
>
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -618,6 +618,8 @@ int nf_ct_frag6_gather(struct net *net, struct
> sk_buff *skb, u32 user)
> fq->q.meat == fq->q.len &&
> nf_ct_frag6_reasm(fq, skb, dev))
> ret = 0;
> + else
> + skb_dst_drop(skb);
This is only supposed to drop dst of skbs that are enqueued,
i.e. frag6_gather returns NF_STOLEN.
In case skb completes the queue, then that skbs dst_entry
is supposed to be kept, so skb_dst() does NOT return NULL.
Its not supposed to be any different than ipv4 defrag.
> const struct dst_entry *dst = skb_dst(skb); // returns NULL
That is not supposed to happen.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()
2018-10-16 8:11 ` Florian Westphal
@ 2018-10-16 9:40 ` Maciej Żenczykowski
2018-10-16 9:41 ` Maciej Żenczykowski
0 siblings, 1 reply; 10+ messages in thread
From: Maciej Żenczykowski @ 2018-10-16 9:40 UTC (permalink / raw)
To: Florian Westphal; +Cc: Lorenzo Colitti, Eric Dumazet, Linux NetDev
> That is not supposed to happen.
# uname -a
Linux (none) 4.9.119 #3 Tue Oct 16 02:34:36 PDT 2018 x86_64 GNU/Linux
root@(none)# ip6tables -A OUTPUT -m policy --dir out --pol ipsec
root@(none)# python -c "import os, socket;
ip='00000000000000000000000000000001';
x='6001234504d82c40'+ip+ip+'3a000001a1224d20' + 'ff'*(1280-40-8);
y='6001234500092c40'+ip+ip+'3a0004d0a1224d20' + 'ff';
s=socket.socket(socket.AF_INET6,socket.SOCK_RAW,socket.IPPROTO_RAW);
s.sendto(x.decode('hex'),('::1',0,0,1));
s.sendto(y.decode('hex'),('::1',0,0,1));"
Modules linked in:
Pid: 297, comm: python Not tainted 4.9.119
RIP: 0033:[<0000000060272eca>]
RSP: 00000000802afa10 EFLAGS: 00010246
RAX: 0000000060492fa8 RBX: 0000000060272c6f RCX: 00000000803a12a8
RDX: 00000000803a1288 RSI: 00000000802afa98 RDI: 0000000080314d00
RBP: 00000000802afa40 R08: 0000000000000001 R09: 0100000000000000
R10: 0000000000000000 R11: 00000000803a12a8 R12: 0000000000010002
R13: 000000000000000a R14: 0000000000000000 R15: 0000000000000000
Kernel panic - not syncing: Kernel mode fault at addr 0x48, ip 0x60272eca
CPU: 0 PID: 297 Comm: python Not tainted 4.9.119 #3
Stack:
800d5000 803a11e0 80314d00 803a1000
00000000 00000000 802afb00 6031afe1
00000000 803a1288 803a100c 100000003
Call Trace:
[<6031afe1>] ip6t_do_table+0x2a3/0x3d4
[<6026d440>] ? netfilter_net_init+0xbe/0x14f
[<6026d4d1>] ? nf_iterate+0x0/0x5c
[<6031cca5>] ip6table_filter_hook+0x21/0x23
[<6026d509>] nf_iterate+0x38/0x5c
[<6026d561>] nf_hook_slow+0x34/0xa2
[<6003166c>] ? set_signals+0x0/0x3f
[<6003165d>] ? get_signals+0x0/0xf
[<603048b0>] rawv6_sendmsg+0x842/0xc4b
[<60033d15>] ? wait_stub_done+0x40/0x10a
[<60021176>] ? copy_chunk_from_user+0x23/0x2e
[<60021153>] ? copy_chunk_from_user+0x0/0x2e
[<6030307f>] ? dst_output+0x0/0x11
[<602b0926>] inet_sendmsg+0x1e/0x5c
[<600fe15f>] ? __fdget+0x15/0x17
[<602264b9>] sock_sendmsg+0xf/0x62
[<602279aa>] SyS_sendto+0x108/0x140
[<600389c2>] ? arch_switch_to+0x2b/0x2e
[<60367ff4>] ? __schedule+0x428/0x44f
[<60367bcc>] ? __schedule+0x0/0x44f
[<60021125>] handle_syscall+0x79/0xa7
[<6003445c>] userspace+0x3bb/0x453
[<6001dd92>] ? interrupt_end+0x0/0x94
[<6001dc42>] fork_handler+0x85/0x87
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()
2018-10-16 9:40 ` Maciej Żenczykowski
@ 2018-10-16 9:41 ` Maciej Żenczykowski
2018-10-16 9:49 ` Maciej Żenczykowski
0 siblings, 1 reply; 10+ messages in thread
From: Maciej Żenczykowski @ 2018-10-16 9:41 UTC (permalink / raw)
To: Florian Westphal; +Cc: Lorenzo Colitti, Eric Dumazet, Linux NetDev
(and v4.9.133 latest 4.9 LTS fails the same way, but curiously 4.19-rc8 doesn't)
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather()
2018-10-16 9:41 ` Maciej Żenczykowski
@ 2018-10-16 9:49 ` Maciej Żenczykowski
0 siblings, 0 replies; 10+ messages in thread
From: Maciej Żenczykowski @ 2018-10-16 9:49 UTC (permalink / raw)
To: Florian Westphal; +Cc: Lorenzo Colitti, Eric Dumazet, Linux NetDev
4.19-rc8 - pass
4.14.76 - pass
4.9.133 - fail
4.9.133 + revert of ad8b1ffc3efae2f65080bdb11145c87d299b8f9a - pass
On Tue, Oct 16, 2018 at 2:41 AM Maciej Żenczykowski
<zenczykowski@gmail.com> wrote:
>
> (and v4.9.133 latest 4.9 LTS fails the same way, but curiously 4.19-rc8 doesn't)
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments
2018-10-16 4:13 crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather() Maciej Żenczykowski
2018-10-16 8:11 ` Florian Westphal
@ 2018-10-23 14:47 ` Florian Westphal
2018-10-23 14:54 ` Eric Dumazet
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Florian Westphal @ 2018-10-23 14:47 UTC (permalink / raw)
To: netfilter-devel
Cc: lorenzo, zenczykowski, edumazet, netdev, maze, Florian Westphal
Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did
not save/restore skb->dst.
This causes oops when handling locally generated ipv6 fragments, as
output path needs a valid dst.
Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
Fixes: 84379c9afe01 ("netfilter: ipv6: nf_defrag: drop skb dst before queueing")
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/ipv6/netfilter/nf_conntrack_reasm.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index 8f68a518d9db..f76bd4d15704 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -587,11 +587,16 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
*/
ret = -EINPROGRESS;
if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
- fq->q.meat == fq->q.len &&
- nf_ct_frag6_reasm(fq, skb, dev))
- ret = 0;
- else
+ fq->q.meat == fq->q.len) {
+ unsigned long orefdst = skb->_skb_refdst;
+
+ skb->_skb_refdst = 0UL;
+ if (nf_ct_frag6_reasm(fq, skb, dev))
+ ret = 0;
+ skb->_skb_refdst = orefdst;
+ } else {
skb_dst_drop(skb);
+ }
out_unlock:
spin_unlock_bh(&fq->q.lock);
--
2.18.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments
2018-10-23 14:47 ` [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments Florian Westphal
@ 2018-10-23 14:54 ` Eric Dumazet
2018-10-23 21:04 ` Maciej Żenczykowski
2018-10-25 8:18 ` Pablo Neira Ayuso
2 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2018-10-23 14:54 UTC (permalink / raw)
To: Florian Westphal
Cc: netfilter-devel, Lorenzo Colitti, zenczykowski, netdev,
Maciej Żenczykowski
On Tue, Oct 23, 2018 at 7:48 AM Florian Westphal <fw@strlen.de> wrote:
>
> Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did
> not save/restore skb->dst.
>
> This causes oops when handling locally generated ipv6 fragments, as
> output path needs a valid dst.
>
> Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
> Fixes: 84379c9afe01 ("netfilter: ipv6: nf_defrag: drop skb dst before queueing")
> Signed-off-by: Florian Westphal <fw@strlen.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments
2018-10-23 14:47 ` [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments Florian Westphal
2018-10-23 14:54 ` Eric Dumazet
@ 2018-10-23 21:04 ` Maciej Żenczykowski
2018-10-23 22:04 ` Florian Westphal
2018-10-25 8:18 ` Pablo Neira Ayuso
2 siblings, 1 reply; 10+ messages in thread
From: Maciej Żenczykowski @ 2018-10-23 21:04 UTC (permalink / raw)
To: Florian Westphal
Cc: netfilter-devel, Lorenzo Colitti, Eric Dumazet, Linux NetDev
On Tue, Oct 23, 2018 at 7:47 AM, Florian Westphal <fw@strlen.de> wrote:
> Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did
> not save/restore skb->dst.
>
> This causes oops when handling locally generated ipv6 fragments, as
> output path needs a valid dst.
>
> Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com>
> Fixes: 84379c9afe01 ("netfilter: ipv6: nf_defrag: drop skb dst before queueing")
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/ipv6/netfilter/nf_conntrack_reasm.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
> index 8f68a518d9db..f76bd4d15704 100644
> --- a/net/ipv6/netfilter/nf_conntrack_reasm.c
> +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
> @@ -587,11 +587,16 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
> */
> ret = -EINPROGRESS;
> if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> - fq->q.meat == fq->q.len &&
> - nf_ct_frag6_reasm(fq, skb, dev))
> - ret = 0;
> - else
> + fq->q.meat == fq->q.len) {
> + unsigned long orefdst = skb->_skb_refdst;
> +
> + skb->_skb_refdst = 0UL;
> + if (nf_ct_frag6_reasm(fq, skb, dev))
> + ret = 0;
> + skb->_skb_refdst = orefdst;
> + } else {
> skb_dst_drop(skb);
> + }
>
> out_unlock:
> spin_unlock_bh(&fq->q.lock);
> --
> 2.18.1
>
I don't quite follow how this fixes things, but I'll trust you on it.
(nor do I understand why only 4.9 LTS appears to crash with a null ptr deref)
Thanks for the fix.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments
2018-10-23 21:04 ` Maciej Żenczykowski
@ 2018-10-23 22:04 ` Florian Westphal
0 siblings, 0 replies; 10+ messages in thread
From: Florian Westphal @ 2018-10-23 22:04 UTC (permalink / raw)
To: Maciej Żenczykowski
Cc: Florian Westphal, netfilter-devel, Lorenzo Colitti, Eric Dumazet,
Linux NetDev
Maciej Żenczykowski <maze@google.com> wrote:
> > ret = -EINPROGRESS;
> > if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> > - fq->q.meat == fq->q.len &&
> > - nf_ct_frag6_reasm(fq, skb, dev))
> > - ret = 0;
> > - else
> > + fq->q.meat == fq->q.len) {
> > + unsigned long orefdst = skb->_skb_refdst;
> > +
> > + skb->_skb_refdst = 0UL;
> > + if (nf_ct_frag6_reasm(fq, skb, dev))
> > + ret = 0;
> > + skb->_skb_refdst = orefdst;
> > + } else {
> > skb_dst_drop(skb);
> > + }
> >
> > out_unlock:
> > spin_unlock_bh(&fq->q.lock);
> > --
> > 2.18.1
> >
>
> I don't quite follow how this fixes things, but I'll trust you on it.
The problematic spot is skb_morph() in nf_ct_frag6_reasm(), when we hit
this code path we take dst from a dst-less skb that got queued earlier.
> (nor do I understand why only 4.9 LTS appears to crash with a null ptr deref)
Newer kernels need nft or iptables rule that enables defrag, such as
"ip6tables -A INPUT -m conntrack --ctstate NEW"; 4.9 still enables it by
default.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments
2018-10-23 14:47 ` [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments Florian Westphal
2018-10-23 14:54 ` Eric Dumazet
2018-10-23 21:04 ` Maciej Żenczykowski
@ 2018-10-25 8:18 ` Pablo Neira Ayuso
2 siblings, 0 replies; 10+ messages in thread
From: Pablo Neira Ayuso @ 2018-10-25 8:18 UTC (permalink / raw)
To: Florian Westphal
Cc: netfilter-devel, lorenzo, zenczykowski, edumazet, netdev, maze
On Tue, Oct 23, 2018 at 04:47:16PM +0200, Florian Westphal wrote:
> Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did
> not save/restore skb->dst.
>
> This causes oops when handling locally generated ipv6 fragments, as
> output path needs a valid dst.
Applied, thanks!
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-10-25 16:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-16 4:13 crash in xt_policy due to skb_dst_drop() in nf_ct_frag6_gather() Maciej Żenczykowski
2018-10-16 8:11 ` Florian Westphal
2018-10-16 9:40 ` Maciej Żenczykowski
2018-10-16 9:41 ` Maciej Żenczykowski
2018-10-16 9:49 ` Maciej Żenczykowski
2018-10-23 14:47 ` [PATCH nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments Florian Westphal
2018-10-23 14:54 ` Eric Dumazet
2018-10-23 21:04 ` Maciej Żenczykowski
2018-10-23 22:04 ` Florian Westphal
2018-10-25 8:18 ` Pablo Neira Ayuso
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).