[PATCH 1/6] sysfs: attribute_group: allow registration of const attribute

Thomas Weißschuh posted 6 patches 11 months ago
There is a newer version of this series
[PATCH 1/6] sysfs: attribute_group: allow registration of const attribute
Posted by Thomas Weißschuh 11 months ago
To be able to constify instances of struct attribute it has to be
possible to add them to struct attribute_group.
The current type of the attrs member however is not compatible with that.
Introduce a union that allows registration of both const and non-const
attributes to enable a piecewise transition.
As both union member types are compatible no logic needs to be adapted.

Technically it is now possible register a const struct
attribute and receive it as mutable pointer in the callbacks.
This is a soundness issue.
But this same soundness issue already exists today in
sysfs_create_file().
Also the struct definition and callback implementation are always
closely linked and are meant to be moved to const in lockstep.

Similar to commit 906c508afdca ("sysfs: attribute_group: allow registration of const bin_attribute")

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/linux/sysfs.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 0f2fcd244523f050c5286f19d4fe1846506f9214..f5e25bed777a6a6e717f10973f1abcd12111f5c5 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -105,7 +105,10 @@ struct attribute_group {
 	size_t			(*bin_size)(struct kobject *,
 					    const struct bin_attribute *,
 					    int);
-	struct attribute	**attrs;
+	union {
+		struct attribute	**attrs;
+		const struct attribute	*const *attrs_new;
+	};
 	union {
 		struct bin_attribute		**bin_attrs;
 		const struct bin_attribute	*const *bin_attrs_new;

-- 
2.48.1

Re: [PATCH 1/6] sysfs: attribute_group: allow registration of const attribute
Posted by Greg Kroah-Hartman 11 months ago
On Thu, Jan 16, 2025 at 06:32:27PM +0100, Thomas Weißschuh wrote:
> To be able to constify instances of struct attribute it has to be
> possible to add them to struct attribute_group.
> The current type of the attrs member however is not compatible with that.
> Introduce a union that allows registration of both const and non-const
> attributes to enable a piecewise transition.
> As both union member types are compatible no logic needs to be adapted.
> 
> Technically it is now possible register a const struct
> attribute and receive it as mutable pointer in the callbacks.
> This is a soundness issue.
> But this same soundness issue already exists today in
> sysfs_create_file().
> Also the struct definition and callback implementation are always
> closely linked and are meant to be moved to const in lockstep.
> 
> Similar to commit 906c508afdca ("sysfs: attribute_group: allow registration of const bin_attribute")
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>  include/linux/sysfs.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 0f2fcd244523f050c5286f19d4fe1846506f9214..f5e25bed777a6a6e717f10973f1abcd12111f5c5 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -105,7 +105,10 @@ struct attribute_group {
>  	size_t			(*bin_size)(struct kobject *,
>  					    const struct bin_attribute *,
>  					    int);
> -	struct attribute	**attrs;
> +	union {
> +		struct attribute	**attrs;
> +		const struct attribute	*const *attrs_new;
> +	};

I'm all for the idea, BUT, let's finish up doing this one:

>  	union {
>  		struct bin_attribute		**bin_attrs;
>  		const struct bin_attribute	*const *bin_attrs_new;

first please.

That way we can see just how "easy" the switch from _new to not-new goes :)

thanks,

greg k-h
Re: [PATCH 1/6] sysfs: attribute_group: allow registration of const attribute
Posted by Thomas Weißschuh 5 months, 2 weeks ago
Hi Greg,

On 2025-01-17 08:01:00+0100, Greg Kroah-Hartman wrote:
> On Thu, Jan 16, 2025 at 06:32:27PM +0100, Thomas Weißschuh wrote:
> > To be able to constify instances of struct attribute it has to be
> > possible to add them to struct attribute_group.
> > The current type of the attrs member however is not compatible with that.
> > Introduce a union that allows registration of both const and non-const
> > attributes to enable a piecewise transition.
> > As both union member types are compatible no logic needs to be adapted.
> > 
> > Technically it is now possible register a const struct
> > attribute and receive it as mutable pointer in the callbacks.
> > This is a soundness issue.
> > But this same soundness issue already exists today in
> > sysfs_create_file().
> > Also the struct definition and callback implementation are always
> > closely linked and are meant to be moved to const in lockstep.
> > 
> > Similar to commit 906c508afdca ("sysfs: attribute_group: allow registration of const bin_attribute")
> > 
> > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > ---
> >  include/linux/sysfs.h | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > index 0f2fcd244523f050c5286f19d4fe1846506f9214..f5e25bed777a6a6e717f10973f1abcd12111f5c5 100644
> > --- a/include/linux/sysfs.h
> > +++ b/include/linux/sysfs.h
> > @@ -105,7 +105,10 @@ struct attribute_group {
> >  	size_t			(*bin_size)(struct kobject *,
> >  					    const struct bin_attribute *,
> >  					    int);
> > -	struct attribute	**attrs;
> > +	union {
> > +		struct attribute	**attrs;
> > +		const struct attribute	*const *attrs_new;
> > +	};
> 
> I'm all for the idea, BUT, let's finish up doing this one:
> 
> >  	union {
> >  		struct bin_attribute		**bin_attrs;
> >  		const struct bin_attribute	*const *bin_attrs_new;
> 
> first please.
> 
> That way we can see just how "easy" the switch from _new to not-new goes :)

I'd like to resend these preparatory patches so they go into v6.17-rc1
and I can work on the follow-up changes.
In my opinion the switch from _new will work nicely. There have been no
new users of _new in -next at all.

Any objections?


Thomas
Re: [PATCH 1/6] sysfs: attribute_group: allow registration of const attribute
Posted by Greg Kroah-Hartman 5 months, 2 weeks ago
On Sat, Jun 28, 2025 at 10:19:07AM +0200, Thomas Weißschuh wrote:
> Hi Greg,
> 
> On 2025-01-17 08:01:00+0100, Greg Kroah-Hartman wrote:
> > On Thu, Jan 16, 2025 at 06:32:27PM +0100, Thomas Weißschuh wrote:
> > > To be able to constify instances of struct attribute it has to be
> > > possible to add them to struct attribute_group.
> > > The current type of the attrs member however is not compatible with that.
> > > Introduce a union that allows registration of both const and non-const
> > > attributes to enable a piecewise transition.
> > > As both union member types are compatible no logic needs to be adapted.
> > > 
> > > Technically it is now possible register a const struct
> > > attribute and receive it as mutable pointer in the callbacks.
> > > This is a soundness issue.
> > > But this same soundness issue already exists today in
> > > sysfs_create_file().
> > > Also the struct definition and callback implementation are always
> > > closely linked and are meant to be moved to const in lockstep.
> > > 
> > > Similar to commit 906c508afdca ("sysfs: attribute_group: allow registration of const bin_attribute")
> > > 
> > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> > > ---
> > >  include/linux/sysfs.h | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > > index 0f2fcd244523f050c5286f19d4fe1846506f9214..f5e25bed777a6a6e717f10973f1abcd12111f5c5 100644
> > > --- a/include/linux/sysfs.h
> > > +++ b/include/linux/sysfs.h
> > > @@ -105,7 +105,10 @@ struct attribute_group {
> > >  	size_t			(*bin_size)(struct kobject *,
> > >  					    const struct bin_attribute *,
> > >  					    int);
> > > -	struct attribute	**attrs;
> > > +	union {
> > > +		struct attribute	**attrs;
> > > +		const struct attribute	*const *attrs_new;
> > > +	};
> > 
> > I'm all for the idea, BUT, let's finish up doing this one:
> > 
> > >  	union {
> > >  		struct bin_attribute		**bin_attrs;
> > >  		const struct bin_attribute	*const *bin_attrs_new;
> > 
> > first please.
> > 
> > That way we can see just how "easy" the switch from _new to not-new goes :)
> 
> I'd like to resend these preparatory patches so they go into v6.17-rc1
> and I can work on the follow-up changes.
> In my opinion the switch from _new will work nicely. There have been no
> new users of _new in -next at all.
> 
> Any objections?

Sure, please do.

greg k-h