diff mbox series

[2/2] net: RDMA: don't expose hw_stats into non-init net namespaces

Message ID 20250226033526.2769817-2-roman.gushchin@linux.dev (mailing list archive)
State New
Headers show
Series [1/2] net: RDMA: explicitly enumerate ib_device attribute groups | expand

Commit Message

Roman Gushchin Feb. 26, 2025, 3:35 a.m. UTC
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@linux.dev>
Cc: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Leon Romanovsky <leon@kernel.org>
Cc: Maher Sanalla <msanalla@nvidia.com>
Cc: Parav Pandit <parav@mellanox.com>
Cc: linux-rdma@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
 drivers/infiniband/core/device.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Parav Pandit Feb. 26, 2025, 3:55 a.m. UTC | #1
> From: Roman Gushchin <roman.gushchin@linux.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@linux.dev>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Leon Romanovsky <leon@kernel.org>
> Cc: Maher Sanalla <msanalla@nvidia.com>
> Cc: Parav Pandit <parav@mellanox.com>
> Cc: linux-rdma@vger.kernel.org
> Cc: linux-kernel@vger.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
diff mbox series

Patch

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);
+
 	/* 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;
+
 	device_initialize(&coredev->dev);
 	coredev->owner = dev;
 	INIT_LIST_HEAD(&coredev->port_list);