Re: [PATCH 6.4 118/800] net/handshake: Unpin sock->file if a handshake is cancelled

Chuck Lever III
2023-07-17T17:56:03Z

> On Jul 17, 2023, at 1:10 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>
> On Sun, Jul 16, 2023 at 08:43:58PM +0000, Chuck Lever III wrote:
>>
>>
>>> On Jul 16, 2023, at 3:39 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>>>
>>> From: Chuck Lever <chuck.lever@oracle.com>
>>>
>>> [ Upstream commit f921bd41001ccff2249f5f443f2917f7ef937daf ]
>>>
>>> If user space never calls DONE, sock->file's reference count remains
>>> elevated. Enable sock->file to be freed eventually in this case.
>>>
>>> Reported-by: Jakub Kacinski <kuba@kernel.org>
>>> Fixes: 3b3009ea8abb ("net/handshake: Create a NETLINK service for handling handshake requests")
>>> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>>> ---
>>> net/handshake/handshake.h | 1 +
>>> net/handshake/request.c | 4 ++++
>>> 2 files changed, 5 insertions(+)
>>>
>>> diff --git a/net/handshake/handshake.h b/net/handshake/handshake.h
>>> index 4dac965c99df0..8aeaadca844fd 100644
>>> --- a/net/handshake/handshake.h
>>> +++ b/net/handshake/handshake.h
>>> @@ -31,6 +31,7 @@ struct handshake_req {
>>> struct list_head hr_list;
>>> struct rhash_head hr_rhash;
>>> unsigned long hr_flags;
>>> + struct file *hr_file;
>>> const struct handshake_proto *hr_proto;
>>> struct sock *hr_sk;
>>> void (*hr_odestruct)(struct sock *sk);
>>> diff --git a/net/handshake/request.c b/net/handshake/request.c
>>> index 94d5cef3e048b..d78d41abb3d99 100644
>>> --- a/net/handshake/request.c
>>> +++ b/net/handshake/request.c
>>> @@ -239,6 +239,7 @@ int handshake_req_submit(struct socket *sock, struct handshake_req *req,
>>> }
>>> req->hr_odestruct = req->hr_sk->sk_destruct;
>>> req->hr_sk->sk_destruct = handshake_sk_destruct;
>>> + req->hr_file = sock->file;
>>>
>>> ret = -EOPNOTSUPP;
>>> net = sock_net(req->hr_sk);
>>> @@ -334,6 +335,9 @@ bool handshake_req_cancel(struct sock *sk)
>>> return false;
>>> }
>>>
>>> + /* Request accepted and waiting for DONE */
>>> + fput(req->hr_file);
>>> +
>>> out_true:
>>> trace_handshake_cancel(net, req, sk);
>>>
>>> --
>>> 2.39.2
>>>
>>>
>>>
>>
>> Don't take this one. It's fixed by a later commit:
>>
>> 361b6889ae636926cdff517add240c3c8e24593a
>>
>> that reverts it.
>
> How? That commit is in 6.4 already, yet this commit, is from 6.5-rc1.

I do not see f921bd41001ccff2249f5f443f2917f7ef937daf in v6.5-rc2.
Whatever that is, it's not in upstream.

[cel@manet server-development]$ git log --pretty=oneline v6.5-rc2 -- net/handshake/
173780ff18a93298ca84224cc79df69f9cc198ce Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net
361b6889ae636926cdff517add240c3c8e24593a net/handshake: remove fput() that causes use-after-free
9b66ee06e5ca2698d0ba12a7ad7188cb724279e7 net: ynl: prefix uAPI header include with uapi/
26fb5480a27d34975cc2b680b77af189620dd740 net/handshake: Enable the SNI extension to work properly
1ce77c998f0415d7d9d91cb9bd7665e25c8f75f1 net/handshake: Unpin sock->file if a handshake is cancelled
fc490880e39d86c65ab2bcbd357af1950fa55e48 net/handshake: handshake_genl_notify() shouldn't ignore @flags
7afc6d0a107ffbd448c96eb2458b9e64a5af7860 net/handshake: Fix uninitialized local variable
7ea9c1ec66bc099b0bfba961a8a46dfe25d7d8e4 net/handshake: Fix handshake_dup() ref counting
a095326e2c0f33743ce8e887d5b90edf3f36cced net/handshake: Remove unneeded check from handshake_dup()
18c40a1cc1d990c51381ef48cd93fdb31d5cd903 net/handshake: Fix sock->file allocation
b21c7ba6d9a5532add3827a3b49f49cbc0cb9779 net/handshake: Squelch allocation warning during Kunit test
6aa445e39693bff9c98b12f960e66b4e18c7378b net/handshake: Fix section mismatch in handshake_exit
88232ec1ec5ecf4aa5de439cff3d5e2b7adcac93 net/handshake: Add Kunit tests for the handshake consumer API
2fd5532044a89d2403b543520b4902e196f7d165 net/handshake: Add a kernel API for requesting a TLSv1.3 handshake
3b3009ea8abb713b022d94fba95ec270cf6e7eae net/handshake: Create a NETLINK service for handling handshake requests
[cel@manet server-development]$

But I do see 1ce77c998f0415d7d9d91cb9bd7665e25c8f75f1 and the
commit that reverts it, 361b6889ae636926cdff517add240c3c8e24593a.

--
Chuck Lever