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: <2eb0df2c5dd8b16b5103f0e2859690519c4f2dad.camel@oracle.com>
Date: Mon, 14 Jul 2025 04:36:32 +0000
From: Allison Henderson <allison.henderson@...cle.com>
To: "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [RFC][PATCH] don't open-code kernel_accept() in
 rds_tcp_accept_one()

On Sun, 2025-07-13 at 19:01 +0100, Al Viro wrote:
> 	rds_tcp_accept_one() starts with a pretty much verbatim
> copy of kernel_accept().  Might as well use the real thing...
> 
> 	That code went into mainline in 2009, kernel_accept()
> had been added in Aug 2006, the copyright on rds/tcp_listen.c
> is "Copyright (c) 2006 Oracle", so it's entirely possible
> that it predates the introduction of kernel_accept().
> 
> Signed-off-by: Al Viro <viro@...iv.linux.org.uk>
> ---
> diff --git a/net/rds/tcp_listen.c b/net/rds/tcp_listen.c
> index d89bd8d0c354..af36f5bf8649 100644
> --- a/net/rds/tcp_listen.c
> +++ b/net/rds/tcp_listen.c
> @@ -105,10 +105,6 @@ int rds_tcp_accept_one(struct socket *sock)
>  	int conn_state;
>  	struct rds_conn_path *cp;
>  	struct in6_addr *my_addr, *peer_addr;
> -	struct proto_accept_arg arg = {
> -		.flags = O_NONBLOCK,
> -		.kern = true,
> -	};
>  #if !IS_ENABLED(CONFIG_IPV6)
>  	struct in6_addr saddr, daddr;
>  #endif
> @@ -117,25 +113,9 @@ int rds_tcp_accept_one(struct socket *sock)
>  	if (!sock) /* module unload or netns delete in progress */
>  		return -ENETUNREACH;
>  
> -	ret = sock_create_lite(sock->sk->sk_family,
> -			       sock->sk->sk_type, sock->sk->sk_protocol,
> -			       &new_sock);
> +	ret = kernel_accept(sock, &new_sock, O_NONBLOCK);
>  	if (ret)
> -		goto out;
> -
> -	ret = sock->ops->accept(sock, new_sock, &arg);
> -	if (ret < 0)
> -		goto out;
> -
> -	/* sock_create_lite() does not get a hold on the owner module so we
> -	 * need to do it here.  Note that sock_release() uses sock->ops to
> -	 * determine if it needs to decrement the reference count.  So set
> -	 * sock->ops after calling accept() in case that fails.  And there's
> -	 * no need to do try_module_get() as the listener should have a hold
> -	 * already.
> -	 */
> -	new_sock->ops = sock->ops;
> -	__module_get(new_sock->ops->owner);
> +		return ret;
I think we need the "goto out" here, or we will miss the mutex unlock.  Otherwise kernel_accept looks like a pretty
synonymous wrapper.

Thanks!
Allison

>  
>  	rds_tcp_keepalive(new_sock);
>  	if (!rds_tcp_tune(new_sock)) {

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ