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

Thomas Weißschuh posted 5 patches 1 year ago
[PATCH 5/5] s390/pkey: Constify 'struct bin_attribute'
Posted by Thomas Weißschuh 1 year ago
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>
---
 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
@@ -184,7 +184,7 @@ static ssize_t pkey_protkey_hmac_attr_read(u32 keytype, char *buf,
 
 static ssize_t protkey_aes_128_read(struct file *filp,
 				    struct kobject *kobj,
-				    struct bin_attribute *attr,
+				    const struct bin_attribute *attr,
 				    char *buf, loff_t off,
 				    size_t count)
 {
@@ -194,7 +194,7 @@ static ssize_t protkey_aes_128_read(struct file *filp,
 
 static ssize_t protkey_aes_192_read(struct file *filp,
 				    struct kobject *kobj,
-				    struct bin_attribute *attr,
+				    const struct bin_attribute *attr,
 				    char *buf, loff_t off,
 				    size_t count)
 {
@@ -204,7 +204,7 @@ static ssize_t protkey_aes_192_read(struct file *filp,
 
 static ssize_t protkey_aes_256_read(struct file *filp,
 				    struct kobject *kobj,
-				    struct bin_attribute *attr,
+				    const struct bin_attribute *attr,
 				    char *buf, loff_t off,
 				    size_t count)
 {
@@ -214,7 +214,7 @@ static ssize_t protkey_aes_256_read(struct file *filp,
 
 static ssize_t protkey_aes_128_xts_read(struct file *filp,
 					struct kobject *kobj,
-					struct bin_attribute *attr,
+					const struct bin_attribute *attr,
 					char *buf, loff_t off,
 					size_t count)
 {
@@ -224,7 +224,7 @@ static ssize_t protkey_aes_128_xts_read(struct file *filp,
 
 static ssize_t protkey_aes_256_xts_read(struct file *filp,
 					struct kobject *kobj,
-					struct bin_attribute *attr,
+					const struct bin_attribute *attr,
 					char *buf, loff_t off,
 					size_t count)
 {
@@ -234,7 +234,7 @@ static ssize_t protkey_aes_256_xts_read(struct file *filp,
 
 static ssize_t protkey_aes_xts_128_read(struct file *filp,
 					struct kobject *kobj,
-					struct bin_attribute *attr,
+					const struct bin_attribute *attr,
 					char *buf, loff_t off,
 					size_t count)
 {
@@ -244,7 +244,7 @@ static ssize_t protkey_aes_xts_128_read(struct file *filp,
 
 static ssize_t protkey_aes_xts_256_read(struct file *filp,
 					struct kobject *kobj,
-					struct bin_attribute *attr,
+					const struct bin_attribute *attr,
 					char *buf, loff_t off,
 					size_t count)
 {
@@ -254,7 +254,7 @@ static ssize_t protkey_aes_xts_256_read(struct file *filp,
 
 static ssize_t protkey_hmac_512_read(struct file *filp,
 				     struct kobject *kobj,
-				     struct bin_attribute *attr,
+				     const struct bin_attribute *attr,
 				     char *buf, loff_t off,
 				     size_t count)
 {
@@ -264,7 +264,7 @@ static ssize_t protkey_hmac_512_read(struct file *filp,
 
 static ssize_t protkey_hmac_1024_read(struct file *filp,
 				      struct kobject *kobj,
-				      struct bin_attribute *attr,
+				      const struct bin_attribute *attr,
 				      char *buf, loff_t off,
 				      size_t count)
 {
@@ -272,17 +272,17 @@ static ssize_t protkey_hmac_1024_read(struct file *filp,
 					   buf, off, count);
 }
 
-static BIN_ATTR_RO(protkey_aes_128, sizeof(struct protaeskeytoken));
-static BIN_ATTR_RO(protkey_aes_192, sizeof(struct protaeskeytoken));
-static BIN_ATTR_RO(protkey_aes_256, sizeof(struct protaeskeytoken));
-static BIN_ATTR_RO(protkey_aes_128_xts, 2 * sizeof(struct protaeskeytoken));
-static BIN_ATTR_RO(protkey_aes_256_xts, 2 * sizeof(struct protaeskeytoken));
-static BIN_ATTR_RO(protkey_aes_xts_128, sizeof(struct protkeytoken) + 64);
-static BIN_ATTR_RO(protkey_aes_xts_256, sizeof(struct protkeytoken) + 96);
-static BIN_ATTR_RO(protkey_hmac_512, sizeof(struct protkeytoken) + 96);
-static BIN_ATTR_RO(protkey_hmac_1024, sizeof(struct protkeytoken) + 160);
-
-static struct bin_attribute *protkey_attrs[] = {
+static const BIN_ATTR_RO(protkey_aes_128, sizeof(struct protaeskeytoken));
+static const BIN_ATTR_RO(protkey_aes_192, sizeof(struct protaeskeytoken));
+static const BIN_ATTR_RO(protkey_aes_256, sizeof(struct protaeskeytoken));
+static const BIN_ATTR_RO(protkey_aes_128_xts, 2 * sizeof(struct protaeskeytoken));
+static const BIN_ATTR_RO(protkey_aes_256_xts, 2 * sizeof(struct protaeskeytoken));
+static const BIN_ATTR_RO(protkey_aes_xts_128, sizeof(struct protkeytoken) + 64);
+static const BIN_ATTR_RO(protkey_aes_xts_256, sizeof(struct protkeytoken) + 96);
+static const BIN_ATTR_RO(protkey_hmac_512, sizeof(struct protkeytoken) + 96);
+static const BIN_ATTR_RO(protkey_hmac_1024, sizeof(struct protkeytoken) + 160);
+
+static const struct bin_attribute *const protkey_attrs[] = {
 	&bin_attr_protkey_aes_128,
 	&bin_attr_protkey_aes_192,
 	&bin_attr_protkey_aes_256,
@@ -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,
 };
 
 /*
@@ -341,7 +341,7 @@ static ssize_t pkey_ccadata_aes_attr_read(u32 keytype, bool is_xts, char *buf,
 
 static ssize_t ccadata_aes_128_read(struct file *filp,
 				    struct kobject *kobj,
-				    struct bin_attribute *attr,
+				    const struct bin_attribute *attr,
 				    char *buf, loff_t off,
 				    size_t count)
 {
@@ -351,7 +351,7 @@ static ssize_t ccadata_aes_128_read(struct file *filp,
 
 static ssize_t ccadata_aes_192_read(struct file *filp,
 				    struct kobject *kobj,
-				    struct bin_attribute *attr,
+				    const struct bin_attribute *attr,
 				    char *buf, loff_t off,
 				    size_t count)
 {
@@ -361,7 +361,7 @@ static ssize_t ccadata_aes_192_read(struct file *filp,
 
 static ssize_t ccadata_aes_256_read(struct file *filp,
 				    struct kobject *kobj,
-				    struct bin_attribute *attr,
+				    const struct bin_attribute *attr,
 				    char *buf, loff_t off,
 				    size_t count)
 {
@@ -371,7 +371,7 @@ static ssize_t ccadata_aes_256_read(struct file *filp,
 
 static ssize_t ccadata_aes_128_xts_read(struct file *filp,
 					struct kobject *kobj,
-					struct bin_attribute *attr,
+					const struct bin_attribute *attr,
 					char *buf, loff_t off,
 					size_t count)
 {
@@ -381,7 +381,7 @@ static ssize_t ccadata_aes_128_xts_read(struct file *filp,
 
 static ssize_t ccadata_aes_256_xts_read(struct file *filp,
 					struct kobject *kobj,
-					struct bin_attribute *attr,
+					const struct bin_attribute *attr,
 					char *buf, loff_t off,
 					size_t count)
 {
@@ -389,13 +389,13 @@ static ssize_t ccadata_aes_256_xts_read(struct file *filp,
 					  off, count);
 }
 
-static BIN_ATTR_RO(ccadata_aes_128, sizeof(struct secaeskeytoken));
-static BIN_ATTR_RO(ccadata_aes_192, sizeof(struct secaeskeytoken));
-static BIN_ATTR_RO(ccadata_aes_256, sizeof(struct secaeskeytoken));
-static BIN_ATTR_RO(ccadata_aes_128_xts, 2 * sizeof(struct secaeskeytoken));
-static BIN_ATTR_RO(ccadata_aes_256_xts, 2 * sizeof(struct secaeskeytoken));
+static const BIN_ATTR_RO(ccadata_aes_128, sizeof(struct secaeskeytoken));
+static const BIN_ATTR_RO(ccadata_aes_192, sizeof(struct secaeskeytoken));
+static const BIN_ATTR_RO(ccadata_aes_256, sizeof(struct secaeskeytoken));
+static const BIN_ATTR_RO(ccadata_aes_128_xts, 2 * sizeof(struct secaeskeytoken));
+static const BIN_ATTR_RO(ccadata_aes_256_xts, 2 * sizeof(struct secaeskeytoken));
 
-static struct bin_attribute *ccadata_attrs[] = {
+static const struct bin_attribute *const ccadata_attrs[] = {
 	&bin_attr_ccadata_aes_128,
 	&bin_attr_ccadata_aes_192,
 	&bin_attr_ccadata_aes_256,
@@ -404,9 +404,9 @@ static struct bin_attribute *ccadata_attrs[] = {
 	NULL
 };
 
-static struct attribute_group ccadata_attr_group = {
-	.name	   = "ccadata",
-	.bin_attrs = ccadata_attrs,
+static const struct attribute_group ccadata_attr_group = {
+	.name	       = "ccadata",
+	.bin_attrs_new = ccadata_attrs,
 };
 
 #define CCACIPHERTOKENSIZE	(sizeof(struct cipherkeytoken) + 80)
@@ -455,7 +455,7 @@ static ssize_t pkey_ccacipher_aes_attr_read(enum pkey_key_size keybits,
 
 static ssize_t ccacipher_aes_128_read(struct file *filp,
 				      struct kobject *kobj,
-				      struct bin_attribute *attr,
+				      const struct bin_attribute *attr,
 				      char *buf, loff_t off,
 				      size_t count)
 {
@@ -465,7 +465,7 @@ static ssize_t ccacipher_aes_128_read(struct file *filp,
 
 static ssize_t ccacipher_aes_192_read(struct file *filp,
 				      struct kobject *kobj,
-				      struct bin_attribute *attr,
+				      const struct bin_attribute *attr,
 				      char *buf, loff_t off,
 				      size_t count)
 {
@@ -475,7 +475,7 @@ static ssize_t ccacipher_aes_192_read(struct file *filp,
 
 static ssize_t ccacipher_aes_256_read(struct file *filp,
 				      struct kobject *kobj,
-				      struct bin_attribute *attr,
+				      const struct bin_attribute *attr,
 				      char *buf, loff_t off,
 				      size_t count)
 {
@@ -485,7 +485,7 @@ static ssize_t ccacipher_aes_256_read(struct file *filp,
 
 static ssize_t ccacipher_aes_128_xts_read(struct file *filp,
 					  struct kobject *kobj,
-					  struct bin_attribute *attr,
+					  const struct bin_attribute *attr,
 					  char *buf, loff_t off,
 					  size_t count)
 {
@@ -495,7 +495,7 @@ static ssize_t ccacipher_aes_128_xts_read(struct file *filp,
 
 static ssize_t ccacipher_aes_256_xts_read(struct file *filp,
 					  struct kobject *kobj,
-					  struct bin_attribute *attr,
+					  const struct bin_attribute *attr,
 					  char *buf, loff_t off,
 					  size_t count)
 {
@@ -503,13 +503,13 @@ static ssize_t ccacipher_aes_256_xts_read(struct file *filp,
 					    off, count);
 }
 
-static BIN_ATTR_RO(ccacipher_aes_128, CCACIPHERTOKENSIZE);
-static BIN_ATTR_RO(ccacipher_aes_192, CCACIPHERTOKENSIZE);
-static BIN_ATTR_RO(ccacipher_aes_256, CCACIPHERTOKENSIZE);
-static BIN_ATTR_RO(ccacipher_aes_128_xts, 2 * CCACIPHERTOKENSIZE);
-static BIN_ATTR_RO(ccacipher_aes_256_xts, 2 * CCACIPHERTOKENSIZE);
+static const BIN_ATTR_RO(ccacipher_aes_128, CCACIPHERTOKENSIZE);
+static const BIN_ATTR_RO(ccacipher_aes_192, CCACIPHERTOKENSIZE);
+static const BIN_ATTR_RO(ccacipher_aes_256, CCACIPHERTOKENSIZE);
+static const BIN_ATTR_RO(ccacipher_aes_128_xts, 2 * CCACIPHERTOKENSIZE);
+static const BIN_ATTR_RO(ccacipher_aes_256_xts, 2 * CCACIPHERTOKENSIZE);
 
-static struct bin_attribute *ccacipher_attrs[] = {
+static const struct bin_attribute *const ccacipher_attrs[] = {
 	&bin_attr_ccacipher_aes_128,
 	&bin_attr_ccacipher_aes_192,
 	&bin_attr_ccacipher_aes_256,
@@ -518,9 +518,9 @@ static struct bin_attribute *ccacipher_attrs[] = {
 	NULL
 };
 
-static struct attribute_group ccacipher_attr_group = {
-	.name	   = "ccacipher",
-	.bin_attrs = ccacipher_attrs,
+static const struct attribute_group ccacipher_attr_group = {
+	.name	       = "ccacipher",
+	.bin_attrs_new = ccacipher_attrs,
 };
 
 /*
@@ -570,7 +570,7 @@ static ssize_t pkey_ep11_aes_attr_read(enum pkey_key_size keybits,
 
 static ssize_t ep11_aes_128_read(struct file *filp,
 				 struct kobject *kobj,
-				 struct bin_attribute *attr,
+				 const struct bin_attribute *attr,
 				 char *buf, loff_t off,
 				 size_t count)
 {
@@ -580,7 +580,7 @@ static ssize_t ep11_aes_128_read(struct file *filp,
 
 static ssize_t ep11_aes_192_read(struct file *filp,
 				 struct kobject *kobj,
-				 struct bin_attribute *attr,
+				 const struct bin_attribute *attr,
 				 char *buf, loff_t off,
 				 size_t count)
 {
@@ -590,7 +590,7 @@ static ssize_t ep11_aes_192_read(struct file *filp,
 
 static ssize_t ep11_aes_256_read(struct file *filp,
 				 struct kobject *kobj,
-				 struct bin_attribute *attr,
+				 const struct bin_attribute *attr,
 				 char *buf, loff_t off,
 				 size_t count)
 {
@@ -600,7 +600,7 @@ static ssize_t ep11_aes_256_read(struct file *filp,
 
 static ssize_t ep11_aes_128_xts_read(struct file *filp,
 				     struct kobject *kobj,
-				     struct bin_attribute *attr,
+				     const struct bin_attribute *attr,
 				     char *buf, loff_t off,
 				     size_t count)
 {
@@ -610,7 +610,7 @@ static ssize_t ep11_aes_128_xts_read(struct file *filp,
 
 static ssize_t ep11_aes_256_xts_read(struct file *filp,
 				     struct kobject *kobj,
-				     struct bin_attribute *attr,
+				     const struct bin_attribute *attr,
 				     char *buf, loff_t off,
 				     size_t count)
 {
@@ -618,13 +618,13 @@ static ssize_t ep11_aes_256_xts_read(struct file *filp,
 				       off, count);
 }
 
-static BIN_ATTR_RO(ep11_aes_128, MAXEP11AESKEYBLOBSIZE);
-static BIN_ATTR_RO(ep11_aes_192, MAXEP11AESKEYBLOBSIZE);
-static BIN_ATTR_RO(ep11_aes_256, MAXEP11AESKEYBLOBSIZE);
-static BIN_ATTR_RO(ep11_aes_128_xts, 2 * MAXEP11AESKEYBLOBSIZE);
-static BIN_ATTR_RO(ep11_aes_256_xts, 2 * MAXEP11AESKEYBLOBSIZE);
+static const BIN_ATTR_RO(ep11_aes_128, MAXEP11AESKEYBLOBSIZE);
+static const BIN_ATTR_RO(ep11_aes_192, MAXEP11AESKEYBLOBSIZE);
+static const BIN_ATTR_RO(ep11_aes_256, MAXEP11AESKEYBLOBSIZE);
+static const BIN_ATTR_RO(ep11_aes_128_xts, 2 * MAXEP11AESKEYBLOBSIZE);
+static const BIN_ATTR_RO(ep11_aes_256_xts, 2 * MAXEP11AESKEYBLOBSIZE);
 
-static struct bin_attribute *ep11_attrs[] = {
+static const struct bin_attribute *const ep11_attrs[] = {
 	&bin_attr_ep11_aes_128,
 	&bin_attr_ep11_aes_192,
 	&bin_attr_ep11_aes_256,
@@ -633,9 +633,9 @@ static struct bin_attribute *ep11_attrs[] = {
 	NULL
 };
 
-static struct attribute_group ep11_attr_group = {
+static const struct attribute_group ep11_attr_group = {
 	.name	   = "ep11",
-	.bin_attrs = ep11_attrs,
+	.bin_attrs_new = ep11_attrs,
 };
 
 const struct attribute_group *pkey_attr_groups[] = {

-- 
2.47.1

Re: [PATCH 5/5] s390/pkey: Constify 'struct bin_attribute'
Posted by Holger Dengler 1 year ago
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

Re: [PATCH 5/5] s390/pkey: Constify 'struct bin_attribute'
Posted by Alexander Gordeev 11 months, 2 weeks ago
On Thu, Dec 12, 2024 at 04:03:18PM +0100, Holger Dengler wrote:
> 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>

Hi Harald,

Would you like to pull this patch via the crypto or s390 tree?

Thanks!
Re: [PATCH 5/5] s390/pkey: Constify 'struct bin_attribute'
Posted by Harald Freudenberger 11 months, 1 week ago
On 2025-01-03 15:21, Alexander Gordeev wrote:
> On Thu, Dec 12, 2024 at 04:03:18PM +0100, Holger Dengler wrote:
>> 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>
> 
> Hi Harald,
> 
> Would you like to pull this patch via the crypto or s390 tree?
> 
> Thanks!

Hi Alexander
If you don't mind then take it together with the other patches as part 
of the s390 tree.