* [PATCH] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
@ 2025-05-13 15:47 Cedric Xing
2025-05-13 16:24 ` Thomas Weißschuh
0 siblings, 1 reply; 8+ messages in thread
From: Cedric Xing @ 2025-05-13 15:47 UTC (permalink / raw)
To: linux, dan.j.williams
Cc: sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo, aik,
suzuki.poulose, steven.price, lukas, greg, linux-next,
Cedric Xing
Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
the ro-after-init principle by making elements of bin_attrs pointing to
const.
To align with this change, introduce a temporary variable `bap` within the
initialization loop. This improves code clarity by explicitly marking the
initialization scope and eliminates the need for type casts after bin_attrs
was constified.
Signed-off-by: Cedric Xing <cedric.xing@intel•com>
---
drivers/virt/coco/tsm-mr.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/virt/coco/tsm-mr.c b/drivers/virt/coco/tsm-mr.c
index 1f0c43a516fb..a6362b144e8b 100644
--- a/drivers/virt/coco/tsm-mr.c
+++ b/drivers/virt/coco/tsm-mr.c
@@ -173,7 +173,7 @@ tsm_mr_create_attribute_group(const struct tsm_measurements *tm)
* so that we don't have to free MR names one-by-one in
* tsm_mr_free_attribute_group()
*/
- const struct bin_attribute * const *attrs __free(kfree) =
+ const struct bin_attribute **attrs __free(kfree) =
kzalloc(sizeof(*attrs) * (tm->nr_mrs + 1) + nlen, GFP_KERNEL);
struct tm_context *ctx __free(kfree) =
kzalloc(struct_size(ctx, mrs, tm->nr_mrs), GFP_KERNEL);
@@ -187,16 +187,14 @@ tsm_mr_create_attribute_group(const struct tsm_measurements *tm)
end = name + nlen;
for (size_t i = 0; i < tm->nr_mrs; ++i) {
- /* break const for init */
- struct bin_attribute **bas = (struct bin_attribute **)attrs;
+ struct bin_attribute *bap = &ctx->mrs[i];
- bas[i] = &ctx->mrs[i];
- sysfs_bin_attr_init(bas[i]);
+ sysfs_bin_attr_init(bap);
if (tm->mrs[i].mr_flags & TSM_MR_F_NOHASH)
- bas[i]->attr.name = tm->mrs[i].mr_name;
+ bap->attr.name = tm->mrs[i].mr_name;
else if (name < end) {
- bas[i]->attr.name = name;
+ bap->attr.name = name;
name += snprintf(name, end - name, "%s:%s",
tm->mrs[i].mr_name,
hash_algo_name[tm->mrs[i].mr_hash]);
@@ -206,21 +204,23 @@ tsm_mr_create_attribute_group(const struct tsm_measurements *tm)
/* check for duplicated MR definitions */
for (size_t j = 0; j < i; ++j)
- if (!strcmp(bas[i]->attr.name, bas[j]->attr.name))
+ if (!strcmp(bap->attr.name, attrs[j]->attr.name))
return ERR_PTR(-EINVAL);
if (tm->mrs[i].mr_flags & TSM_MR_F_READABLE) {
- bas[i]->attr.mode |= 0444;
- bas[i]->read_new = tm_digest_read;
+ bap->attr.mode |= 0444;
+ bap->read_new = tm_digest_read;
}
if (tm->mrs[i].mr_flags & TSM_MR_F_WRITABLE) {
- bas[i]->attr.mode |= 0200;
- bas[i]->write_new = tm_digest_write;
+ bap->attr.mode |= 0200;
+ bap->write_new = tm_digest_write;
}
- bas[i]->size = tm->mrs[i].mr_size;
- bas[i]->private = ctx;
+ bap->size = tm->mrs[i].mr_size;
+ bap->private = ctx;
+
+ attrs[i] = bap;
}
if (name != end)
base-commit: 7c3f259dfe03f5dcd898126602818a8fbe94d3c5
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 15:47 [PATCH] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase Cedric Xing
@ 2025-05-13 16:24 ` Thomas Weißschuh
2025-05-13 16:41 ` [PATCH v2] " Cedric Xing
0 siblings, 1 reply; 8+ messages in thread
From: Thomas Weißschuh @ 2025-05-13 16:24 UTC (permalink / raw)
To: Cedric Xing
Cc: dan.j.williams, sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo,
aik, suzuki.poulose, steven.price, lukas, greg, linux-next
On 2025-05-13 10:47:42-0500, Cedric Xing wrote:
> Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
> the ro-after-init principle by making elements of bin_attrs pointing to
> const.
>
> To align with this change, introduce a temporary variable `bap` within the
> initialization loop. This improves code clarity by explicitly marking the
> initialization scope and eliminates the need for type casts after bin_attrs
> was constified.
Please also switch to .bin_attrs_new instead of .bin_attrs.
To make sure that the code is compatible with the new logic.
> Signed-off-by: Cedric Xing <cedric.xing@intel•com>
> ---
> drivers/virt/coco/tsm-mr.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
<snip>
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 16:24 ` Thomas Weißschuh
@ 2025-05-13 16:41 ` Cedric Xing
2025-05-13 16:49 ` Thomas Weißschuh
2025-05-13 18:37 ` Dan Williams
0 siblings, 2 replies; 8+ messages in thread
From: Cedric Xing @ 2025-05-13 16:41 UTC (permalink / raw)
To: linux, dan.j.williams
Cc: sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo, aik,
suzuki.poulose, steven.price, lukas, greg, linux-next,
Cedric Xing
Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
the ro-after-init principle by making elements of bin_attrs_new pointing to
const.
To align with this change, introduce a temporary variable `bap` within the
initialization loop. This improves code clarity by explicitly marking the
initialization scope and eliminates the need for type casts when assigning
to bin_attrs_new.
Signed-off-by: Cedric Xing <cedric.xing@intel•com>
---
drivers/virt/coco/tsm-mr.c | 30 +++++++++++++++---------------
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/drivers/virt/coco/tsm-mr.c b/drivers/virt/coco/tsm-mr.c
index 1f0c43a516fb..feb30af90a20 100644
--- a/drivers/virt/coco/tsm-mr.c
+++ b/drivers/virt/coco/tsm-mr.c
@@ -173,7 +173,7 @@ tsm_mr_create_attribute_group(const struct tsm_measurements *tm)
* so that we don't have to free MR names one-by-one in
* tsm_mr_free_attribute_group()
*/
- const struct bin_attribute * const *attrs __free(kfree) =
+ const struct bin_attribute **attrs __free(kfree) =
kzalloc(sizeof(*attrs) * (tm->nr_mrs + 1) + nlen, GFP_KERNEL);
struct tm_context *ctx __free(kfree) =
kzalloc(struct_size(ctx, mrs, tm->nr_mrs), GFP_KERNEL);
@@ -187,16 +187,14 @@ tsm_mr_create_attribute_group(const struct tsm_measurements *tm)
end = name + nlen;
for (size_t i = 0; i < tm->nr_mrs; ++i) {
- /* break const for init */
- struct bin_attribute **bas = (struct bin_attribute **)attrs;
+ struct bin_attribute *bap = &ctx->mrs[i];
- bas[i] = &ctx->mrs[i];
- sysfs_bin_attr_init(bas[i]);
+ sysfs_bin_attr_init(bap);
if (tm->mrs[i].mr_flags & TSM_MR_F_NOHASH)
- bas[i]->attr.name = tm->mrs[i].mr_name;
+ bap->attr.name = tm->mrs[i].mr_name;
else if (name < end) {
- bas[i]->attr.name = name;
+ bap->attr.name = name;
name += snprintf(name, end - name, "%s:%s",
tm->mrs[i].mr_name,
hash_algo_name[tm->mrs[i].mr_hash]);
@@ -206,21 +204,23 @@ tsm_mr_create_attribute_group(const struct tsm_measurements *tm)
/* check for duplicated MR definitions */
for (size_t j = 0; j < i; ++j)
- if (!strcmp(bas[i]->attr.name, bas[j]->attr.name))
+ if (!strcmp(bap->attr.name, attrs[j]->attr.name))
return ERR_PTR(-EINVAL);
if (tm->mrs[i].mr_flags & TSM_MR_F_READABLE) {
- bas[i]->attr.mode |= 0444;
- bas[i]->read_new = tm_digest_read;
+ bap->attr.mode |= 0444;
+ bap->read_new = tm_digest_read;
}
if (tm->mrs[i].mr_flags & TSM_MR_F_WRITABLE) {
- bas[i]->attr.mode |= 0200;
- bas[i]->write_new = tm_digest_write;
+ bap->attr.mode |= 0200;
+ bap->write_new = tm_digest_write;
}
- bas[i]->size = tm->mrs[i].mr_size;
- bas[i]->private = ctx;
+ bap->size = tm->mrs[i].mr_size;
+ bap->private = ctx;
+
+ attrs[i] = bap;
}
if (name != end)
@@ -244,7 +244,7 @@ EXPORT_SYMBOL_GPL(tsm_mr_create_attribute_group);
void tsm_mr_free_attribute_group(const struct attribute_group *attr_grp)
{
if (!IS_ERR_OR_NULL(attr_grp)) {
- kfree(attr_grp->bin_attrs);
+ kfree(attr_grp->bin_attrs_new);
kfree(container_of(attr_grp, struct tm_context, agrp));
}
}
base-commit: 7c3f259dfe03f5dcd898126602818a8fbe94d3c5
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 16:41 ` [PATCH v2] " Cedric Xing
@ 2025-05-13 16:49 ` Thomas Weißschuh
2025-05-13 17:27 ` dan.j.williams
2025-05-13 18:37 ` Dan Williams
1 sibling, 1 reply; 8+ messages in thread
From: Thomas Weißschuh @ 2025-05-13 16:49 UTC (permalink / raw)
To: Cedric Xing
Cc: dan.j.williams, sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo,
aik, suzuki.poulose, steven.price, lukas, greg, linux-next
On 2025-05-13 11:41:54-0500, Cedric Xing wrote:
> Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
> the ro-after-init principle by making elements of bin_attrs_new pointing to
> const.
>
> To align with this change, introduce a temporary variable `bap` within the
> initialization loop. This improves code clarity by explicitly marking the
> initialization scope and eliminates the need for type casts when assigning
> to bin_attrs_new.
>
> Signed-off-by: Cedric Xing <cedric.xing@intel•com>
> ---
> drivers/virt/coco/tsm-mr.c | 30 +++++++++++++++---------------
> 1 file changed, 15 insertions(+), 15 deletions(-)
<snip>
> @@ -244,7 +244,7 @@ EXPORT_SYMBOL_GPL(tsm_mr_create_attribute_group);
> void tsm_mr_free_attribute_group(const struct attribute_group *attr_grp)
> {
> if (!IS_ERR_OR_NULL(attr_grp)) {
> - kfree(attr_grp->bin_attrs);
> + kfree(attr_grp->bin_attrs_new);
This is good, but the assignment also needs to be done to .bin_attrs_new.
That is the code I can find on LKML:
https://lore.kernel.org/lkml/20250506-tdx-rtmr-v6-1-ac6ff5e9d58a@intel.com/
> kfree(container_of(attr_grp, struct tm_context, agrp));
> }
> }
>
> base-commit: 7c3f259dfe03f5dcd898126602818a8fbe94d3c5
> --
> 2.43.0
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 16:49 ` Thomas Weißschuh
@ 2025-05-13 17:27 ` dan.j.williams
2025-05-13 17:29 ` dan.j.williams
0 siblings, 1 reply; 8+ messages in thread
From: dan.j.williams @ 2025-05-13 17:27 UTC (permalink / raw)
To: Thomas Weißschuh, Cedric Xing
Cc: dan.j.williams, sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo,
aik, suzuki.poulose, steven.price, lukas, greg, linux-next
Thomas Weißschuh wrote:
> On 2025-05-13 11:41:54-0500, Cedric Xing wrote:
> > Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
> > the ro-after-init principle by making elements of bin_attrs_new pointing to
> > const.
> >
> > To align with this change, introduce a temporary variable `bap` within the
> > initialization loop. This improves code clarity by explicitly marking the
> > initialization scope and eliminates the need for type casts when assigning
> > to bin_attrs_new.
> >
> > Signed-off-by: Cedric Xing <cedric.xing@intel•com>
> > ---
> > drivers/virt/coco/tsm-mr.c | 30 +++++++++++++++---------------
> > 1 file changed, 15 insertions(+), 15 deletions(-)
>
> <snip>
>
> > @@ -244,7 +244,7 @@ EXPORT_SYMBOL_GPL(tsm_mr_create_attribute_group);
> > void tsm_mr_free_attribute_group(const struct attribute_group *attr_grp)
> > {
> > if (!IS_ERR_OR_NULL(attr_grp)) {
> > - kfree(attr_grp->bin_attrs);
> > + kfree(attr_grp->bin_attrs_new);
>
> This is good, but the assignment also needs to be done to .bin_attrs_new.
> That is the code I can find on LKML:
> https://lore.kernel.org/lkml/20250506-tdx-rtmr-v6-1-ac6ff5e9d58a@intel.com/
Oh, the latest fixup that preceded this only went to linux-coco:
http://lore.kernel.org/20250509010104.669669-1-dan.j.williams@intel.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 17:27 ` dan.j.williams
@ 2025-05-13 17:29 ` dan.j.williams
2025-05-13 18:25 ` Thomas Weißschuh
0 siblings, 1 reply; 8+ messages in thread
From: dan.j.williams @ 2025-05-13 17:29 UTC (permalink / raw)
To: dan.j.williams, Thomas Weißschuh, Cedric Xing
Cc: dan.j.williams, sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo,
aik, suzuki.poulose, steven.price, lukas, greg, linux-next
dan.j.williams@ wrote:
> Thomas Weißschuh wrote:
> > On 2025-05-13 11:41:54-0500, Cedric Xing wrote:
> > > Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
> > > the ro-after-init principle by making elements of bin_attrs_new pointing to
> > > const.
> > >
> > > To align with this change, introduce a temporary variable `bap` within the
> > > initialization loop. This improves code clarity by explicitly marking the
> > > initialization scope and eliminates the need for type casts when assigning
> > > to bin_attrs_new.
> > >
> > > Signed-off-by: Cedric Xing <cedric.xing@intel•com>
> > > ---
> > > drivers/virt/coco/tsm-mr.c | 30 +++++++++++++++---------------
> > > 1 file changed, 15 insertions(+), 15 deletions(-)
> >
> > <snip>
> >
> > > @@ -244,7 +244,7 @@ EXPORT_SYMBOL_GPL(tsm_mr_create_attribute_group);
> > > void tsm_mr_free_attribute_group(const struct attribute_group *attr_grp)
> > > {
> > > if (!IS_ERR_OR_NULL(attr_grp)) {
> > > - kfree(attr_grp->bin_attrs);
> > > + kfree(attr_grp->bin_attrs_new);
> >
> > This is good, but the assignment also needs to be done to .bin_attrs_new.
> > That is the code I can find on LKML:
> > https://lore.kernel.org/lkml/20250506-tdx-rtmr-v6-1-ac6ff5e9d58a@intel.com/
>
> Oh, the latest fixup that preceded this only went to linux-coco:
>
> http://lore.kernel.org/20250509010104.669669-1-dan.j.williams@intel.com
Sorry, this one:
http://lore.kernel.org/20250509020739.882913-1-dan.j.williams@intel.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 17:29 ` dan.j.williams
@ 2025-05-13 18:25 ` Thomas Weißschuh
0 siblings, 0 replies; 8+ messages in thread
From: Thomas Weißschuh @ 2025-05-13 18:25 UTC (permalink / raw)
To: dan.j.williams
Cc: Cedric Xing, sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo,
aik, suzuki.poulose, steven.price, lukas, greg, linux-next
On 2025-05-13 10:29:46-0700, dan.j.williams@intel•com wrote:
> dan.j.williams@ wrote:
> > Thomas Weißschuh wrote:
> > > On 2025-05-13 11:41:54-0500, Cedric Xing wrote:
> > > > Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
> > > > the ro-after-init principle by making elements of bin_attrs_new pointing to
> > > > const.
> > > >
> > > > To align with this change, introduce a temporary variable `bap` within the
> > > > initialization loop. This improves code clarity by explicitly marking the
> > > > initialization scope and eliminates the need for type casts when assigning
> > > > to bin_attrs_new.
> > > >
> > > > Signed-off-by: Cedric Xing <cedric.xing@intel•com>
> > > > ---
> > > > drivers/virt/coco/tsm-mr.c | 30 +++++++++++++++---------------
> > > > 1 file changed, 15 insertions(+), 15 deletions(-)
> > >
> > > <snip>
> > >
> > > > @@ -244,7 +244,7 @@ EXPORT_SYMBOL_GPL(tsm_mr_create_attribute_group);
> > > > void tsm_mr_free_attribute_group(const struct attribute_group *attr_grp)
> > > > {
> > > > if (!IS_ERR_OR_NULL(attr_grp)) {
> > > > - kfree(attr_grp->bin_attrs);
> > > > + kfree(attr_grp->bin_attrs_new);
> > >
> > > This is good, but the assignment also needs to be done to .bin_attrs_new.
> > > That is the code I can find on LKML:
> > > https://lore.kernel.org/lkml/20250506-tdx-rtmr-v6-1-ac6ff5e9d58a@intel.com/
> >
> > Oh, the latest fixup that preceded this only went to linux-coco:
> >
> > http://lore.kernel.org/20250509010104.669669-1-dan.j.williams@intel.com
>
> Sorry, this one:
>
> http://lore.kernel.org/20250509020739.882913-1-dan.j.williams@intel.com
Thanks for the pointer!
For this patch:
Reviewed-by: Thomas Weißschuh <linux@weissschuh•net>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase
2025-05-13 16:41 ` [PATCH v2] " Cedric Xing
2025-05-13 16:49 ` Thomas Weißschuh
@ 2025-05-13 18:37 ` Dan Williams
1 sibling, 0 replies; 8+ messages in thread
From: Dan Williams @ 2025-05-13 18:37 UTC (permalink / raw)
To: Cedric Xing, linux, dan.j.williams
Cc: sfr, sathyanarayanan.kuppuswamy, yilun.xu, sameo, aik,
suzuki.poulose, steven.price, lukas, greg, linux-next,
Cedric Xing
Cedric Xing wrote:
> Commit 9bec944506fa ("sysfs: constify attribute_group::bin_attrs") enforced
> the ro-after-init principle by making elements of bin_attrs_new pointing to
> const.
>
> To align with this change, introduce a temporary variable `bap` within the
> initialization loop. This improves code clarity by explicitly marking the
> initialization scope and eliminates the need for type casts when assigning
> to bin_attrs_new.
>
> Signed-off-by: Cedric Xing <cedric.xing@intel•com>
Looks good to me, added to linux-next for more exposure.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-05-13 18:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-13 15:47 [PATCH] tsm-mr: Fix init breakage after bin_attrs constification by scoping non-const pointers to init phase Cedric Xing
2025-05-13 16:24 ` Thomas Weißschuh
2025-05-13 16:41 ` [PATCH v2] " Cedric Xing
2025-05-13 16:49 ` Thomas Weißschuh
2025-05-13 17:27 ` dan.j.williams
2025-05-13 17:29 ` dan.j.williams
2025-05-13 18:25 ` Thomas Weißschuh
2025-05-13 18:37 ` Dan Williams
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox