Re: [PATCH 0/2] vsock: fix use-after free and null-ptr-deref

[Michal Luczaj <mhal@rbox.co>]

On 2/18/25 09:35, Stefano Garzarella wrote:
> On Mon, Feb 17, 2025 at 08:45:57PM +0100, Michal Luczaj wrote:
>> On 2/17/25 12:18, Luigi Leonardi wrote:
>>> On Fri, Feb 14, 2025 at 06:53:54PM +0100, Luigi Leonardi wrote:
>>>> Hi all,
>>>>
>>>> This series contains two patches that are already available upstream:
>>>>
>>>> - The first commit fixes a use-after-free[1], but introduced a
>>>> null-ptr-deref[2].
>>>> - The second commit fixes it. [3]
>>>>
>>>> I suggested waiting for both of them to be merged upstream and then
>>>> applying them togheter to stable[4].
>>>>
>>>> It should be applied to:
>>>> - 6.13.y
>>>> - 6.12.y
>>>> - 6.6.y
>>>>
>>>> I will send another series for
>>>> - 6.1.y
>>>> - 5.15.y
>>>> - 5.10.y
>>>>
>>>> because of conflicts.
>>>>
>>>> [1]https://lore.kernel.org/all/20250128-vsock-transport-vs-autobind-v3-0-1cf57065b770@rbox.co/
>>>> [2]https://lore.kernel.org/all/67a09300.050a0220.d7c5a.008b.GAE@google.com/
>>>> [3]https://lore.kernel.org/all/20250210-vsock-linger-nullderef-v3-0-ef6244d02b54@rbox.co/
>>>> [4]https://lore.kernel.org/all/2025020644-unwitting-scary-3c0d@gregkh/
>>>>
>>>> Thanks,
>>>> Luigi
>>>>
>>>> ---
>>>> Michal Luczaj (2):
>>>>      vsock: Keep the binding until socket destruction
>>>>      vsock: Orphan socket after transport release
>>>>
>>>> net/vmw_vsock/af_vsock.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>> ---
>>>> base-commit: a1856aaa2ca74c88751f7d255dfa0c8c50fcc1ca
>>>> change-id: 20250214-linux-rolling-stable-d73f0bed815d
>>>>
>>>> Best regards,
>>>> -- Luigi Leonardi <leonardi@redhat.com>
>>>>
>>>
>>> Looks like I forgot to add my SoB to the commits, my bad.
>>>
>>> For all the other stable trees (6.1, 5.15 and 5.10), there are some
>>> conflicts due to some indentation changes introduced by 135ffc7 ("bpf,
>>> vsock: Invoke proto::close on close()"). Should I backport this commit
>>> too?  There is no real dependency on the commit in the Fixes tag
>>> ("vsock: support sockmap"). IMHO, this would help future backports,
>>> because of indentation conficts! Otherwise I can simply fix the patches.
>>> WDYT?
>>
>> Just a note: since sockmap does not support AF_VSOCK in those kernels <=
>> 6.1, backporting 135ffc7 would introduce a (no-op) callback function
>> vsock_close(), which would then be (unnecessarily) called on every
>> vsock_release().
>>
> 
> But this is the same behavior we have now upstream (without considering 
> sockmap), right?

Oh, right, that's true.

> Do you see any potential problems?

No, nothing I can think of.

Note however that the comment above vsock_close() ("Dummy callback required
by sockmap. See unconditional call of saved_close() in sock_map_close().")
becomes somewhat misleading :)