netdev - Re: [PATCH] net/vmw_vsock: improve locking in vsock_connect

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] [day] [month] [year] [list]

Message-ID: <20210206165652.5bb69b67@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Sat, 6 Feb 2021 16:56:52 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Stefano Garzarella <sgarzare@...hat.com>
Cc:     Norbert Slusarek <nslusarek@....net>, alex.popov@...ux.com,
        eric.dumazet@...il.com, netdev@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH] net/vmw_vsock: improve locking in
 vsock_connect_timeout()

On Fri, 5 Feb 2021 18:19:51 +0100 Stefano Garzarella wrote:
> On Fri, Feb 05, 2021 at 03:32:02PM +0100, Norbert Slusarek wrote:
> >From: Norbert Slusarek <nslusarek@....net>
> >Date: Fri, 5 Feb 2021 13:14:05 +0100
> >Subject: [PATCH] net/vmw_vsock: improve locking in vsock_connect_timeout()
> >
> >A possible locking issue in vsock_connect_timeout() was recognized by
> >Eric Dumazet which might cause a null pointer dereference in
> >vsock_transport_cancel_pkt(). This patch assures that
> >vsock_transport_cancel_pkt() will be called within the lock, so a race
> >condition won't occur which could result in vsk->transport to be set to NULL.
> >
> >Fixes: 380feae0def7 ("vsock: cancel packets when failing to connect")  
> 
> I have a doubt about the tag to use, since until we introduced 
> transports in commit c0cfa2d8a788 ("vsock: add multi-transports 
> support") this issue didn't cause many problems.
> 
> But it must be said that in the commit 380feae0def7 ("vsock: cancel 
> packets when failing to connect") the vsock_transport_cancel_pkt() was 
> called with the lock held in vsock_stream_connect() and without lock in 
> vsock_connect_timeout(), so maybe this tag is okay.
> 
> Anyway, the patch LGTM:
> 
> Reviewed-by: Stefano Garzarella <sgarzare@...hat.com>

Applied, thanks!