netdev - Re: [PATCH v3 net-next 11/15] socket: Remove kernel socket conversion.

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241213135437.44216-1-kuniyu@amazon.com>
Date: Fri, 13 Dec 2024 22:54:37 +0900
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <wenjia@...ux.ibm.com>
CC: <alibuda@...ux.alibaba.com>, <allison.henderson@...cle.com>,
	<chuck.lever@...cle.com>, <davem@...emloft.net>, <edumazet@...gle.com>,
	<horms@...nel.org>, <jaka@...ux.ibm.com>, <jlayton@...nel.org>,
	<kuba@...nel.org>, <kuni1840@...il.com>, <kuniyu@...zon.com>,
	<matttbe@...nel.org>, <netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<sfrench@...ba.org>
Subject: Re: [PATCH v3 net-next 11/15] socket: Remove kernel socket conversion.

From: Wenjia Zhang <wenjia@...ux.ibm.com>
Date: Fri, 13 Dec 2024 14:45:20 +0100
> > diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
> > index 6e93f188a908..7b0de80b3aca 100644
> > --- a/net/smc/af_smc.c
> > +++ b/net/smc/af_smc.c
> > @@ -3310,25 +3310,8 @@ static const struct proto_ops smc_sock_ops = {
> >   
> >   int smc_create_clcsk(struct net *net, struct sock *sk, int family)
> >   {
> > -	struct smc_sock *smc = smc_sk(sk);
> > -	int rc;
> > -
> > -	rc = sock_create_kern(net, family, SOCK_STREAM, IPPROTO_TCP,
> > -			      &smc->clcsock);
> > -	if (rc)
> > -		return rc;
> > -
> > -	/* smc_clcsock_release() does not wait smc->clcsock->sk's
> > -	 * destruction;  its sk_state might not be TCP_CLOSE after
> > -	 * smc->sk is close()d, and TCP timers can be fired later,
> > -	 * which need net ref.
> > -	 */
> > -	sk = smc->clcsock->sk;
> > -	__netns_tracker_free(net, &sk->ns_tracker, false);
> > -	sk->sk_net_refcnt = 1;
> > -	get_net_track(net, &sk->ns_tracker, GFP_KERNEL);
> > -	sock_inuse_add(net, 1);
> I don't think this line shoud be removed. Otherwise, the popurse here to 
> manage the per namespace statistics in the case of network namespace 
> isolation would be lost.

Now it's counted in sk_alloc().

sock_create_net() below passes hold_net=true to sk_alloc() and if
sk->sk_netns_refcnt (== hold_net) is true, sock_inuse_add() is
called there.

See patch 9 and 10:
https://lore.kernel.org/netdev/20241213092152.14057-10-kuniyu@amazon.com/
https://lore.kernel.org/netdev/20241213092152.14057-11-kuniyu@amazon.com/


> @D. Wythe, could you please check it again? Maybe you have some good 
> testing on this case.
> 
> > -	return 0;
> > +	return sock_create_net(net, family, SOCK_STREAM, IPPROTO_TCP,
> > +			       &smc_sk(sk)->clcsock);
> >   }