[PATCH 4/4] nfs: sysfs: use default get_ownership() callback

Alexander Aring posted 4 patches 1 month, 3 weeks ago
[PATCH 4/4] nfs: sysfs: use default get_ownership() callback
Posted by Alexander Aring 1 month, 3 weeks ago
Since commit 5f81880d5204 ("sysfs, kobject: allow creating kobject
belonging to arbitrary users") it seems that there could be cases for
kobjects belonging to arbitrary users. This callback is set by default
when using kset_create_and_add() to allow creating kobjects with
different ownerships according to its parent.

This patch will assign the default callback now for nfs kobjects for
cases when the parent has different ownership than the default one.

Signed-off-by: Alexander Aring <aahringo@redhat.com>
---
 fs/nfs/sysfs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
index a6584203b7ff..b5737464b892 100644
--- a/fs/nfs/sysfs.c
+++ b/fs/nfs/sysfs.c
@@ -27,6 +27,7 @@ static const struct kobj_ns_type_operations *nfs_netns_object_child_ns_type(
 }
 
 static struct kobj_type nfs_kset_type = {
+	.get_ownership = kset_get_ownership,
 	.release = kset_release,
 	.sysfs_ops = &kobj_sysfs_ops,
 	.child_ns_type = nfs_netns_object_child_ns_type,
-- 
2.43.0
Re: [PATCH 4/4] nfs: sysfs: use default get_ownership() callback
Posted by Benjamin Coddington 1 month, 2 weeks ago
On 3 Oct 2024, at 11:14, Alexander Aring wrote:

> Since commit 5f81880d5204 ("sysfs, kobject: allow creating kobject
> belonging to arbitrary users") it seems that there could be cases for
> kobjects belonging to arbitrary users. This callback is set by default
> when using kset_create_and_add() to allow creating kobjects with
> different ownerships according to its parent.
>
> This patch will assign the default callback now for nfs kobjects for
> cases when the parent has different ownership than the default one.
>
> Signed-off-by: Alexander Aring <aahringo@redhat.com>
> ---
>  fs/nfs/sysfs.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
> index a6584203b7ff..b5737464b892 100644
> --- a/fs/nfs/sysfs.c
> +++ b/fs/nfs/sysfs.c
> @@ -27,6 +27,7 @@ static const struct kobj_ns_type_operations *nfs_netns_object_child_ns_type(
>  }
>
>  static struct kobj_type nfs_kset_type = {
> +	.get_ownership = kset_get_ownership,
>  	.release = kset_release,
>  	.sysfs_ops = &kobj_sysfs_ops,
>  	.child_ns_type = nfs_netns_object_child_ns_type,
> -- 
> 2.43.0

Hi Alex, if I understand this correctly, this patch just punts the ownership
callback up to fs_kobj, which, because it has no .get_ownership is just
going to be the same result: root.

Does this patch add value?

Ben
Re: [PATCH 4/4] nfs: sysfs: use default get_ownership() callback
Posted by Alexander Aring 1 month, 2 weeks ago
Hi,

On Tue, Oct 8, 2024 at 4:09 PM Benjamin Coddington <bcodding@redhat.com> wrote:
>
> On 3 Oct 2024, at 11:14, Alexander Aring wrote:
>
> > Since commit 5f81880d5204 ("sysfs, kobject: allow creating kobject
> > belonging to arbitrary users") it seems that there could be cases for
> > kobjects belonging to arbitrary users. This callback is set by default
> > when using kset_create_and_add() to allow creating kobjects with
> > different ownerships according to its parent.
> >
> > This patch will assign the default callback now for nfs kobjects for
> > cases when the parent has different ownership than the default one.
> >
> > Signed-off-by: Alexander Aring <aahringo@redhat.com>
> > ---
> >  fs/nfs/sysfs.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
> > index a6584203b7ff..b5737464b892 100644
> > --- a/fs/nfs/sysfs.c
> > +++ b/fs/nfs/sysfs.c
> > @@ -27,6 +27,7 @@ static const struct kobj_ns_type_operations *nfs_netns_object_child_ns_type(
> >  }
> >
> >  static struct kobj_type nfs_kset_type = {
> > +     .get_ownership = kset_get_ownership,
> >       .release = kset_release,
> >       .sysfs_ops = &kobj_sysfs_ops,
> >       .child_ns_type = nfs_netns_object_child_ns_type,
> > --
> > 2.43.0
>
> Hi Alex, if I understand this correctly, this patch just punts the ownership
> callback up to fs_kobj, which, because it has no .get_ownership is just
> going to be the same result: root.
>

Yes, from my understanding this has to do with user namespaces and
being able to write to the related sysfs/kobject files?

> Does this patch add value?
>

For me no, but it is there now as default and I think somebody
probably forgot to update all the other users that created their kset
in a more low-level kind of way.
I think the whole "transitioning process" to make the same things with
namespaces created under root vs user is still kind of still in
process.

I am not sure if any of the nfs userspace namespace aware tools are
also limited by other things that require additional changes to do
something else as a "normal" user.

For me it's fine to drop this patch series and if somebody wants to
make a user namespace working with nfs tooling can come back again and
find out such patch might be necessary?

Thanks.

- Alex