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

Parav Pandit
2025-02-26T03:55:40Z

> 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