* [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
@ 2023-02-07 18:37 Kuniyuki Iwashima
2023-02-07 19:25 ` Eric Dumazet
0 siblings, 1 reply; 6+ messages in thread
From: Kuniyuki Iwashima @ 2023-02-07 18:37 UTC (permalink / raw)
To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Matthieu Baerts, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev,
syzbot, Christoph Paasch
In commit b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in
sk->sk_prot->destroy()."), we delay freeing some IPv6 resources
from sk->destroy() to sk->sk_destruct().
Christoph Paasch reported the commit started triggering
WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues()
(See [0 - 2]).
For example, if inet6_sk(sk)->rxopt is not zero by setting
IPV6_RECVPKTINFO or its friends, tcp_v6_do_rcv() clones a skb
and calls skb_set_owner_r(), which charges it to sk. The skb
has not been uncharged in inet_csk_destroy_sock(), thus, calling
sk_stream_kill_queues() there triggers the WARN_ON_ONCE().
The same check has been in inet_sock_destruct() from at least
v2.6. Since only CAIF is not calling inet_sock_destruct() among
the users of sk_stream_kill_queues(), we remove the WARN_ON_ONCE()
from sk_stream_kill_queues() and add it to caif_sock_destructor().
[0]: https://lore.kernel.org/netdev/39725AB4-88F1-41B3-B07F-949C5CAEFF4F@icloud.com/
[1]: https://github.com/multipath-tcp/mptcp_net-next/issues/341
[2]:
WARNING: CPU: 0 PID: 3232 at net/core/stream.c:212 sk_stream_kill_queues+0x2f9/0x3e0
Modules linked in:
CPU: 0 PID: 3232 Comm: syz-executor.0 Not tainted 6.2.0-rc5ab24eb4698afbe147b424149c529e2a43ec24eb5 #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
RIP: 0010:sk_stream_kill_queues+0x2f9/0x3e0
Code: 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e ec 00 00 00 8b ab 08 01 00 00 e9 60 ff ff ff e8 d0 5f b6 fe 0f 0b eb 97 e8 c7 5f b6 fe <0f> 0b eb a0 e8 be 5f b6 fe 0f 0b e9 6a fe ff ff e8 02 07 e3 fe e9
RSP: 0018:ffff88810570fc68 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
RDX: ffff888101f38f40 RSI: ffffffff8285e529 RDI: 0000000000000005
RBP: 0000000000000ce0 R08: 0000000000000005 R09: 0000000000000000
R10: 0000000000000ce0 R11: 0000000000000001 R12: ffff8881009e9488
R13: ffffffff84af2cc0 R14: 0000000000000000 R15: ffff8881009e9458
FS: 00007f7fdfbd5800(0000) GS:ffff88811b600000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000001b32923000 CR3: 00000001062fc006 CR4: 0000000000170ef0
Call Trace:
<TASK>
inet_csk_destroy_sock+0x1a1/0x320
__tcp_close+0xab6/0xe90
tcp_close+0x30/0xc0
inet_release+0xe9/0x1f0
inet6_release+0x4c/0x70
__sock_release+0xd2/0x280
sock_close+0x15/0x20
__fput+0x252/0xa20
task_work_run+0x169/0x250
exit_to_user_mode_prepare+0x113/0x120
syscall_exit_to_user_mode+0x1d/0x40
do_syscall_64+0x48/0x90
entry_SYSCALL_64_after_hwframe+0x72/0xdc
RIP: 0033:0x7f7fdf7ae28d
Code: c1 20 00 00 75 10 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee fb ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 37 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
RSP: 002b:00000000007dfbb0 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
RAX: 0000000000000000 RBX: 0000000000000004 RCX: 00007f7fdf7ae28d
RDX: 0000000000000000 RSI: ffffffffffffffff RDI: 0000000000000003
RBP: 0000000000000000 R08: 000000007f338e0f R09: 0000000000000e0f
R10: 000000007f338e13 R11: 0000000000000293 R12: 00007f7fdefff000
R13: 00007f7fdefffcd8 R14: 00007f7fdefffce0 R15: 00007f7fdefffcd8
</TASK>
Fixes: b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in sk->sk_prot->destroy().")
Reported-by: syzbot <syzkaller@googlegroups.com>
Reported-by: Christoph Paasch <christophpaasch@icloud.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
net/caif/caif_socket.c | 1 +
net/core/stream.c | 1 -
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index 748be7253248..78c9729a6057 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -1015,6 +1015,7 @@ static void caif_sock_destructor(struct sock *sk)
return;
}
sk_stream_kill_queues(&cf_sk->sk);
+ WARN_ON_ONCE(sk->sk_forward_alloc);
caif_free_client(&cf_sk->layer);
}
diff --git a/net/core/stream.c b/net/core/stream.c
index cd06750dd329..434446ab14c5 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -209,7 +209,6 @@ void sk_stream_kill_queues(struct sock *sk)
sk_mem_reclaim_final(sk);
WARN_ON_ONCE(sk->sk_wmem_queued);
- WARN_ON_ONCE(sk->sk_forward_alloc);
/* It is _impossible_ for the backlog to contain anything
* when we get here. All user references to this socket
--
2.30.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
2023-02-07 18:37 [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues() Kuniyuki Iwashima
@ 2023-02-07 19:25 ` Eric Dumazet
2023-02-07 21:37 ` Kuniyuki Iwashima
2023-02-08 0:37 ` Kuniyuki Iwashima
0 siblings, 2 replies; 6+ messages in thread
From: Eric Dumazet @ 2023-02-07 19:25 UTC (permalink / raw)
To: Kuniyuki Iwashima
Cc: David S. Miller, Jakub Kicinski, Paolo Abeni, Matthieu Baerts,
Kuniyuki Iwashima, netdev, syzbot, Christoph Paasch
On Tue, Feb 7, 2023 at 7:37 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> In commit b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in
> sk->sk_prot->destroy()."), we delay freeing some IPv6 resources
> from sk->destroy() to sk->sk_destruct().
>
> Christoph Paasch reported the commit started triggering
> WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues()
> (See [0 - 2]).
>
> For example, if inet6_sk(sk)->rxopt is not zero by setting
> IPV6_RECVPKTINFO or its friends, tcp_v6_do_rcv() clones a skb
> and calls skb_set_owner_r(), which charges it to sk.
skb_set_owner_r() in this place seems wrong.
This could lead to a negative sk->sk_forward_alloc
(because we have not sk_rmem_schedule() it ?)
Do you have a repro ?
The skb
> has not been uncharged in inet_csk_destroy_sock(), thus, calling
> sk_stream_kill_queues() there triggers the WARN_ON_ONCE().
>
> The same check has been in inet_sock_destruct() from at least
> v2.6. Since only CAIF is not calling inet_sock_destruct() among
> the users of sk_stream_kill_queues(), we remove the WARN_ON_ONCE()
> from sk_stream_kill_queues() and add it to caif_sock_destructor().
>
> [0]: https://lore.kernel.org/netdev/39725AB4-88F1-41B3-B07F-949C5CAEFF4F@icloud.com/
> [1]: https://github.com/multipath-tcp/mptcp_net-next/issues/341
> [2]:
> WARNING: CPU: 0 PID: 3232 at net/core/stream.c:212 sk_stream_kill_queues+0x2f9/0x3e0
> Modules linked in:
> CPU: 0 PID: 3232 Comm: syz-executor.0 Not tainted 6.2.0-rc5ab24eb4698afbe147b424149c529e2a43ec24eb5 #2
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> RIP: 0010:sk_stream_kill_queues+0x2f9/0x3e0
> Code: 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e ec 00 00 00 8b ab 08 01 00 00 e9 60 ff ff ff e8 d0 5f b6 fe 0f 0b eb 97 e8 c7 5f b6 fe <0f> 0b eb a0 e8 be 5f b6 fe 0f 0b e9 6a fe ff ff e8 02 07 e3 fe e9
> RSP: 0018:ffff88810570fc68 EFLAGS: 00010293
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: ffff888101f38f40 RSI: ffffffff8285e529 RDI: 0000000000000005
> RBP: 0000000000000ce0 R08: 0000000000000005 R09: 0000000000000000
> R10: 0000000000000ce0 R11: 0000000000000001 R12: ffff8881009e9488
> R13: ffffffff84af2cc0 R14: 0000000000000000 R15: ffff8881009e9458
> FS: 00007f7fdfbd5800(0000) GS:ffff88811b600000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000001b32923000 CR3: 00000001062fc006 CR4: 0000000000170ef0
> Call Trace:
> <TASK>
> inet_csk_destroy_sock+0x1a1/0x320
> __tcp_close+0xab6/0xe90
> tcp_close+0x30/0xc0
> inet_release+0xe9/0x1f0
> inet6_release+0x4c/0x70
> __sock_release+0xd2/0x280
> sock_close+0x15/0x20
> __fput+0x252/0xa20
> task_work_run+0x169/0x250
> exit_to_user_mode_prepare+0x113/0x120
> syscall_exit_to_user_mode+0x1d/0x40
> do_syscall_64+0x48/0x90
> entry_SYSCALL_64_after_hwframe+0x72/0xdc
> RIP: 0033:0x7f7fdf7ae28d
> Code: c1 20 00 00 75 10 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee fb ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 37 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
> RSP: 002b:00000000007dfbb0 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
> RAX: 0000000000000000 RBX: 0000000000000004 RCX: 00007f7fdf7ae28d
> RDX: 0000000000000000 RSI: ffffffffffffffff RDI: 0000000000000003
> RBP: 0000000000000000 R08: 000000007f338e0f R09: 0000000000000e0f
> R10: 000000007f338e13 R11: 0000000000000293 R12: 00007f7fdefff000
> R13: 00007f7fdefffcd8 R14: 00007f7fdefffce0 R15: 00007f7fdefffcd8
> </TASK>
>
> Fixes: b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in sk->sk_prot->destroy().")
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Reported-by: Christoph Paasch <christophpaasch@icloud.com>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> net/caif/caif_socket.c | 1 +
> net/core/stream.c | 1 -
> 2 files changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
> index 748be7253248..78c9729a6057 100644
> --- a/net/caif/caif_socket.c
> +++ b/net/caif/caif_socket.c
> @@ -1015,6 +1015,7 @@ static void caif_sock_destructor(struct sock *sk)
> return;
> }
> sk_stream_kill_queues(&cf_sk->sk);
> + WARN_ON_ONCE(sk->sk_forward_alloc);
> caif_free_client(&cf_sk->layer);
> }
>
> diff --git a/net/core/stream.c b/net/core/stream.c
> index cd06750dd329..434446ab14c5 100644
> --- a/net/core/stream.c
> +++ b/net/core/stream.c
> @@ -209,7 +209,6 @@ void sk_stream_kill_queues(struct sock *sk)
> sk_mem_reclaim_final(sk);
>
> WARN_ON_ONCE(sk->sk_wmem_queued);
> - WARN_ON_ONCE(sk->sk_forward_alloc);
>
> /* It is _impossible_ for the backlog to contain anything
> * when we get here. All user references to this socket
> --
> 2.30.2
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
2023-02-07 19:25 ` Eric Dumazet
@ 2023-02-07 21:37 ` Kuniyuki Iwashima
2023-02-08 0:37 ` Kuniyuki Iwashima
1 sibling, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2023-02-07 21:37 UTC (permalink / raw)
To: edumazet
Cc: christophpaasch, davem, kuba, kuni1840, kuniyu, matthieu.baerts,
netdev, pabeni, syzkaller
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 7 Feb 2023 20:25:19 +0100
> On Tue, Feb 7, 2023 at 7:37 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> >
> > In commit b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in
> > sk->sk_prot->destroy()."), we delay freeing some IPv6 resources
> > from sk->destroy() to sk->sk_destruct().
> >
> > Christoph Paasch reported the commit started triggering
> > WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues()
> > (See [0 - 2]).
> >
> > For example, if inet6_sk(sk)->rxopt is not zero by setting
> > IPV6_RECVPKTINFO or its friends, tcp_v6_do_rcv() clones a skb
> > and calls skb_set_owner_r(), which charges it to sk.
>
> skb_set_owner_r() in this place seems wrong.
> This could lead to a negative sk->sk_forward_alloc
> (because we have not sk_rmem_schedule() it ?)
>
> Do you have a repro ?
Christoph attached a repro on GitHub [0], but I couldn't reproduce it,
so I also thought the WARN_ON_ONCE() was triggered by a negative value.
[0]: https://github.com/multipath-tcp/mptcp_net-next/issues/341#issuecomment-1410818557
>
> The skb
> > has not been uncharged in inet_csk_destroy_sock(), thus, calling
> > sk_stream_kill_queues() there triggers the WARN_ON_ONCE().
> >
> > The same check has been in inet_sock_destruct() from at least
> > v2.6. Since only CAIF is not calling inet_sock_destruct() among
> > the users of sk_stream_kill_queues(), we remove the WARN_ON_ONCE()
> > from sk_stream_kill_queues() and add it to caif_sock_destructor().
> >
> > [0]: https://lore.kernel.org/netdev/39725AB4-88F1-41B3-B07F-949C5CAEFF4F@icloud.com/
> > [1]: https://github.com/multipath-tcp/mptcp_net-next/issues/341
> > [2]:
> > WARNING: CPU: 0 PID: 3232 at net/core/stream.c:212 sk_stream_kill_queues+0x2f9/0x3e0
> > Modules linked in:
> > CPU: 0 PID: 3232 Comm: syz-executor.0 Not tainted 6.2.0-rc5ab24eb4698afbe147b424149c529e2a43ec24eb5 #2
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> > RIP: 0010:sk_stream_kill_queues+0x2f9/0x3e0
> > Code: 03 0f b6 04 02 84 c0 74 08 3c 03 0f 8e ec 00 00 00 8b ab 08 01 00 00 e9 60 ff ff ff e8 d0 5f b6 fe 0f 0b eb 97 e8 c7 5f b6 fe <0f> 0b eb a0 e8 be 5f b6 fe 0f 0b e9 6a fe ff ff e8 02 07 e3 fe e9
> > RSP: 0018:ffff88810570fc68 EFLAGS: 00010293
> > RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000000
> > RDX: ffff888101f38f40 RSI: ffffffff8285e529 RDI: 0000000000000005
> > RBP: 0000000000000ce0 R08: 0000000000000005 R09: 0000000000000000
> > R10: 0000000000000ce0 R11: 0000000000000001 R12: ffff8881009e9488
> > R13: ffffffff84af2cc0 R14: 0000000000000000 R15: ffff8881009e9458
> > FS: 00007f7fdfbd5800(0000) GS:ffff88811b600000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000001b32923000 CR3: 00000001062fc006 CR4: 0000000000170ef0
> > Call Trace:
> > <TASK>
> > inet_csk_destroy_sock+0x1a1/0x320
> > __tcp_close+0xab6/0xe90
> > tcp_close+0x30/0xc0
> > inet_release+0xe9/0x1f0
> > inet6_release+0x4c/0x70
> > __sock_release+0xd2/0x280
> > sock_close+0x15/0x20
> > __fput+0x252/0xa20
> > task_work_run+0x169/0x250
> > exit_to_user_mode_prepare+0x113/0x120
> > syscall_exit_to_user_mode+0x1d/0x40
> > do_syscall_64+0x48/0x90
> > entry_SYSCALL_64_after_hwframe+0x72/0xdc
> > RIP: 0033:0x7f7fdf7ae28d
> > Code: c1 20 00 00 75 10 b8 03 00 00 00 0f 05 48 3d 01 f0 ff ff 73 31 c3 48 83 ec 08 e8 ee fb ff ff 48 89 04 24 b8 03 00 00 00 0f 05 <48> 8b 3c 24 48 89 c2 e8 37 fc ff ff 48 89 d0 48 83 c4 08 48 3d 01
> > RSP: 002b:00000000007dfbb0 EFLAGS: 00000293 ORIG_RAX: 0000000000000003
> > RAX: 0000000000000000 RBX: 0000000000000004 RCX: 00007f7fdf7ae28d
> > RDX: 0000000000000000 RSI: ffffffffffffffff RDI: 0000000000000003
> > RBP: 0000000000000000 R08: 000000007f338e0f R09: 0000000000000e0f
> > R10: 000000007f338e13 R11: 0000000000000293 R12: 00007f7fdefff000
> > R13: 00007f7fdefffcd8 R14: 00007f7fdefffce0 R15: 00007f7fdefffcd8
> > </TASK>
> >
> > Fixes: b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in sk->sk_prot->destroy().")
> > Reported-by: syzbot <syzkaller@googlegroups.com>
> > Reported-by: Christoph Paasch <christophpaasch@icloud.com>
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> > net/caif/caif_socket.c | 1 +
> > net/core/stream.c | 1 -
> > 2 files changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
> > index 748be7253248..78c9729a6057 100644
> > --- a/net/caif/caif_socket.c
> > +++ b/net/caif/caif_socket.c
> > @@ -1015,6 +1015,7 @@ static void caif_sock_destructor(struct sock *sk)
> > return;
> > }
> > sk_stream_kill_queues(&cf_sk->sk);
> > + WARN_ON_ONCE(sk->sk_forward_alloc);
> > caif_free_client(&cf_sk->layer);
> > }
> >
> > diff --git a/net/core/stream.c b/net/core/stream.c
> > index cd06750dd329..434446ab14c5 100644
> > --- a/net/core/stream.c
> > +++ b/net/core/stream.c
> > @@ -209,7 +209,6 @@ void sk_stream_kill_queues(struct sock *sk)
> > sk_mem_reclaim_final(sk);
> >
> > WARN_ON_ONCE(sk->sk_wmem_queued);
> > - WARN_ON_ONCE(sk->sk_forward_alloc);
> >
> > /* It is _impossible_ for the backlog to contain anything
> > * when we get here. All user references to this socket
> > --
> > 2.30.2
> >
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
2023-02-07 19:25 ` Eric Dumazet
2023-02-07 21:37 ` Kuniyuki Iwashima
@ 2023-02-08 0:37 ` Kuniyuki Iwashima
2023-02-08 0:42 ` Kuniyuki Iwashima
2023-02-08 1:01 ` Kuniyuki Iwashima
1 sibling, 2 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2023-02-08 0:37 UTC (permalink / raw)
To: edumazet
Cc: christophpaasch, davem, kuba, kuni1840, kuniyu, matthieu.baerts,
netdev, pabeni, syzkaller
From: Eric Dumazet <edumazet@google.com>
Date: Tue, 7 Feb 2023 20:25:19 +0100
> On Tue, Feb 7, 2023 at 7:37 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> >
> > In commit b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in
> > sk->sk_prot->destroy()."), we delay freeing some IPv6 resources
> > from sk->destroy() to sk->sk_destruct().
> >
> > Christoph Paasch reported the commit started triggering
> > WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues()
> > (See [0 - 2]).
> >
> > For example, if inet6_sk(sk)->rxopt is not zero by setting
> > IPV6_RECVPKTINFO or its friends, tcp_v6_do_rcv() clones a skb
> > and calls skb_set_owner_r(), which charges it to sk.
>
> skb_set_owner_r() in this place seems wrong.
> This could lead to a negative sk->sk_forward_alloc
> (because we have not sk_rmem_schedule() it ?)
>
> Do you have a repro ?
I created a repro and confirmed sk->sk_forward_alloc was always positive.
---8<---
#include <stdio.h>
#include <sys/socket.h>
#include <netinet/in.h>
#define IPV6_FLOWINFO 11
int main(void)
{
struct sockaddr_in6 addr = {
.sin6_family = AF_INET6,
.sin6_addr = in6addr_any,
.sin6_port = htons(0),
};
int fd, ret = 0;
socklen_t len;
ret = socket(AF_INET6, SOCK_STREAM, IPPROTO_IP);
perror("socket");
if (ret < 0)
return ret;
fd = ret;
ret = setsockopt(fd, SOL_IPV6, IPV6_FLOWINFO, &(int){1}, sizeof(int));
perror("setsockopt");
ret = bind(fd, (struct sockaddr *)&addr, sizeof(addr));
perror("bind");
if (ret)
goto out;
len = sizeof(addr);
ret = getsockname(fd, (struct sockaddr *)&addr, &len);
perror("getsockname");
if (ret)
goto out;
ret = connect(fd, (struct sockaddr *)&addr, len);
perror("connect");
if (ret)
goto out;
out:
close(fd);
return ret;
}
---8<---
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
2023-02-08 0:37 ` Kuniyuki Iwashima
@ 2023-02-08 0:42 ` Kuniyuki Iwashima
2023-02-08 1:01 ` Kuniyuki Iwashima
1 sibling, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2023-02-08 0:42 UTC (permalink / raw)
To: kuniyu
Cc: christophpaasch, davem, edumazet, kuba, kuni1840,
matthieu.baerts, netdev, pabeni, syzkaller
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Tue, 7 Feb 2023 16:37:13 -0800
> From: Eric Dumazet <edumazet@google.com>
> Date: Tue, 7 Feb 2023 20:25:19 +0100
> > On Tue, Feb 7, 2023 at 7:37 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> > >
> > > In commit b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in
> > > sk->sk_prot->destroy()."), we delay freeing some IPv6 resources
> > > from sk->destroy() to sk->sk_destruct().
> > >
> > > Christoph Paasch reported the commit started triggering
> > > WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues()
> > > (See [0 - 2]).
> > >
> > > For example, if inet6_sk(sk)->rxopt is not zero by setting
> > > IPV6_RECVPKTINFO or its friends, tcp_v6_do_rcv() clones a skb
> > > and calls skb_set_owner_r(), which charges it to sk.
> >
> > skb_set_owner_r() in this place seems wrong.
> > This could lead to a negative sk->sk_forward_alloc
> > (because we have not sk_rmem_schedule() it ?)
> >
> > Do you have a repro ?
>
> I created a repro and confirmed sk->sk_forward_alloc was always positive.
>
> ---8<---
Sorry, I missed unistd.h here while copy-and-paste.
#include <unistd.h>
> #include <stdio.h>
>
> #include <sys/socket.h>
> #include <netinet/in.h>
>
> #define IPV6_FLOWINFO 11
>
> int main(void)
> {
> struct sockaddr_in6 addr = {
> .sin6_family = AF_INET6,
> .sin6_addr = in6addr_any,
> .sin6_port = htons(0),
> };
> int fd, ret = 0;
> socklen_t len;
>
> ret = socket(AF_INET6, SOCK_STREAM, IPPROTO_IP);
> perror("socket");
> if (ret < 0)
> return ret;
>
> fd = ret;
> ret = setsockopt(fd, SOL_IPV6, IPV6_FLOWINFO, &(int){1}, sizeof(int));
> perror("setsockopt");
>
> ret = bind(fd, (struct sockaddr *)&addr, sizeof(addr));
> perror("bind");
> if (ret)
> goto out;
>
> len = sizeof(addr);
> ret = getsockname(fd, (struct sockaddr *)&addr, &len);
> perror("getsockname");
> if (ret)
> goto out;
>
> ret = connect(fd, (struct sockaddr *)&addr, len);
> perror("connect");
> if (ret)
> goto out;
>
> out:
> close(fd);
>
> return ret;
> }
> ---8<---
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
2023-02-08 0:37 ` Kuniyuki Iwashima
2023-02-08 0:42 ` Kuniyuki Iwashima
@ 2023-02-08 1:01 ` Kuniyuki Iwashima
1 sibling, 0 replies; 6+ messages in thread
From: Kuniyuki Iwashima @ 2023-02-08 1:01 UTC (permalink / raw)
To: kuniyu
Cc: christophpaasch, davem, edumazet, kuba, kuni1840,
matthieu.baerts, netdev, pabeni, syzkaller
From: Kuniyuki Iwashima <kuniyu@amazon.com>
Date: Tue, 7 Feb 2023 16:37:13 -0800
> From: Eric Dumazet <edumazet@google.com>
> Date: Tue, 7 Feb 2023 20:25:19 +0100
> > On Tue, Feb 7, 2023 at 7:37 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> > >
> > > In commit b5fc29233d28 ("inet6: Remove inet6_destroy_sock() in
> > > sk->sk_prot->destroy()."), we delay freeing some IPv6 resources
> > > from sk->destroy() to sk->sk_destruct().
> > >
> > > Christoph Paasch reported the commit started triggering
> > > WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues()
> > > (See [0 - 2]).
> > >
> > > For example, if inet6_sk(sk)->rxopt is not zero by setting
> > > IPV6_RECVPKTINFO or its friends, tcp_v6_do_rcv() clones a skb
> > > and calls skb_set_owner_r(), which charges it to sk.
> >
> > skb_set_owner_r() in this place seems wrong.
> > This could lead to a negative sk->sk_forward_alloc
> > (because we have not sk_rmem_schedule() it ?)
> >
> > Do you have a repro ?
>
> I created a repro and confirmed sk->sk_forward_alloc was always positive.
This was just before sk_stream_kill_queues(), and actually
sk->sk_forward_alloc was able to be negative by the
skb_set_owner_r() as you thought.
I'll fix this, thank you!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-02-08 1:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-07 18:37 [PATCH v1 net] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues() Kuniyuki Iwashima
2023-02-07 19:25 ` Eric Dumazet
2023-02-07 21:37 ` Kuniyuki Iwashima
2023-02-08 0:37 ` Kuniyuki Iwashima
2023-02-08 0:42 ` Kuniyuki Iwashima
2023-02-08 1:01 ` Kuniyuki Iwashima
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).