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

From: Holger Dengler
Date: Thu Dec 12 2024 - 10:05:40 EST


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@xxxxxxxxxxxxxx>

Thanks for your contribution.

Tested-by: Holger Dengler <dengler@xxxxxxxxxxxxx>
Reviewed-by: Holger Dengler <dengler@xxxxxxxxxxxxx>

> ---
> 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@xxxxxxxxxxxxx