Re: [PATCH v2 2/7] vhost-user: Fix double-close on slave_read() error path

[Stefan Hajnoczi]

On Fri, Mar 12, 2021 at 10:22:07AM +0100, Greg Kurz wrote:
> Some message types, e.g. VHOST_USER_SLAVE_VRING_HOST_NOTIFIER_MSG,
> can convey file descriptors. These must be closed before returning
> from slave_read() to avoid being leaked. This can currently be done
> in two different places:
> 
> [1] just after the request has been processed
> 
> [2] on the error path, under the goto label err:
> 
> These path are supposed to be mutually exclusive but they are not
> actually. If the VHOST_USER_NEED_REPLY_MASK flag was passed and the
> sending of the reply fails, both [1] and [2] are performed with the
> same descriptor values. This can potentially cause subtle bugs if one
> of the descriptor was recycled by some other thread in the meantime.
> 
> This code duplication complicates rollback for no real good benefit.
> Do the closing in a unique place, under a new fdcleanup: goto label
> at the end of the function.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  hw/virtio/vhost-user.c | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

signature.asc
Description: PGP signature