linux-kernel - RE: [PATCH 2/2] net: RDMA: don't expose hw

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:
 <CY8PR12MB71954B7C87C0667B57A29207DCC22@CY8PR12MB7195.namprd12.prod.outlook.com>
Date: Wed, 26 Feb 2025 03:55:37 +0000
From: Parav Pandit <parav@...lanox.com>
To: Roman Gushchin <roman.gushchin@...ux.dev>, Jason Gunthorpe <jgg@...pe.ca>
CC: Leon Romanovsky <leon@...nel.org>, Maher Sanalla <msanalla@...dia.com>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 2/2] net: RDMA: don't expose hw_stats into non-init net
 namespaces


> From: Roman Gushchin <roman.gushchin@...ux.dev>
> Sent: Wednesday, February 26, 2025 9:05 AM
> 
> Commit 467f432a521a ("RDMA/core: Split port and device counter sysfs
> attributes") accidentally exposed hw_counters to non-init net namespaces.
> 
> Fix this by hiding the IB_ATTR_GROUP_HW_STATS group when initializing a
> non-init rdma device.
> 
> Fixes: 467f432a521a ("RDMA/core: Split port and device counter sysfs
> attributes")
> Signed-off-by: Roman Gushchin <roman.gushchin@...ux.dev>
> Cc: Jason Gunthorpe <jgg@...pe.ca>
> Cc: Leon Romanovsky <leon@...nel.org>
> Cc: Maher Sanalla <msanalla@...dia.com>
> Cc: Parav Pandit <parav@...lanox.com>
> Cc: linux-rdma@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> ---
>  drivers/infiniband/core/device.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 8dea307addf1..bf4a016ccb9d 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -528,6 +528,8 @@ static struct class ib_class = {  static void
> rdma_init_coredev(struct ib_core_device *coredev,
>  			      struct ib_device *dev, struct net *net)  {
> +	bool is_full_dev = net_eq(net, &init_net);
> +
Instead of it, below is more elegant check because 
a. its limited to do comparison on core dev area and other generic structure.
b. reuses the infra used in sysfs.c to detect coredev.
c. in upcoming future, I plan to expand device creation in non init ns too, where it still will be coredev.
And this conficts with that idea, hinting that comparing in below manner is still more elegant.

bool is_full_dev = &device->coredev == coredev; 

>  	/* This BUILD_BUG_ON is intended to catch layout change
>  	 * of union of ib_core_device and device.
>  	 * dev must be the first element as ib_core and providers @@ -539,6
> +541,10 @@ static void rdma_init_coredev(struct ib_core_device *coredev,
> 
>  	coredev->dev.class = &ib_class;
>  	coredev->dev.groups = dev->groups;
> +
> +	if (!is_full_dev)
> +		coredev->dev.groups[IB_ATTR_GROUP_HW_STATS] = NULL;
> +
Static hard coding to HW_STATS enum is not enough because when IB_ATTR_GROUP_DRIVER_ATTR group is not used, 
ib_setup_device_attrs() will initialize HW_STATS at index = DRIVER_ATTR.
So you need to store the index of hw_stats group in the ib_device struct, so you can make it null for non_core_dev.

And if that can be done without introducing the new enums, applying the fix on previous stable kernels will be easy.
Otherwise, this patch needs to do to -rc and other previous patch to -next and its difficult.

>  	device_initialize(&coredev->dev);
>  	coredev->owner = dev;
>  	INIT_LIST_HEAD(&coredev->port_list);
> --
> 2.48.1.658.g4767266eb4-goog