linux-kernel - RE: [PATCH 6.12] Drivers: hv: Make the sysfs node size for the ring buffer dynamic

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID:
 <SN6PR02MB41579080792040E166B5EB69D425A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Tue, 29 Jul 2025 18:39:45 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Naman Jain <namjain@...ux.microsoft.com>, "stable@...r.kernel.org"
	<stable@...r.kernel.org>, Wei Liu <wei.liu@...nel.org>, Dexuan Cui
	<decui@...rosoft.com>
CC: "K . Y . Srinivasan" <kys@...rosoft.com>, Haiyang Zhang
	<haiyangz@...rosoft.com>, Long Li <longli@...rosoft.com>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, "linux@...ssschuh.net" <linux@...ssschuh.net>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 6.12] Drivers: hv: Make the sysfs node size for the ring
 buffer dynamic

From: Naman Jain <namjain@...ux.microsoft.com> Sent: Wednesday, July 23, 2025 12:02 AM
> 
> The ring buffer size varies across VMBus channels. The size of sysfs
> node for the ring buffer is currently hardcoded to 4 MB. Userspace
> clients either use fstat() or hardcode this size for doing mmap().
> To address this, make the sysfs node size dynamic to reflect the
> actual ring buffer size for each channel. This will ensure that
> fstat() on ring sysfs node always returns the correct size of
> ring buffer.
> 
> This is a backport of the upstream commit
> 65995e97a1ca ("Drivers: hv: Make the sysfs node size for the ring buffer dynamic")
> with modifications, as the original patch has missing dependencies on
> kernel v6.12.x. The structure "struct attribute_group" does not have
> bin_size field in v6.12.x kernel so the logic of configuring size of
> sysfs node for ring buffer has been moved to
> vmbus_chan_bin_attr_is_visible().
> 
> Original change was not a fix, but it needs to be backported to fix size
> related discrepancy caused by the commit mentioned in Fixes tag.
> 
> Fixes: bf1299797c3c ("uio_hv_generic: Align ring size to system page")
> Cc: <stable@...r.kernel.org> # 6.12.x
> Signed-off-by: Naman Jain <namjain@...ux.microsoft.com>
> ---
> 
> This change won't apply on older kernels currently due to missing
> dependencies. I will take care of them after this goes in.
> 
> I did not retain any Reviewed-by or Tested-by tags, since the code has
> changed completely, while the functionality remains same.
> Requesting Michael, Dexuan, Wei to please review again.
> 
> ---
>  drivers/hv/vmbus_drv.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 1f519e925f06..616e63fb2f15 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1810,7 +1810,6 @@ static struct bin_attribute chan_attr_ring_buffer = {
>  		.name = "ring",
>  		.mode = 0600,
>  	},
> -	.size = 2 * SZ_2M,
>  	.mmap = hv_mmap_ring_buffer_wrapper,
>  };
>  static struct attribute *vmbus_chan_attrs[] = {
> @@ -1866,6 +1865,7 @@ static umode_t vmbus_chan_bin_attr_is_visible(struct kobject *kobj,
>  	/* Hide ring attribute if channel's ring_sysfs_visible is set to false */
>  	if (attr ==  &chan_attr_ring_buffer && !channel->ring_sysfs_visible)
>  		return 0;
> +	attr->size = channel->ringbuffer_pagecount << PAGE_SHIFT;

Suppose a VM has two devices using UIO, such as DPDK network device with
a 2MiB ring buffer, and an fcopy device with a 16KiB ring buffer. Both devices
will be referencing the same static instance of chan_attr_ring_buffer, and the
.size field it contains. The above statement will change that .size field
between 2MiB and 16KiB as the /sys entries are initially populated, and as
the visibility is changed if the devices are removed and re-instantiated (which
is much more likely for fcopy than for netvsc). That changing of the .size
value will probably work most of the time, but it's racy if two devices with
different ring buffer sizes get instantiated or re-instantiated at the same time.

Unfortunately, I don't see a fix, short of backporting support for the
.bin_size function, as this is exactly the problem that function solves.

Michael

> 
>  	return attr->attr.mode;
>  }
> --
> 2.34.1