linux-kernel - RE: [PATCH 1/2] net: RDMA: explicitly enumerate ib

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:
 <CY8PR12MB7195D37C2E3D6DF02B7AAA8ADCC22@CY8PR12MB7195.namprd12.prod.outlook.com>
Date: Wed, 26 Feb 2025 03:43:28 +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 1/2] net: RDMA: explicitly enumerate ib_device attribute
 groups



> From: Roman Gushchin <roman.gushchin@...ux.dev>
> Sent: Wednesday, February 26, 2025 9:05 AM
> 
> Explicitly enumerate ib_device's attribute groups.
> 
> 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 |  4 ++--
>  include/rdma/ib_verbs.h          | 14 ++++++++------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 0ded91f056f3..8dea307addf1 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -1404,8 +1404,8 @@ int ib_register_device(struct ib_device *device,
> const char *name,
>  		return ret;
>  	}
> 
> -	device->groups[0] = &ib_dev_attr_group;
> -	device->groups[1] = device->ops.device_group;
> +	device->groups[IB_ATTR_GROUP_DEV_ATTR] = &ib_dev_attr_group;
> +	device->groups[IB_ATTR_GROUP_DRIVER_ATTR] = device-
> >ops.device_group;
>  	ret = ib_setup_device_attrs(device);
This function may initialize the hw_stats at the index IB_ATTR_GROUP_DRIVER_ATTR when it is optional.
It runs through a for loop to find the first free entry.
This needs to be captured in the comment above, and it will have effect on your next fix patch.

>  	if (ret)
>  		goto cache_cleanup;
> diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h index
> b59bf30de430..9c4c4238e6fc 100644
> --- a/include/rdma/ib_verbs.h
> +++ b/include/rdma/ib_verbs.h
> @@ -2728,6 +2728,13 @@ struct ib_core_device {
>  	struct ib_device *owner; /* reach back to owner ib_device */  };
> 
> +enum ib_device_attr_group {
> +	IB_ATTR_GROUP_DEV_ATTR = 0,	/* Device attributes */
> +	IB_ATTR_GROUP_DRIVER_ATTR = 1,	/* Driver-provided attributes
> */
> +	IB_ATTR_GROUP_HW_STATS = 2,	/* hw_stats */
> +	IB_ATTR_GROUP_LAST,		/* NULL pointer terminating array */
> +};
> +
>  struct rdma_restrack_root;
>  struct ib_device {
>  	/* Do not access @dma_device directly from ULP nor from HW
> drivers. */ @@ -2761,12 +2768,7 @@ struct ib_device {
>  		struct ib_core_device	coredev;
>  	};
> 
> -	/* First group is for device attributes,
> -	 * Second group is for driver provided attributes (optional).
This text 'optional' is missing in above enums, please add it.
Also please add the optional text comment for hw_stats too, as its optional too.

> -	 * Third group is for the hw_stats
> -	 * It is a NULL terminated array.
> -	 */
> -	const struct attribute_group	*groups[4];
> +	const struct attribute_group	*groups[IB_ATTR_GROUP_LAST + 1];
> 
>  	u64			     uverbs_cmd_mask;
> 
> --
> 2.48.1.658.g4767266eb4-goog