On Thu, Aug 28, 2025 at 12:42 PM Naman Jain <namj...@linux.microsoft.com> wrote:
>
> Remove the logic to set interrupt mask by default in uio_hv_generic
> driver as the interrupt mask value is supposed to be controlled
> completely by the user space. If the mask bit gets changed
> by the driver, concurrently with user mode operating on the ring,
> the mask bit may be set when it is supposed to be clear, and the
> user-mode driver will miss an interrupt which will cause a hang.
>
> For eg- when the driver sets inbound ring buffer interrupt mask to 1,
> the host does not interrupt the guest on the UIO VMBus channel.
> However, setting the mask does not prevent the host from putting a
> message in the inbound ring buffer. So let’s assume that happens,
> the host puts a message into the ring buffer but does not interrupt.
>
> Subsequently, the user space code in the guest sets the inbound ring
> buffer interrupt mask to 0, saying “Hey, I’m ready for interrupts”.
> User space code then calls pread() to wait for an interrupt.
> Then one of two things happens:
>
> * The host never sends another message. So the pread() waits forever.
> * The host does send another message. But because there’s already a
> message in the ring buffer, it doesn’t generate an interrupt.
> This is the correct behavior, because the host should only send an
> interrupt when the inbound ring buffer transitions from empty to
> not-empty. Adding an additional message to a ring buffer that is not
> empty is not supposed to generate an interrupt on the guest.
> Since the guest is waiting in pread() and not removing messages from
> the ring buffer, the pread() waits forever.
>
> This could be easily reproduced in hv_fcopy_uio_daemon if we delay
> setting interrupt mask to 0.
>
> Similarly if hv_uio_channel_cb() sets the interrupt_mask to 1,
> there’s a race condition. Once user space empties the inbound ring
> buffer, but before user space sets interrupt_mask to 0, the host could
> put another message in the ring buffer but it wouldn’t interrupt.
> Then the next pread() would hang.
>
> Fix these by removing all instances where interrupt_mask is changed,
> while keeping the one in set_event() unchanged to enable userspace
> control the interrupt mask by writing 0/1 to /dev/uioX.
>
> Fixes: 95096f2fbd10 ("uio-hv-generic: new userspace i/o driver for VMBus")
> Suggested-by: John Starks <josta...@microsoft.com>
> Signed-off-by: Naman Jain <namj...@linux.microsoft.com>
> Cc: <sta...@vger.kernel.org>
> ---
> Changes since v1:
> https://lore.kernel.org/all/20250818064846.271294-1-namj...@linux.microsoft.com/
> * Added Fixes and Cc stable tags.
> ---
> drivers/uio/uio_hv_generic.c | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/uio/uio_hv_generic.c b/drivers/uio/uio_hv_generic.c
> index f19efad4d6f8..3f8e2e27697f 100644
> --- a/drivers/uio/uio_hv_generic.c
> +++ b/drivers/uio/uio_hv_generic.c
> @@ -111,7 +111,6 @@ static void hv_uio_channel_cb(void *context)
> struct hv_device *hv_dev;
> struct hv_uio_private_data *pdata;
>
> - chan->inbound.ring_buffer->interrupt_mask = 1;
> virt_mb();
>
> /*
> @@ -183,8 +182,6 @@ hv_uio_new_channel(struct vmbus_channel *new_sc)
> return;
> }
>
> - /* Disable interrupts on sub channel */
> - new_sc->inbound.ring_buffer->interrupt_mask = 1;
> set_channel_read_mode(new_sc, HV_CALL_ISR);
> ret = hv_create_ring_sysfs(new_sc, hv_uio_ring_mmap);
> if (ret) {
> @@ -227,9 +224,7 @@ hv_uio_open(struct uio_info *info, struct inode *inode)
>
> ret = vmbus_connect_ring(dev->channel,
> hv_uio_channel_cb, dev->channel);
> - if (ret == 0)
> - dev->channel->inbound.ring_buffer->interrupt_mask = 1;
> - else
> + if (ret)
> atomic_dec(&pdata->refcnt);
>
> return ret;
> --
> 2.34.1
>
>
Reviewed-by: Tianyu Lan <ti...@microsoft.com>
Tested-by: Tianyu Lan <ti...@microsoft.com>
--
Thanks
Tianyu Lan
