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

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID:
 <DS3PR21MB573592E3AF6599D9DD751BFCCE3AA@DS3PR21MB5735.namprd21.prod.outlook.com>
Date: Fri, 29 Aug 2025 14:17:30 +0000
From: Long Li <longli@...rosoft.com>
To: Naman Jain <namjain@...ux.microsoft.com>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, KY Srinivasan <kys@...rosoft.com>, Haiyang
 Zhang <haiyangz@...rosoft.com>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui
	<decui@...rosoft.com>, Stephen Hemminger <stephen@...workplumber.org>
CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Michael Kelley
	<mhklinux@...look.com>, "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v2] uio_hv_generic: Let userspace take care of interrupt
 mask

> 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@...rosoft.com>
> Signed-off-by: Naman Jain <namjain@...ux.microsoft.com>
> Cc: <stable@...r.kernel.org>

Reviewed-by: Long Li <longli@...rosoft.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