[PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[Naman Jain]

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 <jostarks@microsoft.com>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
Cc: <stable@vger.kernel.org>
---
Changes since v1:
https://lore.kernel.org/all/20250818064846.271294-1-namjain@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

Re: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[Stephen Hemminger]

On Thu, 28 Aug 2025 10:12:00 +0530
Naman Jain <namjain@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 <jostarks@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> Cc: <stable@vger.kernel.org>

Makes sense. I think the logic got carried over from uio.
Does it need to make sure interrupt is masked by default to avoid
races at startup?

Re: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[Naman Jain]

On 8/28/2025 8:32 PM, Stephen Hemminger wrote:
> On Thu, 28 Aug 2025 10:12:00 +0530
> Naman Jain <namjain@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 <jostarks@microsoft.com>
>> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
>> Cc: <stable@vger.kernel.org>
> 
> Makes sense. I think the logic got carried over from uio.
> Does it need to make sure interrupt is masked by default to avoid
> races at startup?

No, initially I also figured that this would be required, and that's why
this was added in the first place. But my experiments with userspace
told me otherwise and I don't think this is required.

Thanks.

Regards,
Naman

RE: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[Long Li]

> Subject: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask
> 
> 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 <jostarks@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> Cc: <stable@vger.kernel.org>

Reviewed-by: Long Li <longli@microsoft.com>

> ---
> Changes since v1:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.
> kernel.org%2Fall%2F20250818064846.271294-1-
> namjain%40linux.microsoft.com%2F&data=05%7C02%7Clongli%40microsoft
> .com%7Cd254da4dfccd4050923f08dde5ed4153%7C72f988bf86f141af91a
> b2d7cd011db47%7C1%7C0%7C638919529361971491%7CUnknown%7CT
> WFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJX
> aW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=75
> A%2BJu5gaUZhYuBXDZEyKBRgJlsnaUenzL3wFOngMnU%3D&reserved=0
> * 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

RE: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[Michael Kelley]

From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, August 27, 2025 9:42 PM
> 
> 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 <jostarks@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> Cc: <stable@vger.kernel.org>

Reviewed-by: Michael Kelley <mhklinux@outlook.com>

> ---
> Changes since v1:
> https://lore.kernel.org/all/20250818064846.271294-1-namjain@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

Re: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt mask

[Tianyu Lan]

On Thu, Aug 28, 2025 at 12:42 PM Naman Jain <namjain@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 <jostarks@microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> Cc: <stable@vger.kernel.org>
> ---
> Changes since v1:
> https://lore.kernel.org/all/20250818064846.271294-1-namjain@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 <tiala@microsoft.com>
Tested-by: Tianyu Lan <tiala@microsoft.com>
--
Thanks

Tianyu Lan