| 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
| ||
|
Message-ID: <20230215045630.85835-1-kuniyu@amazon.com>
Date: Tue, 14 Feb 2023 20:56:30 -0800
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <kerneljasonxing@...il.com>
CC: <bjorn@...nel.org>, <bpf@...r.kernel.org>, <davem@...emloft.net>,
<dsahern@...nel.org>, <edumazet@...gle.com>, <jaka@...ux.ibm.com>,
<jonathan.lemon@...il.com>, <kernelxing@...cent.com>,
<kgraul@...ux.ibm.com>, <kuba@...nel.org>, <kuniyu@...zon.com>,
<linux-kernel@...r.kernel.org>, <linux-s390@...r.kernel.org>,
<linux-sctp@...r.kernel.org>, <lucien.xin@...il.com>,
<maciej.fijalkowski@...el.com>, <magnus.karlsson@...el.com>,
<marcelo.leitner@...il.com>, <matthieu.baerts@...sares.net>,
<mptcp@...ts.linux.dev>, <netdev@...r.kernel.org>,
<nhorman@...driver.com>, <pabeni@...hat.com>,
<wenjia@...ux.ibm.com>, <willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH v2 net-next] net: no longer support SOCK_REFCNT_DEBUG feature
From: Jason Xing <kerneljasonxing@...il.com>
Date: Tue, 14 Feb 2023 12:14:10 +0800
> From: Jason Xing <kernelxing@...cent.com>
>
> Commit e48c414ee61f ("[INET]: Generalise the TCP sock ID lookup routines")
> commented out the definition of SOCK_REFCNT_DEBUG in 2005 and later another
> commit 463c84b97f24 ("[NET]: Introduce inet_connection_sock") removed it.
> Since we could track all of them through bpf and kprobe related tools
> and the feature could print loads of information which might not be
> that helpful even under a little bit pressure, the whole feature which
> has been inactive for many years is no longer supported.
>
> Link: https://lore.kernel.org/lkml/20230211065153.54116-1-kerneljasonxing@gmail.com/
> Suggested-by: Kuniyuki Iwashima <kuniyu@...zon.com>
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>
Thanks!
> ---
> v2:
> 1) change the title and body message.
> 2) remove the whole feature instead suggested by Kuniyuki Iwashima.
> ---
> include/net/sock.h | 28 ----------------------------
> net/core/sock.c | 13 -------------
> net/ipv4/af_inet.c | 3 ---
> net/ipv4/inet_connection_sock.c | 2 --
> net/ipv4/inet_timewait_sock.c | 3 ---
> net/ipv6/af_inet6.c | 10 ----------
> net/ipv6/ipv6_sockglue.c | 12 ------------
> net/mptcp/protocol.c | 1 -
> net/packet/af_packet.c | 4 ----
> net/sctp/ipv6.c | 2 --
> net/sctp/protocol.c | 2 --
> net/smc/af_smc.c | 3 ---
> net/xdp/xsk.c | 4 ----
> 13 files changed, 87 deletions(-)
>
> diff --git a/include/net/sock.h b/include/net/sock.h
> index dcd72e6285b2..e6369068a7bb 100644
> --- a/include/net/sock.h
> +++ b/include/net/sock.h
> @@ -1349,9 +1349,6 @@ struct proto {
> char name[32];
>
> struct list_head node;
> -#ifdef SOCK_REFCNT_DEBUG
> - atomic_t socks;
> -#endif
> int (*diag_destroy)(struct sock *sk, int err);
> } __randomize_layout;
>
> @@ -1359,31 +1356,6 @@ int proto_register(struct proto *prot, int alloc_slab);
> void proto_unregister(struct proto *prot);
> int sock_load_diag_module(int family, int protocol);
>
> -#ifdef SOCK_REFCNT_DEBUG
> -static inline void sk_refcnt_debug_inc(struct sock *sk)
> -{
> - atomic_inc(&sk->sk_prot->socks);
> -}
> -
> -static inline void sk_refcnt_debug_dec(struct sock *sk)
> -{
> - atomic_dec(&sk->sk_prot->socks);
> - printk(KERN_DEBUG "%s socket %p released, %d are still alive\n",
> - sk->sk_prot->name, sk, atomic_read(&sk->sk_prot->socks));
> -}
> -
> -static inline void sk_refcnt_debug_release(const struct sock *sk)
> -{
> - if (refcount_read(&sk->sk_refcnt) != 1)
> - printk(KERN_DEBUG "Destruction of the %s socket %p delayed, refcnt=%d\n",
> - sk->sk_prot->name, sk, refcount_read(&sk->sk_refcnt));
> -}
> -#else /* SOCK_REFCNT_DEBUG */
> -#define sk_refcnt_debug_inc(sk) do { } while (0)
> -#define sk_refcnt_debug_dec(sk) do { } while (0)
> -#define sk_refcnt_debug_release(sk) do { } while (0)
> -#endif /* SOCK_REFCNT_DEBUG */
> -
> INDIRECT_CALLABLE_DECLARE(bool tcp_stream_memory_free(const struct sock *sk, int wake));
>
> static inline int sk_forward_alloc_get(const struct sock *sk)
> diff --git a/net/core/sock.c b/net/core/sock.c
> index f954d5893e79..be7b29d97637 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -2338,17 +2338,6 @@ struct sock *sk_clone_lock(const struct sock *sk, const gfp_t priority)
> smp_wmb();
> refcount_set(&newsk->sk_refcnt, 2);
>
> - /* Increment the counter in the same struct proto as the master
> - * sock (sk_refcnt_debug_inc uses newsk->sk_prot->socks, that
> - * is the same as sk->sk_prot->socks, as this field was copied
> - * with memcpy).
> - *
> - * This _changes_ the previous behaviour, where
> - * tcp_create_openreq_child always was incrementing the
> - * equivalent to tcp_prot->socks (inet_sock_nr), so this have
> - * to be taken into account in all callers. -acme
> - */
> - sk_refcnt_debug_inc(newsk);
> sk_set_socket(newsk, NULL);
> sk_tx_queue_clear(newsk);
> RCU_INIT_POINTER(newsk->sk_wq, NULL);
> @@ -3696,8 +3685,6 @@ void sk_common_release(struct sock *sk)
>
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> -
> sock_put(sk);
> }
> EXPORT_SYMBOL(sk_common_release);
> diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
> index 6c0ec2789943..f46a3924c440 100644
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -156,7 +156,6 @@ void inet_sock_destruct(struct sock *sk)
> kfree(rcu_dereference_protected(inet->inet_opt, 1));
> dst_release(rcu_dereference_protected(sk->sk_dst_cache, 1));
> dst_release(rcu_dereference_protected(sk->sk_rx_dst, 1));
> - sk_refcnt_debug_dec(sk);
> }
> EXPORT_SYMBOL(inet_sock_destruct);
>
> @@ -356,8 +355,6 @@ static int inet_create(struct net *net, struct socket *sock, int protocol,
> inet->mc_list = NULL;
> inet->rcv_tos = 0;
>
> - sk_refcnt_debug_inc(sk);
> -
> if (inet->inet_num) {
> /* It assumes that any protocol which allows
> * the user to assign a number at socket
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index d1f837579398..64be59d93b04 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -1178,8 +1178,6 @@ void inet_csk_destroy_sock(struct sock *sk)
>
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> -
> this_cpu_dec(*sk->sk_prot->orphan_count);
>
> sock_put(sk);
> diff --git a/net/ipv4/inet_timewait_sock.c b/net/ipv4/inet_timewait_sock.c
> index beed32fff484..40052414c7c7 100644
> --- a/net/ipv4/inet_timewait_sock.c
> +++ b/net/ipv4/inet_timewait_sock.c
> @@ -77,9 +77,6 @@ void inet_twsk_free(struct inet_timewait_sock *tw)
> {
> struct module *owner = tw->tw_prot->owner;
> twsk_destructor((struct sock *)tw);
> -#ifdef SOCK_REFCNT_DEBUG
> - pr_debug("%s timewait_sock %p released\n", tw->tw_prot->name, tw);
> -#endif
> kmem_cache_free(tw->tw_prot->twsk_prot->twsk_slab, tw);
> module_put(owner);
> }
> diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
> index fee9163382c2..c93f2e865fea 100644
> --- a/net/ipv6/af_inet6.c
> +++ b/net/ipv6/af_inet6.c
> @@ -238,16 +238,6 @@ static int inet6_create(struct net *net, struct socket *sock, int protocol,
> inet->pmtudisc = IP_PMTUDISC_DONT;
> else
> inet->pmtudisc = IP_PMTUDISC_WANT;
> - /*
> - * Increment only the relevant sk_prot->socks debug field, this changes
> - * the previous behaviour of incrementing both the equivalent to
> - * answer->prot->socks (inet6_sock_nr) and inet_sock_nr.
> - *
> - * This allows better debug granularity as we'll know exactly how many
> - * UDPv6, TCPv6, etc socks were allocated, not the sum of all IPv6
> - * transport protocol socks. -acme
> - */
> - sk_refcnt_debug_inc(sk);
>
> if (inet->inet_num) {
> /* It assumes that any protocol which allows
> diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
> index 9ce51680290b..2917dd8d198c 100644
> --- a/net/ipv6/ipv6_sockglue.c
> +++ b/net/ipv6/ipv6_sockglue.c
> @@ -464,13 +464,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
> __ipv6_sock_mc_close(sk);
> __ipv6_sock_ac_close(sk);
>
> - /*
> - * Sock is moving from IPv6 to IPv4 (sk_prot), so
> - * remove it from the refcnt debug socks count in the
> - * original family...
> - */
> - sk_refcnt_debug_dec(sk);
> -
> if (sk->sk_protocol == IPPROTO_TCP) {
> struct inet_connection_sock *icsk = inet_csk(sk);
>
> @@ -507,11 +500,6 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
>
> inet6_cleanup_sock(sk);
>
> - /*
> - * ... and add it to the refcnt debug socks count
> - * in the new family. -acme
> - */
> - sk_refcnt_debug_inc(sk);
> module_put(THIS_MODULE);
> retv = 0;
> break;
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 8cd6cc67c2c5..e913752df112 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -2876,7 +2876,6 @@ static void __mptcp_destroy_sock(struct sock *sk)
> sk_stream_kill_queues(sk);
> xfrm_sk_free_policy(sk);
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
> }
>
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index b5ab98ca2511..a4c8f86ac12a 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -1335,8 +1335,6 @@ static void packet_sock_destruct(struct sock *sk)
> pr_err("Attempt to release alive packet socket: %p\n", sk);
> return;
> }
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static bool fanout_flow_is_huge(struct packet_sock *po, struct sk_buff *skb)
> @@ -3172,7 +3170,6 @@ static int packet_release(struct socket *sock)
>
> skb_queue_purge(&sk->sk_receive_queue);
> packet_free_pending(po);
> - sk_refcnt_debug_release(sk);
>
> sock_put(sk);
> return 0;
> @@ -3362,7 +3359,6 @@ static int packet_create(struct net *net, struct socket *sock, int protocol,
> packet_cached_dev_reset(po);
>
> sk->sk_destruct = packet_sock_destruct;
> - sk_refcnt_debug_inc(sk);
>
> /*
> * Attach a protocol block
> diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
> index 097bd60ce964..62b436a2c8fe 100644
> --- a/net/sctp/ipv6.c
> +++ b/net/sctp/ipv6.c
> @@ -807,8 +807,6 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
>
> newsk->sk_v6_rcv_saddr = sk->sk_v6_rcv_saddr;
>
> - sk_refcnt_debug_inc(newsk);
> -
> if (newsk->sk_prot->init(newsk)) {
> sk_common_release(newsk);
> newsk = NULL;
> diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
> index 909a89a1cff4..c365df24ad33 100644
> --- a/net/sctp/protocol.c
> +++ b/net/sctp/protocol.c
> @@ -601,8 +601,6 @@ static struct sock *sctp_v4_create_accept_sk(struct sock *sk,
>
> newinet->inet_daddr = asoc->peer.primary_addr.v4.sin_addr.s_addr;
>
> - sk_refcnt_debug_inc(newsk);
> -
> if (newsk->sk_prot->init(newsk)) {
> sk_common_release(newsk);
> newsk = NULL;
> diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> index e12d4fa5aece..c594312e22cd 100644
> --- a/net/smc/af_smc.c
> +++ b/net/smc/af_smc.c
> @@ -359,8 +359,6 @@ static void smc_destruct(struct sock *sk)
> return;
> if (!sock_flag(sk, SOCK_DEAD))
> return;
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
> @@ -389,7 +387,6 @@ static struct sock *smc_sock_alloc(struct net *net, struct socket *sock,
> spin_lock_init(&smc->accept_q_lock);
> spin_lock_init(&smc->conn.send_lock);
> sk->sk_prot->hash(sk);
> - sk_refcnt_debug_inc(sk);
> mutex_init(&smc->clcsock_release_lock);
> smc_init_saved_callbacks(smc);
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 9f0561b67c12..a245c1b4a21b 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -845,7 +845,6 @@ static int xsk_release(struct socket *sock)
> sock_orphan(sk);
> sock->sk = NULL;
>
> - sk_refcnt_debug_release(sk);
> sock_put(sk);
>
> return 0;
> @@ -1396,8 +1395,6 @@ static void xsk_destruct(struct sock *sk)
>
> if (!xp_put_pool(xs->pool))
> xdp_put_umem(xs->umem, !xs->pool);
> -
> - sk_refcnt_debug_dec(sk);
> }
>
> static int xsk_create(struct net *net, struct socket *sock, int protocol,
> @@ -1427,7 +1424,6 @@ static int xsk_create(struct net *net, struct socket *sock, int protocol,
> sk->sk_family = PF_XDP;
>
> sk->sk_destruct = xsk_destruct;
> - sk_refcnt_debug_inc(sk);
>
> sock_set_flag(sk, SOCK_RCU_FREE);
>
> --
> 2.37.3
Powered by blists - more mailing lists