RE: [RFC PATCH 1/6] hv_sock: Check hv_pkt_iter_first_raw()'s return value

From: Michael Kelley (LINUX)
Date: Thu Apr 14 2022 - 23:47:55 EST


From: Andrea Parri (Microsoft) <parri.andrea@xxxxxxxxx> 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@xxxxxxxxx>
> ---
> 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