]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
net/handshake: Take a long-lived file reference at submit
authorChuck Lever <chuck.lever@oracle.com>
Mon, 25 May 2026 16:51:18 +0000 (12:51 -0400)
committerPaolo Abeni <pabeni@redhat.com>
Thu, 28 May 2026 11:35:31 +0000 (13:35 +0200)
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: Chuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260525-handshake-file-pin-v3-4-66c616906ead@oracle.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
net/handshake/handshake.h
net/handshake/netlink.c
net/handshake/request.c

index 2289b0e274f40a833af8d9aed0dbcce4a8e0bd73..da61cadd1ad3e7e1d0109abfd3e9c70c6b1c8761 100644 (file)
@@ -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);
 
index 039344979de934a352af6e0ed483797cacb1011b..1a5821eb7184708c25ebaf7aa888b473aa630271 100644 (file)
@@ -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);
        }
 }
index 97f9f82399499474919cfe85efb1cdfe85e1dec9..da064511ab86006c0ce9307fa2873c2f95e7b120 100644 (file)
@@ -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);
-       handshake_req_destroy(req);
+       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 @@ out_true:
 
        /* Handshake request is no longer pending */
        sock_put(sk);
+       fput(req->hr_file);
        return true;
 }
 EXPORT_SYMBOL(handshake_req_cancel);