Commit 09dba37e authored by Chuck Lever's avatar Chuck Lever Committed by Paolo Abeni
Browse files

net/handshake: Take a long-lived file reference at submit 



handshake_nl_accept_doit() needs the file pointer backing
req->hr_sk->sk_socket to survive the window between
handshake_req_next() and the subsequent FD_PREPARE() and get_file().
The submit-side sock_hold() does not provide that.  sk_refcnt keeps
struct sock alive, but struct socket is owned by sock->file: when
the consumer fputs the last file reference, sock_release() tears
the socket down regardless of any sock_hold.

Add an hr_file pointer to struct handshake_req and acquire an
explicit reference on sock->file during handshake_req_submit().
handshake_complete() and handshake_req_cancel() release the
reference on the completion-bit-winning path.

The submit error path must also release the file reference, but
after rhashtable insertion a concurrent handshake_req_cancel() can
discover the request and race the error path.  Gate the error-path
cleanup -- sk_destruct restoration, fput, and request destruction
-- with test_and_set_bit(HANDSHAKE_F_REQ_COMPLETED), the same
serialization handshake_complete() and handshake_req_cancel()
already use.  When cancel has already claimed ownership, the submit
error path returns without touching the request; socket teardown
handles final destruction.

The accept-side dereferences are not yet retargeted; that change
comes in the next patch.

Signed-off-by: default avatarChuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260525-handshake-file-pin-v3-4-66c616906ead@oracle.com


Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 6b22d433

net/handshake/handshake.h

+2 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ enum hn_flags_bits { 
	HANDSHAKE_F_NET_DRAINING,

};



struct file;

struct handshake_proto;



/* One handshake request */
@@ -32,6 +33,7 @@ struct handshake_req { 
	struct rhash_head		hr_rhash;

	unsigned long			hr_flags;

	const struct handshake_proto	*hr_proto;

	struct file			*hr_file;

	struct sock			*hr_sk;

	void				(*hr_odestruct)(struct sock *sk);

net/handshake/netlink.c

+0 −6
Original line number Diff line number Diff line
@@ -210,12 +210,6 @@ static void __net_exit handshake_net_exit(struct net *net) 
	while (!list_empty(&requests)) {

		req = list_first_entry(&requests, struct handshake_req, hr_list);

		list_del(&req->hr_list);



		/*

		 * Requests on this list have not yet been

		 * accepted, so they do not have an fd to put.

		 */



		handshake_complete(req, -ETIMEDOUT, NULL);

	}

}

net/handshake/request.c

+35 −7
Original line number Diff line number Diff line
@@ -13,6 +13,7 @@ 
#include <linux/module.h>

#include <linux/skbuff.h>

#include <linux/inet.h>

#include <linux/file.h>

#include <linux/rhashtable.h>



#include <net/sock.h>
@@ -215,9 +216,16 @@ EXPORT_SYMBOL_IF_KUNIT(handshake_req_next); 
 * A zero return value from handshake_req_submit() means that

 * exactly one subsequent completion callback is guaranteed.

 *

 * A negative return value from handshake_req_submit() means that

 * no completion callback will be done and that @req has been

 * destroyed.

 * A negative return value from handshake_req_submit() guarantees that

 * no completion callback will occur and that @req is no longer owned by

 * the caller. If cancellation wins the completion race after the request

 * has been published, final destruction is deferred until socket teardown.

 *

 * The caller must hold a reference on @sock->file for the duration

 * of this call. Once the request is published to the accept side, a

 * concurrent completion or cancellation may release the request's pin on

 * @sock->file; the caller's reference is what keeps @sock->sk valid until

 * handshake_req_submit() returns.

 */

int handshake_req_submit(struct socket *sock, struct handshake_req *req,

			 gfp_t flags)
@@ -236,6 +244,14 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req, 
		kfree(req);

		return -EINVAL;

	}



	/*

	 * Pin sock->file for the lifetime of the request so the

	 * accept side does not race a consumer that releases the

	 * socket while a handshake is pending.

	 */

	req->hr_file = get_file(sock->file);



	req->hr_odestruct = req->hr_sk->sk_destruct;

	req->hr_sk->sk_destruct = handshake_sk_destruct;
@@ -267,7 +283,11 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req, 
			goto out_err;

	}



	/* Prevent socket release while a handshake request is pending */

	/*

	 * Pin struct sock so sk_destruct does not run until the

	 * handshake completion path releases it; struct socket is

	 * held separately via hr_file above.

	 */

	sock_hold(req->hr_sk);



	trace_handshake_submit(net, req, req->hr_sk);
@@ -276,10 +296,13 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req, 
out_unlock:

	spin_unlock_bh(&hn->hn_lock);

out_err:

	/* Restore original destructor so socket teardown still runs on failure */

	req->hr_sk->sk_destruct = req->hr_odestruct;

	trace_handshake_submit_err(net, req, req->hr_sk, ret);

	if (!test_and_set_bit(HANDSHAKE_F_REQ_COMPLETED, &req->hr_flags)) {

		/* Restore original destructor so socket teardown still runs. */

		req->hr_sk->sk_destruct = req->hr_odestruct;

		fput(req->hr_file);

		handshake_req_destroy(req);

	}

	return ret;

}

EXPORT_SYMBOL(handshake_req_submit);
@@ -291,11 +314,15 @@ void handshake_complete(struct handshake_req *req, int status, 
	struct net *net = sock_net(sk);



	if (!test_and_set_bit(HANDSHAKE_F_REQ_COMPLETED, &req->hr_flags)) {

		struct file *file = req->hr_file;



		trace_handshake_complete(net, req, sk, status);

		req->hr_proto->hp_done(req, status, info);



		/* Handshake request is no longer pending */

		sock_put(sk);



		fput(file);

	}

}

EXPORT_SYMBOL_IF_KUNIT(handshake_complete);
@@ -344,6 +371,7 @@ bool handshake_req_cancel(struct sock *sk) 


	/* Handshake request is no longer pending */

	sock_put(sk);

	fput(req->hr_file);

	return true;

}

EXPORT_SYMBOL(handshake_req_cancel);