linux-kernel - RE: [RFC PATCH 1/6] hv_sock: Check hv_pkt_iter_first

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

Message-ID: <PH0PR21MB3025FA1943D74A31E47B7F8FD7EE9@PH0PR21MB3025.namprd21.prod.outlook.com>
Date:   Fri, 15 Apr 2022 03:33:23 +0000
From:   "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To:     "Andrea Parri (Microsoft)" <parri.andrea@...il.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
        Stefano Garzarella <sgarzare@...hat.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH 1/6] hv_sock: Check hv_pkt_iter_first_raw()'s return
 value

From: Andrea Parri (Microsoft) <parri.andrea@...il.com> Sent: Wednesday, April 13, 2022 1:48 PM
> 
> The function returns NULL if the ring buffer has no enough space
> available for a packet descriptor.  The ring buffer's write_index

The first sentence wording is a bit scrambled.  I think you mean the
ring buffer doesn't contain enough readable bytes to constitute a
packet descriptor.

> is in memory which is shared with the Hyper-V host, its value is
> thus subject to being changed at any time.

This second sentence is true, but I'm not making the connection
with the code change below.   Evidently, there is some previous
check made to ensure that enough bytes are available to be
received when hvs_stream_dequeue() is called, so we assumed that
NULL could never be returned?  I looked but didn't find such a check, 
so maybe I didn't look carefully enough.  But now we are assuming
that Hyper-V might have invalidated that previous check by 
subsequently changing the write_index in a bogus way?  So now, NULL
could be returned when previously we assumed it couldn't.

Michael

> 
> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@...il.com>
> ---
>  net/vmw_vsock/hyperv_transport.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
> index e111e13b66604..943352530936e 100644
> --- a/net/vmw_vsock/hyperv_transport.c
> +++ b/net/vmw_vsock/hyperv_transport.c
> @@ -603,6 +603,8 @@ static ssize_t hvs_stream_dequeue(struct vsock_sock *vsk,
> struct msghdr *msg,
> 
>  	if (need_refill) {
>  		hvs->recv_desc = hv_pkt_iter_first_raw(hvs->chan);
> +		if (!hvs->recv_desc)
> +			return -ENOBUFS;
>  		ret = hvs_update_recv_data(hvs);
>  		if (ret)
>  			return ret;
> --
> 2.25.1