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: <e1d394bd-93a6-4d8f-b7f9-fc01449df98a@t-8ch.de>
Date: Tue, 29 Jul 2025 20:46:59 +0200
From: Thomas Weißschuh <linux@...ssschuh.net>
To: Michael Kelley <mhklinux@...look.com>
Cc: 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>, 
	"K . Y . Srinivasan" <kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, 
	Long Li <longli@...rosoft.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	"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

On 2025-07-29 18:39:45+0000, Michael Kelley wrote:
> 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.

IIRC it works out in practice. While the global attribute instance is indeed
modified back-and-forth the size from it will be *copied* into kernfs
after each recalculation. So each attribute should get its own correct size.

> 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.

It should work out in practice. (I introduced the .bin_size function)

Thomas