Re: [PATCH 5/5] s390/pkey: Constify 'struct bin_attribute'

[Holger Dengler <dengler@linux.ibm.com>]

On 11/12/2024 18:54, Thomas Weißschuh wrote:
> The sysfs core now allows instances of 'struct bin_attribute' to be
> moved into read-only memory. Make use of that to protect them against
> accidental or malicious modifications.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>

Thanks for your contribution.

Tested-by: Holger Dengler <dengler@linux.ibm.com>
Reviewed-by: Holger Dengler <dengler@linux.ibm.com>

> ---
>  drivers/s390/crypto/pkey_sysfs.c | 128 +++++++++++++++++++--------------------
>  1 file changed, 64 insertions(+), 64 deletions(-)
> 
> diff --git a/drivers/s390/crypto/pkey_sysfs.c b/drivers/s390/crypto/pkey_sysfs.c
> index a4eb45803f5e6d6b17dec709e6068448973399f6..57edc97bafd29483eedc405d47eabe3d7f6c28fc 100644
> --- a/drivers/s390/crypto/pkey_sysfs.c
> +++ b/drivers/s390/crypto/pkey_sysfs.c
[...]
> @@ -295,9 +295,9 @@ static struct bin_attribute *protkey_attrs[] = {
>  	NULL
>  };
>  
> -static struct attribute_group protkey_attr_group = {
> -	.name	   = "protkey",
> -	.bin_attrs = protkey_attrs,
> +static const struct attribute_group protkey_attr_group = {
> +	.name	       = "protkey",
> +	.bin_attrs_new = protkey_attrs,

This is more a comment to 906c508afdca (\"sysfs: attribute_group: allow registration of const bin_attribute\") than to this patch:
Why have you named the pointer `bin_attrs_new` and not something meaningful e.g. `bin_attrs_const`? I know, it is already in the kernel, but I would highly recommend to rename the pointer in another patch.

[...]

-- 
Mit freundlichen Grüßen / Kind regards
Holger Dengler
--
IBM Systems, Linux on IBM Z Development
dengler@linux.ibm.com