[PATCH v3 net 0/2] sk->sk_forward_alloc fixes.

[PATCH v3 net 0/2] sk->sk_forward_alloc fixes.

[Kuniyuki Iwashima]

The first patch fixes a negative sk_forward_alloc by adding
sk_rmem_schedule() before skb_set_owner_r(), and second patch
removes an unnecessary WARN_ON_ONCE().

Changes:
  v3:
    * Factorise a common pattern in Patch 1 as suggested by Eric

  v2: https://lore.kernel.org/netdev/20230209013329.87879-1-kuniyu@amazon.com/
    * Add the first patch

  v1: https://lore.kernel.org/netdev/20230207183718.54520-1-kuniyu@amazon.com/


Kuniyuki Iwashima (2):
  dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.
  net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from
    sk_stream_kill_queues().

 include/net/sock.h     | 13 +++++++++++++
 net/caif/caif_socket.c |  1 +
 net/core/stream.c      |  1 -
 net/dccp/ipv6.c        |  7 ++-----
 net/ipv6/tcp_ipv6.c    | 10 +++-------
 5 files changed, 19 insertions(+), 13 deletions(-)

-- 
2.30.2

[PATCH v3 net 1/2] dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.

[Kuniyuki Iwashima]

Eric Dumazet pointed out [0] that when we call skb_set_owner_r()
for ipv6_pinfo.pktoptions, sk_rmem_schedule() has not been called,
resulting in a negative sk_forward_alloc.

We add a new helper which clones a skb and sets its owner only
when sk_rmem_schedule() succeeds.

Note that we move skb_set_owner_r() forward in (dccp|tcp)_v6_do_rcv()
because tcp_send_synack() can make sk_forward_alloc negative before
ipv6_opt_accepted() in the crossed SYN-ACK or self-connect() cases.

[0]: https://lore.kernel.org/netdev/CANn89iK9oc20Jdi_41jb9URdF210r7d1Y-+uypbMSbOfY6jqrg@mail.gmail.com/

Fixes: 323fbd0edf3f ("net: dccp: Add handling of IPV6_PKTOPTIONS to dccp_v6_do_rcv()")
Fixes: 3df80d9320bc ("[DCCP]: Introduce DCCPv6")
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Andrii <tulup@mail.ru>
Cc: Arnaldo Carvalho de Melo <acme@mandriva.com>
---
 include/net/sock.h  | 13 +++++++++++++
 net/dccp/ipv6.c     |  7 ++-----
 net/ipv6/tcp_ipv6.c | 10 +++-------
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index dcd72e6285b2..556209727633 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2434,6 +2434,19 @@ static inline __must_check bool skb_set_owner_sk_safe(struct sk_buff *skb, struc
 	return false;
 }
 
+static inline struct sk_buff *skb_clone_and_charge_r(struct sk_buff *skb, struct sock *sk)
+{
+	skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC));
+	if (skb) {
+		if (sk_rmem_schedule(sk, skb, skb->truesize)) {
+			skb_set_owner_r(skb, sk);
+			return skb;
+		}
+		__kfree_skb(skb);
+	}
+	return NULL;
+}
+
 static inline void skb_prepare_for_gro(struct sk_buff *skb)
 {
 	if (skb->destructor != sock_wfree) {
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 4260fe466993..b9d7c3dd1cb3 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -551,11 +551,9 @@ static struct sock *dccp_v6_request_recv_sock(const struct sock *sk,
 	*own_req = inet_ehash_nolisten(newsk, req_to_sk(req_unhash), NULL);
 	/* Clone pktoptions received with SYN, if we own the req */
 	if (*own_req && ireq->pktopts) {
-		newnp->pktoptions = skb_clone(ireq->pktopts, GFP_ATOMIC);
+		newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
 		consume_skb(ireq->pktopts);
 		ireq->pktopts = NULL;
-		if (newnp->pktoptions)
-			skb_set_owner_r(newnp->pktoptions, newsk);
 	}
 
 	return newsk;
@@ -615,7 +613,7 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 					       --ANK (980728)
 	 */
 	if (np->rxopt.all)
-		opt_skb = skb_clone(skb, GFP_ATOMIC);
+		opt_skb = skb_clone_and_charge_r(skb, sk);
 
 	if (sk->sk_state == DCCP_OPEN) { /* Fast path */
 		if (dccp_rcv_established(sk, skb, dccp_hdr(skb), skb->len))
@@ -679,7 +677,6 @@ static int dccp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 			np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
 		if (ipv6_opt_accepted(sk, opt_skb,
 				      &DCCP_SKB_CB(opt_skb)->header.h6)) {
-			skb_set_owner_r(opt_skb, sk);
 			memmove(IP6CB(opt_skb),
 				&DCCP_SKB_CB(opt_skb)->header.h6,
 				sizeof(struct inet6_skb_parm));
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 11b736a76bd7..b681644547d0 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1387,14 +1387,11 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *
 
 		/* Clone pktoptions received with SYN, if we own the req */
 		if (ireq->pktopts) {
-			newnp->pktoptions = skb_clone(ireq->pktopts,
-						      sk_gfp_mask(sk, GFP_ATOMIC));
+			newnp->pktoptions = skb_clone_and_charge_r(ireq->pktopts, newsk);
 			consume_skb(ireq->pktopts);
 			ireq->pktopts = NULL;
-			if (newnp->pktoptions) {
+			if (newnp->pktoptions)
 				tcp_v6_restore_cb(newnp->pktoptions);
-				skb_set_owner_r(newnp->pktoptions, newsk);
-			}
 		}
 	} else {
 		if (!req_unhash && found_dup_sk) {
@@ -1466,7 +1463,7 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 					       --ANK (980728)
 	 */
 	if (np->rxopt.all)
-		opt_skb = skb_clone(skb, sk_gfp_mask(sk, GFP_ATOMIC));
+		opt_skb = skb_clone_and_charge_r(skb, sk);
 
 	reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	if (sk->sk_state == TCP_ESTABLISHED) { /* Fast path */
@@ -1552,7 +1549,6 @@ int tcp_v6_do_rcv(struct sock *sk, struct sk_buff *skb)
 		if (np->repflow)
 			np->flow_label = ip6_flowlabel(ipv6_hdr(opt_skb));
 		if (ipv6_opt_accepted(sk, opt_skb, &TCP_SKB_CB(opt_skb)->header.h6)) {
-			skb_set_owner_r(opt_skb, sk);
 			tcp_v6_restore_cb(opt_skb);
 			opt_skb = xchg(&np->pktoptions, opt_skb);
 		} else {
-- 
2.30.2

Re: [PATCH v3 net 1/2] dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.

[Eric Dumazet]

On Fri, Feb 10, 2023 at 1:22 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> Eric Dumazet pointed out [0] that when we call skb_set_owner_r()
> for ipv6_pinfo.pktoptions, sk_rmem_schedule() has not been called,
> resulting in a negative sk_forward_alloc.
>
> We add a new helper which clones a skb and sets its owner only
> when sk_rmem_schedule() succeeds.
>
> Note that we move skb_set_owner_r() forward in (dccp|tcp)_v6_do_rcv()
> because tcp_send_synack() can make sk_forward_alloc negative before
> ipv6_opt_accepted() in the crossed SYN-ACK or self-connect() cases.
>
> [0]: https://lore.kernel.org/netdev/CANn89iK9oc20Jdi_41jb9URdF210r7d1Y-+uypbMSbOfY6jqrg@mail.gmail.com/
>
> Fixes: 323fbd0edf3f ("net: dccp: Add handling of IPV6_PKTOPTIONS to dccp_v6_do_rcv()")
> Fixes: 3df80d9320bc ("[DCCP]: Introduce DCCPv6")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>


Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !

Re: [PATCH v3 net 1/2] dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.

[Hauke Mehrtens]

On 2/10/23 01:22, Kuniyuki Iwashima wrote:
> Eric Dumazet pointed out [0] that when we call skb_set_owner_r()
> for ipv6_pinfo.pktoptions, sk_rmem_schedule() has not been called,
> resulting in a negative sk_forward_alloc.
> 
> We add a new helper which clones a skb and sets its owner only
> when sk_rmem_schedule() succeeds.
> 
> Note that we move skb_set_owner_r() forward in (dccp|tcp)_v6_do_rcv()
> because tcp_send_synack() can make sk_forward_alloc negative before
> ipv6_opt_accepted() in the crossed SYN-ACK or self-connect() cases.
> 
> [0]: https://lore.kernel.org/netdev/CANn89iK9oc20Jdi_41jb9URdF210r7d1Y-+uypbMSbOfY6jqrg@mail.gmail.com/
> 
> Fixes: 323fbd0edf3f ("net: dccp: Add handling of IPV6_PKTOPTIONS to dccp_v6_do_rcv()")
> Fixes: 3df80d9320bc ("[DCCP]: Introduce DCCPv6")
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> Cc: Andrii <tulup@mail.ru>
> Cc: Arnaldo Carvalho de Melo <acme@mandriva.com>
> ---
>   include/net/sock.h  | 13 +++++++++++++
>   net/dccp/ipv6.c     |  7 ++-----
>   net/ipv6/tcp_ipv6.c | 10 +++-------
>   3 files changed, 18 insertions(+), 12 deletions(-)
> 
Hi,

Multiples people reported kernel warnings after the upgrade from kernel 
5.15.94 to 5.15.95 in OpenWrt master, see 
https://github.com/openwrt/openwrt/pull/12071

This was seen on a MIPS and a x86 CPU. It is happening when a Windows 
client connected, it works fine when an Android or iPad connects.

OpenWrt has some additional patches on top of the kernel, we haven't 
checked yet if they are causing this problem in combination with this 
new change. With kernel 5.15.94 as a base it works fine.

The problem is not showing up when the backport of this patch is reverted.

This warning was reported:

[  257.978586] ------------[ cut here ]------------
[  257.987882] WARNING: CPU: 0 PID: 4377 at net/core/stream.c:212 
inet_csk_destroy_sock+0x6c/0x17c
[  258.005287] Modules linked in: rt2800soc rt2800mmio rt2800lib pppoe 
ppp_async nft_fib_inet nf_flow_table_ipv6 nf_flow_table_ipv4 
nf_flow_table_inet rt2x00soc rt2x00mmio rt2x00lib pppox ppp_generic 
nft_reject_ipv6 nft_reject_ipv4 nft_reject_inet nft_reject nft_redir 
nft_quota nft_objref nft_numgen nft_nat nft_masq nft_log nft_limit 
nft_hash nft_flow_offload nft_fib_ipv6 nft_fib_ipv4 nft_fib nft_ct 
nft_counter nft_chain_nat nf_tables nf_nat nf_flow_table nf_conntrack 
mt76x2e mt76x2_common mt76x02_lib mt76 mac80211 lzo cfg80211 slhc 
nfnetlink nf_reject_ipv6 nf_reject_ipv4 nf_log_syslog nf_defrag_ipv6 
nf_defrag_ipv4 lzo_rle lzo_decompress lzo_compress libcrc32c crc_ccitt 
compat sha256_generic libsha256 seqiv jitterentropy_rng drbg hmac cmac 
crypto_acompress leds_gpio gpio_button_hotplug crc32c_generic
[  258.146540] CPU: 0 PID: 4377 Comm: dnsmasq Tainted: G        W 
  5.15.95 #0
[  258.161475] Stack : 00000000 00000000 80c099dc 80860000 806b0000 
80608730 80f19830 806abe23
[  258.178178]         808633b4 00001119 00000003 80061c60 80601eac 
00000001 80c09998 84245f1d
[  258.194872]         00000000 00000000 80608730 80c09830 fffff171 
00000000 00000000 ffffffea
[  258.211568]         00000000 80c0983c 00000171 806b2278 80860000 
00000009 00000000 804651e0
[  258.228273]         00000009 00000003 00000002 00000006 00000018 
803347a4 00000000 80860000
[  258.244993]         ...
[  258.249877] Call Trace:
[  258.254737] [<8000700c>] show_stack+0x28/0xf0
[  258.263459] [<8002622c>] __warn+0x9c/0x124
[  258.271649] [<80026310>] warn_slowpath_fmt+0x5c/0xac
[  258.281564] [<804651e0>] inet_csk_destroy_sock+0x6c/0x17c
[  258.292356] [<80479328>] tcp_reset+0x50/0xb0
[  258.300896] [<8047973c>] tcp_validate_incoming+0x3b4/0x624
[  258.311853] [<8047bc48>] tcp_rcv_state_process+0x32c/0xf80
[  258.322810] [<80521fa0>] tcp_v6_do_rcv+0x290/0x4e4
[  258.332400] [<80522da0>] tcp_v6_rcv+0xbac/0xc3c
[  258.341454] [<804e6798>] ip6_protocol_deliver_rcu+0x118/0x640
[  258.352945] [<804e6d6c>] ip6_input+0x84/0x9c
[  258.361477] [<803e224c>] __netif_receive_skb_one_core+0x44/0x54
[  258.373306] [<803e22ac>] netif_receive_skb+0x34/0xc8
[  258.383221] [<8054ba34>] br_handle_frame_finish+0x330/0x5b0
[  258.394362] [<8054c140>] br_handle_frame+0x48c/0x4fc
[  258.404279] [<803dfa30>] __netif_receive_skb_core.constprop.0+0x268/0xc30
[  258.417839] [<803e04e4>] __netif_receive_skb_list_core+0xec/0x224
[  258.430008] [<803e07c0>] netif_receive_skb_list_internal+0x1a4/0x254
[  258.442718] [<81c35b08>] ieee80211_rx_napi+0x84/0x8c [mac80211]
[  258.454881] [<819e07c0>] rt2x00lib_rxdone+0x318/0x940 [rt2x00lib]
[  258.467100] [<819bf08c>] rt2x00mmio_rxdone+0x8c/0xd8 [rt2x00mmio]
[  258.479279] [<819f6df0>] rt2800mmio_rxdone_tasklet+0x18/0xac [rt2800mmio]
[  258.492844] [<800294ec>] tasklet_action_common.constprop.0+0xc0/0xf8
[  258.505545] [<8057742c>] __do_softirq+0x10c/0x2c4
[  258.514934] [<80002950>] except_vec_vi_end+0xb8/0xc4
[  258.524847] [<8013b058>] __do_munmap+0xac/0x54c
[  258.533917] [<8013b55c>] __vm_munmap+0x64/0xd4
[  258.542800] [<8000ea44>] syscall_common+0x34/0x58
[  258.552197]
[  258.555161] ---[ end trace 5910e4a6b837d518 ]---

And this one:
[  259.281306] ------------[ cut here ]------------
[  259.290596] WARNING: CPU: 0 PID: 502 at net/core/stream.c:212 
inet_csk_destroy_sock+0x6c/0x17c
[  259.307833] Modules linked in: rt2800soc rt2800mmio rt2800lib pppoe 
ppp_async nft_fib_inet nf_flow_table_ipv6 nf_flow_table_ipv4 
nf_flow_table_inet rt2x00soc rt2x00mmio rt2x00lib pppox ppp_generic 
nft_reject_ipv6 nft_reject_ipv4 nft_reject_inet nft_reject nft_redir 
nft_quota nft_objref nft_numgen nft_nat nft_masq nft_log nft_limit 
nft_hash nft_flow_offload nft_fib_ipv6 nft_fib_ipv4 nft_fib nft_ct 
nft_counter nft_chain_nat nf_tables nf_nat nf_flow_table nf_conntrack 
mt76x2e mt76x2_common mt76x02_lib mt76 mac80211 lzo cfg80211 slhc 
nfnetlink nf_reject_ipv6 nf_reject_ipv4 nf_log_syslog nf_defrag_ipv6 
nf_defrag_ipv4 lzo_rle lzo_decompress lzo_compress libcrc32c crc_ccitt 
compat sha256_generic libsha256 seqiv jitterentropy_rng drbg hmac cmac 
crypto_acompress leds_gpio gpio_button_hotplug crc32c_generic
[  259.449095] CPU: 0 PID: 502 Comm: napi/phy0-3 Tainted: G        W 
     5.15.95 #0
[  259.464551] Stack : 00000000 00000000 81a21934 80860000 806b0000 
80608730 80e38870 806abe23
[  259.481251]         808633b4 000001f6 00000003 80061c60 80601eac 
00000001 81a218f0 22ee1ee2
[  259.497945]         00000000 00000000 80608730 81a21788 fffff19a 
00000000 00000000 ffffffea
[  259.514639]         00000000 81a21794 0000019a 806b2278 80860000 
00000009 00000000 804651e0
[  259.531332]         00000009 00000003 00000002 00000006 00000018 
803347a4 00000000 80860000
[  259.548030]         ...
[  259.552907] Call Trace:
[  259.557767] [<8000700c>] show_stack+0x28/0xf0
[  259.566489] [<8002622c>] __warn+0x9c/0x124
[  259.574672] [<80026310>] warn_slowpath_fmt+0x5c/0xac
[  259.584584] [<804651e0>] inet_csk_destroy_sock+0x6c/0x17c
[  259.595374] [<80479328>] tcp_reset+0x50/0xb0
[  259.603909] [<8047973c>] tcp_validate_incoming+0x3b4/0x624
[  259.614861] [<8047bc48>] tcp_rcv_state_process+0x32c/0xf80
[  259.625817] [<80521fa0>] tcp_v6_do_rcv+0x290/0x4e4
[  259.635399] [<80522da0>] tcp_v6_rcv+0xbac/0xc3c
[  259.644451] [<804e6798>] ip6_protocol_deliver_rcu+0x118/0x640
[  259.655939] [<804e6d6c>] ip6_input+0x84/0x9c
[  259.664461] [<803e224c>] __netif_receive_skb_one_core+0x44/0x54
[  259.676289] [<803e22ac>] netif_receive_skb+0x34/0xc8
[  259.686201] [<8054ba34>] br_handle_frame_finish+0x330/0x5b0
[  259.697339] [<8054c140>] br_handle_frame+0x48c/0x4fc
[  259.707255] [<803dfa30>] __netif_receive_skb_core.constprop.0+0x268/0xc30
[  259.720807] [<803e04e4>] __netif_receive_skb_list_core+0xec/0x224
[  259.732979] [<803e07c0>] netif_receive_skb_list_internal+0x1a4/0x254
[  259.745670] [<803e0bf8>] napi_complete_done+0x74/0x214
[  259.755957] [<818f2484>] mt76_dma_rx_poll+0x4b0/0x50c [mt76]
[  259.767307] [<803e0e08>] __napi_poll+0x70/0x1f8
[  259.776358] [<803e1034>] napi_threaded_poll+0xa4/0x110
[  259.786618] [<80045ef0>] kthread+0x140/0x164
[  259.795161] [<80002458>] ret_from_kernel_thread+0x14/0x1c
[  259.805942]
[  259.808899] ---[ end trace 5910e4a6b837d519 ]---

Hauke

Re: [PATCH v3 net 1/2] dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.

[Kuniyuki Iwashima]

From:   Hauke Mehrtens <hauke@hauke-m.de>
Date:   Mon, 27 Feb 2023 01:17:55 +0100
> On 2/10/23 01:22, Kuniyuki Iwashima wrote:
> > Eric Dumazet pointed out [0] that when we call skb_set_owner_r()
> > for ipv6_pinfo.pktoptions, sk_rmem_schedule() has not been called,
> > resulting in a negative sk_forward_alloc.
> > 
> > We add a new helper which clones a skb and sets its owner only
> > when sk_rmem_schedule() succeeds.
> > 
> > Note that we move skb_set_owner_r() forward in (dccp|tcp)_v6_do_rcv()
> > because tcp_send_synack() can make sk_forward_alloc negative before
> > ipv6_opt_accepted() in the crossed SYN-ACK or self-connect() cases.
> > 
> > [0]: https://lore.kernel.org/netdev/CANn89iK9oc20Jdi_41jb9URdF210r7d1Y-+uypbMSbOfY6jqrg@mail.gmail.com/
> > 
> > Fixes: 323fbd0edf3f ("net: dccp: Add handling of IPV6_PKTOPTIONS to dccp_v6_do_rcv()")
> > Fixes: 3df80d9320bc ("[DCCP]: Introduce DCCPv6")
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> > Cc: Andrii <tulup@mail.ru>
> > Cc: Arnaldo Carvalho de Melo <acme@mandriva.com>
> > ---
> >   include/net/sock.h  | 13 +++++++++++++
> >   net/dccp/ipv6.c     |  7 ++-----
> >   net/ipv6/tcp_ipv6.c | 10 +++-------
> >   3 files changed, 18 insertions(+), 12 deletions(-)
> > 
> Hi,
> 
> Multiples people reported kernel warnings after the upgrade from kernel 
> 5.15.94 to 5.15.95 in OpenWrt master, see 
> https://github.com/openwrt/openwrt/pull/12071
> 
> This was seen on a MIPS and a x86 CPU. It is happening when a Windows 
> client connected, it works fine when an Android or iPad connects.
> 
> OpenWrt has some additional patches on top of the kernel, we haven't 
> checked yet if they are causing this problem in combination with this 
> new change. With kernel 5.15.94 as a base it works fine.
> 
> The problem is not showing up when the backport of this patch is reverted.
> 
> This warning was reported:
> 
> [  257.978586] ------------[ cut here ]------------
> [  257.987882] WARNING: CPU: 0 PID: 4377 at net/core/stream.c:212 
> inet_csk_destroy_sock+0x6c/0x17c

Hi,

Thanks for the report.

The both WARNINGs come from the same place which was removed in the
next patch in this series.

It seems the stable tree only backport this patch without the
following one.  I'll cook patches for 4.14/4.19/5.4/5.10/5.15/6.1.
Until they go into the stable, please apply this patch on top of
your tree.
https://lore.kernel.org/netdev/20230210002202.81442-3-kuniyu@amazon.com/

Thanks,
Kuniyuki


> [  258.005287] Modules linked in: rt2800soc rt2800mmio rt2800lib pppoe 
> ppp_async nft_fib_inet nf_flow_table_ipv6 nf_flow_table_ipv4 
> nf_flow_table_inet rt2x00soc rt2x00mmio rt2x00lib pppox ppp_generic 
> nft_reject_ipv6 nft_reject_ipv4 nft_reject_inet nft_reject nft_redir 
> nft_quota nft_objref nft_numgen nft_nat nft_masq nft_log nft_limit 
> nft_hash nft_flow_offload nft_fib_ipv6 nft_fib_ipv4 nft_fib nft_ct 
> nft_counter nft_chain_nat nf_tables nf_nat nf_flow_table nf_conntrack 
> mt76x2e mt76x2_common mt76x02_lib mt76 mac80211 lzo cfg80211 slhc 
> nfnetlink nf_reject_ipv6 nf_reject_ipv4 nf_log_syslog nf_defrag_ipv6 
> nf_defrag_ipv4 lzo_rle lzo_decompress lzo_compress libcrc32c crc_ccitt 
> compat sha256_generic libsha256 seqiv jitterentropy_rng drbg hmac cmac 
> crypto_acompress leds_gpio gpio_button_hotplug crc32c_generic
> [  258.146540] CPU: 0 PID: 4377 Comm: dnsmasq Tainted: G        W 
>   5.15.95 #0
> [  258.161475] Stack : 00000000 00000000 80c099dc 80860000 806b0000 
> 80608730 80f19830 806abe23
> [  258.178178]         808633b4 00001119 00000003 80061c60 80601eac 
> 00000001 80c09998 84245f1d
> [  258.194872]         00000000 00000000 80608730 80c09830 fffff171 
> 00000000 00000000 ffffffea
> [  258.211568]         00000000 80c0983c 00000171 806b2278 80860000 
> 00000009 00000000 804651e0
> [  258.228273]         00000009 00000003 00000002 00000006 00000018 
> 803347a4 00000000 80860000
> [  258.244993]         ...
> [  258.249877] Call Trace:
> [  258.254737] [<8000700c>] show_stack+0x28/0xf0
> [  258.263459] [<8002622c>] __warn+0x9c/0x124
> [  258.271649] [<80026310>] warn_slowpath_fmt+0x5c/0xac
> [  258.281564] [<804651e0>] inet_csk_destroy_sock+0x6c/0x17c
> [  258.292356] [<80479328>] tcp_reset+0x50/0xb0
> [  258.300896] [<8047973c>] tcp_validate_incoming+0x3b4/0x624
> [  258.311853] [<8047bc48>] tcp_rcv_state_process+0x32c/0xf80
> [  258.322810] [<80521fa0>] tcp_v6_do_rcv+0x290/0x4e4
> [  258.332400] [<80522da0>] tcp_v6_rcv+0xbac/0xc3c
> [  258.341454] [<804e6798>] ip6_protocol_deliver_rcu+0x118/0x640
> [  258.352945] [<804e6d6c>] ip6_input+0x84/0x9c
> [  258.361477] [<803e224c>] __netif_receive_skb_one_core+0x44/0x54
> [  258.373306] [<803e22ac>] netif_receive_skb+0x34/0xc8
> [  258.383221] [<8054ba34>] br_handle_frame_finish+0x330/0x5b0
> [  258.394362] [<8054c140>] br_handle_frame+0x48c/0x4fc
> [  258.404279] [<803dfa30>] __netif_receive_skb_core.constprop.0+0x268/0xc30
> [  258.417839] [<803e04e4>] __netif_receive_skb_list_core+0xec/0x224
> [  258.430008] [<803e07c0>] netif_receive_skb_list_internal+0x1a4/0x254
> [  258.442718] [<81c35b08>] ieee80211_rx_napi+0x84/0x8c [mac80211]
> [  258.454881] [<819e07c0>] rt2x00lib_rxdone+0x318/0x940 [rt2x00lib]
> [  258.467100] [<819bf08c>] rt2x00mmio_rxdone+0x8c/0xd8 [rt2x00mmio]
> [  258.479279] [<819f6df0>] rt2800mmio_rxdone_tasklet+0x18/0xac [rt2800mmio]
> [  258.492844] [<800294ec>] tasklet_action_common.constprop.0+0xc0/0xf8
> [  258.505545] [<8057742c>] __do_softirq+0x10c/0x2c4
> [  258.514934] [<80002950>] except_vec_vi_end+0xb8/0xc4
> [  258.524847] [<8013b058>] __do_munmap+0xac/0x54c
> [  258.533917] [<8013b55c>] __vm_munmap+0x64/0xd4
> [  258.542800] [<8000ea44>] syscall_common+0x34/0x58
> [  258.552197]
> [  258.555161] ---[ end trace 5910e4a6b837d518 ]---
> 
> And this one:
> [  259.281306] ------------[ cut here ]------------
> [  259.290596] WARNING: CPU: 0 PID: 502 at net/core/stream.c:212 
> inet_csk_destroy_sock+0x6c/0x17c
> [  259.307833] Modules linked in: rt2800soc rt2800mmio rt2800lib pppoe 
> ppp_async nft_fib_inet nf_flow_table_ipv6 nf_flow_table_ipv4 
> nf_flow_table_inet rt2x00soc rt2x00mmio rt2x00lib pppox ppp_generic 
> nft_reject_ipv6 nft_reject_ipv4 nft_reject_inet nft_reject nft_redir 
> nft_quota nft_objref nft_numgen nft_nat nft_masq nft_log nft_limit 
> nft_hash nft_flow_offload nft_fib_ipv6 nft_fib_ipv4 nft_fib nft_ct 
> nft_counter nft_chain_nat nf_tables nf_nat nf_flow_table nf_conntrack 
> mt76x2e mt76x2_common mt76x02_lib mt76 mac80211 lzo cfg80211 slhc 
> nfnetlink nf_reject_ipv6 nf_reject_ipv4 nf_log_syslog nf_defrag_ipv6 
> nf_defrag_ipv4 lzo_rle lzo_decompress lzo_compress libcrc32c crc_ccitt 
> compat sha256_generic libsha256 seqiv jitterentropy_rng drbg hmac cmac 
> crypto_acompress leds_gpio gpio_button_hotplug crc32c_generic
> [  259.449095] CPU: 0 PID: 502 Comm: napi/phy0-3 Tainted: G        W 
>      5.15.95 #0
> [  259.464551] Stack : 00000000 00000000 81a21934 80860000 806b0000 
> 80608730 80e38870 806abe23
> [  259.481251]         808633b4 000001f6 00000003 80061c60 80601eac 
> 00000001 81a218f0 22ee1ee2
> [  259.497945]         00000000 00000000 80608730 81a21788 fffff19a 
> 00000000 00000000 ffffffea
> [  259.514639]         00000000 81a21794 0000019a 806b2278 80860000 
> 00000009 00000000 804651e0
> [  259.531332]         00000009 00000003 00000002 00000006 00000018 
> 803347a4 00000000 80860000
> [  259.548030]         ...
> [  259.552907] Call Trace:
> [  259.557767] [<8000700c>] show_stack+0x28/0xf0
> [  259.566489] [<8002622c>] __warn+0x9c/0x124
> [  259.574672] [<80026310>] warn_slowpath_fmt+0x5c/0xac
> [  259.584584] [<804651e0>] inet_csk_destroy_sock+0x6c/0x17c
> [  259.595374] [<80479328>] tcp_reset+0x50/0xb0
> [  259.603909] [<8047973c>] tcp_validate_incoming+0x3b4/0x624
> [  259.614861] [<8047bc48>] tcp_rcv_state_process+0x32c/0xf80
> [  259.625817] [<80521fa0>] tcp_v6_do_rcv+0x290/0x4e4
> [  259.635399] [<80522da0>] tcp_v6_rcv+0xbac/0xc3c
> [  259.644451] [<804e6798>] ip6_protocol_deliver_rcu+0x118/0x640
> [  259.655939] [<804e6d6c>] ip6_input+0x84/0x9c
> [  259.664461] [<803e224c>] __netif_receive_skb_one_core+0x44/0x54
> [  259.676289] [<803e22ac>] netif_receive_skb+0x34/0xc8
> [  259.686201] [<8054ba34>] br_handle_frame_finish+0x330/0x5b0
> [  259.697339] [<8054c140>] br_handle_frame+0x48c/0x4fc
> [  259.707255] [<803dfa30>] __netif_receive_skb_core.constprop.0+0x268/0xc30
> [  259.720807] [<803e04e4>] __netif_receive_skb_list_core+0xec/0x224
> [  259.732979] [<803e07c0>] netif_receive_skb_list_internal+0x1a4/0x254
> [  259.745670] [<803e0bf8>] napi_complete_done+0x74/0x214
> [  259.755957] [<818f2484>] mt76_dma_rx_poll+0x4b0/0x50c [mt76]
> [  259.767307] [<803e0e08>] __napi_poll+0x70/0x1f8
> [  259.776358] [<803e1034>] napi_threaded_poll+0xa4/0x110
> [  259.786618] [<80045ef0>] kthread+0x140/0x164
> [  259.795161] [<80002458>] ret_from_kernel_thread+0x14/0x1c
> [  259.805942]
> [  259.808899] ---[ end trace 5910e4a6b837d519 ]---
> 
> Hauke

[PATCH v3 net 2/2] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().

[Kuniyuki Iwashima]

Christoph Paasch reported that commit b5fc29233d28 ("inet6: Remove
inet6_destroy_sock() in sk->sk_prot->destroy().") started triggering
WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues().  [0 - 2]
Also, we can reproduce it by a program in [3].

In the commit, we delay freeing ipv6_pinfo.pktoptions from sk->destroy()
to sk->sk_destruct(), so sk->sk_forward_alloc is no longer zero in
inet_csk_destroy_sock().

The same check has been in inet_sock_destruct() from at least v2.6,
we can just remove the WARN_ON_ONCE().  However, among the users of
sk_stream_kill_queues(), only CAIF is not calling inet_sock_destruct().
Thus, we add the same WARN_ON_ONCE() 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>

[3]: https://lore.kernel.org/netdev/20230208004245.83497-1-kuniyu@amazon.com/

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>
---
Cc: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 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

Re: [PATCH v3 net 2/2] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().

[Eric Dumazet]

On Fri, Feb 10, 2023 at 1:23 AM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> Christoph Paasch reported that commit b5fc29233d28 ("inet6: Remove
> inet6_destroy_sock() in sk->sk_prot->destroy().") started triggering
> WARN_ON_ONCE(sk->sk_forward_alloc) in sk_stream_kill_queues().  [0 - 2]
> Also, we can reproduce it by a program in [3].
>
> In the commit, we delay freeing ipv6_pinfo.pktoptions from sk->destroy()
> to sk->sk_destruct(), so sk->sk_forward_alloc is no longer zero in
> inet_csk_destroy_sock().
>
Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !

Re: [PATCH v3 net 0/2] sk->sk_forward_alloc fixes.

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu, 9 Feb 2023 16:22:00 -0800 you wrote:
> The first patch fixes a negative sk_forward_alloc by adding
> sk_rmem_schedule() before skb_set_owner_r(), and second patch
> removes an unnecessary WARN_ON_ONCE().
> 
> Changes:
>   v3:
>     * Factorise a common pattern in Patch 1 as suggested by Eric
> 
> [...]

Here is the summary with links:
  - [v3,net,1/2] dccp/tcp: Avoid negative sk_forward_alloc by ipv6_pinfo.pktoptions.
    https://git.kernel.org/netdev/net/c/ca43ccf41224
  - [v3,net,2/2] net: Remove WARN_ON_ONCE(sk->sk_forward_alloc) from sk_stream_kill_queues().
    https://git.kernel.org/netdev/net/c/62ec33b44e0f

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html