mbox series

[0/4] exportfs: add flag to allow marking export operations as only supporting file handles

Message ID 20241201-work-exportfs-v1-0-b850dda4502a@kernel.org (mailing list archive)
Headers show
Series exportfs: add flag to allow marking export operations as only supporting file handles | expand

Message

Christian Brauner Dec. 1, 2024, 1:12 p.m. UTC
Hey,

Some filesystems like kernfs and pidfs support file handles as a
convenience to enable the use of name_to_handle_at(2) and
open_by_handle_at(2) but don't want to and cannot be reliably exported.
Add a flag that allows them to mark their export operations accordingly
and make NFS check for its presence.

@Amir, I'll reorder the patches such that this series comes prior to the
pidfs file handle series. Doing it that way will mean that there's never
a state where pidfs supports file handles while also being exportable.
It's probably not a big deal but it's definitely cleaner. It also means
the last patch in this series to mark pidfs as non-exportable can be
dropped. Instead pidfs export operations will be marked as
non-exportable in the patch that they are added in.

Thanks!
Christian

---
Christian Brauner (4):
      exportfs: add flag to indicate local file handles
      kernfs: restrict to local file handles
      ovl: restrict to exportable file handles
      pidfs: restrict to local file handles

 fs/kernfs/mount.c        | 1 +
 fs/nfsd/export.c         | 8 +++++++-
 fs/overlayfs/util.c      | 7 ++++++-
 fs/pidfs.c               | 1 +
 include/linux/exportfs.h | 1 +
 5 files changed, 16 insertions(+), 2 deletions(-)
---
base-commit: 74e20c5946ab3f8ad959ea34f63f21e157d3ebae
change-id: 20241201-work-exportfs-cd49bee773c5

Comments

Jeff Layton Dec. 1, 2024, 1:28 p.m. UTC | #1
On Sun, 2024-12-01 at 14:12 +0100, Christian Brauner wrote:
> Hey,
> 
> Some filesystems like kernfs and pidfs support file handles as a
> convenience to enable the use of name_to_handle_at(2) and
> open_by_handle_at(2) but don't want to and cannot be reliably exported.
> Add a flag that allows them to mark their export operations accordingly
> and make NFS check for its presence.
> 
> @Amir, I'll reorder the patches such that this series comes prior to the
> pidfs file handle series. Doing it that way will mean that there's never
> a state where pidfs supports file handles while also being exportable.
> It's probably not a big deal but it's definitely cleaner. It also means
> the last patch in this series to mark pidfs as non-exportable can be
> dropped. Instead pidfs export operations will be marked as
> non-exportable in the patch that they are added in.
> 
> Thanks!
> Christian
> 
> ---
> Christian Brauner (4):
>       exportfs: add flag to indicate local file handles
>       kernfs: restrict to local file handles
>       ovl: restrict to exportable file handles
>       pidfs: restrict to local file handles
> 
>  fs/kernfs/mount.c        | 1 +
>  fs/nfsd/export.c         | 8 +++++++-
>  fs/overlayfs/util.c      | 7 ++++++-
>  fs/pidfs.c               | 1 +
>  include/linux/exportfs.h | 1 +
>  5 files changed, 16 insertions(+), 2 deletions(-)
> ---
> base-commit: 74e20c5946ab3f8ad959ea34f63f21e157d3ebae
> change-id: 20241201-work-exportfs-cd49bee773c5
> 

I've been following the pidfs filehandle discussion and this is exactly
what I was thinking we needed: a way to explicitly label certain
fstypes as unexportable via nfsd.

Reviewed-by: Jeff Layton <jlayton@kernel.org>
Amir Goldstein Dec. 1, 2024, 1:44 p.m. UTC | #2
On Sun, Dec 1, 2024 at 2:12 PM Christian Brauner <brauner@kernel.org> wrote:
>
> Hey,
>
> Some filesystems like kernfs and pidfs support file handles as a
> convenience to enable the use of name_to_handle_at(2) and
> open_by_handle_at(2) but don't want to and cannot be reliably exported.
> Add a flag that allows them to mark their export operations accordingly
> and make NFS check for its presence.
>
> @Amir, I'll reorder the patches such that this series comes prior to the
> pidfs file handle series. Doing it that way will mean that there's never
> a state where pidfs supports file handles while also being exportable.
> It's probably not a big deal but it's definitely cleaner. It also means
> the last patch in this series to mark pidfs as non-exportable can be
> dropped. Instead pidfs export operations will be marked as
> non-exportable in the patch that they are added in.

Yeh, looks good!

Apart from missing update to exporting.rst

Reviewed-by: Amir Goldstein <amir73il@gmail.com>

Thanks,
Amir.
Chuck Lever Dec. 1, 2024, 4:22 p.m. UTC | #3
> On Dec 1, 2024, at 8:28 AM, Jeff Layton <jlayton@kernel.org> wrote:
> 
> On Sun, 2024-12-01 at 14:12 +0100, Christian Brauner wrote:
>> Hey,
>> 
>> Some filesystems like kernfs and pidfs support file handles as a
>> convenience to enable the use of name_to_handle_at(2) and
>> open_by_handle_at(2) but don't want to and cannot be reliably exported.
>> Add a flag that allows them to mark their export operations accordingly
>> and make NFS check for its presence.
>> 
>> @Amir, I'll reorder the patches such that this series comes prior to the
>> pidfs file handle series. Doing it that way will mean that there's never
>> a state where pidfs supports file handles while also being exportable.
>> It's probably not a big deal but it's definitely cleaner. It also means
>> the last patch in this series to mark pidfs as non-exportable can be
>> dropped. Instead pidfs export operations will be marked as
>> non-exportable in the patch that they are added in.
>> 
>> Thanks!
>> Christian
>> 
>> ---
>> Christian Brauner (4):
>>      exportfs: add flag to indicate local file handles
>>      kernfs: restrict to local file handles
>>      ovl: restrict to exportable file handles
>>      pidfs: restrict to local file handles
>> 
>> fs/kernfs/mount.c        | 1 +
>> fs/nfsd/export.c         | 8 +++++++-
>> fs/overlayfs/util.c      | 7 ++++++-
>> fs/pidfs.c               | 1 +
>> include/linux/exportfs.h | 1 +
>> 5 files changed, 16 insertions(+), 2 deletions(-)
>> ---
>> base-commit: 74e20c5946ab3f8ad959ea34f63f21e157d3ebae
>> change-id: 20241201-work-exportfs-cd49bee773c5
>> 
> 
> I've been following the pidfs filehandle discussion and this is exactly
> what I was thinking we needed: a way to explicitly label certain
> fstypes as unexportable via nfsd.
> 
> Reviewed-by: Jeff Layton <jlayton@kernel.org>
> 

Acked-by: Chuck Lever <chuck.lever@oracle.com <mailto:chuck.lever@oracle.com>>

Though, I wonder if a similar but separate prohibition
mechanism might be necessary for other in-kernel network
file system server implementations (eg, ksmbd).


--
Chuck Lever
Christian Brauner Dec. 3, 2024, 9:08 a.m. UTC | #4
> Though, I wonder if a similar but separate prohibition
> mechanism might be necessary for other in-kernel network
> file system server implementations (eg, ksmbd).

Oh hm, interesting question.
I have no idea how ksmbd or 9p "exports" work. I really hope they don't
allow exporting arbitrary pseudo-fses.
Jeff Layton Dec. 3, 2024, 2:32 p.m. UTC | #5
On Tue, 2024-12-03 at 10:08 +0100, Christian Brauner wrote:
> > Though, I wonder if a similar but separate prohibition
> > mechanism might be necessary for other in-kernel network
> > file system server implementations (eg, ksmbd).
> 
> Oh hm, interesting question.
> I have no idea how ksmbd or 9p "exports" work. I really hope they don't
> allow exporting arbitrary pseudo-fses.

SMB is path-based so there's no worry about filehandles there. It looks
like ksmbd keeps a set of ksmbd_share_config objects, that are
configured by userland. If someone deliberately shares stuff under
/proc, then I guess they get to keep all of the pieces. ;)

9P does use filehandles, but there is no in-kernel server, so far.
Wedson had one in development at one point [1], but I haven't heard
anything about it in a while.

[1]: https://kangrejos.com/Async%20Rust%20and%209p%20server.pdf
Christoph Hellwig Dec. 5, 2024, 12:38 a.m. UTC | #6
On Sun, Dec 01, 2024 at 02:12:24PM +0100, Christian Brauner wrote:
> Hey,
> 
> Some filesystems like kernfs and pidfs support file handles as a
> convenience to enable the use of name_to_handle_at(2) and
> open_by_handle_at(2) but don't want to and cannot be reliably exported.
> Add a flag that allows them to mark their export operations accordingly
> and make NFS check for its presence.
> 
> @Amir, I'll reorder the patches such that this series comes prior to the
> pidfs file handle series. Doing it that way will mean that there's never
> a state where pidfs supports file handles while also being exportable.
> It's probably not a big deal but it's definitely cleaner. It also means
> the last patch in this series to mark pidfs as non-exportable can be
> dropped. Instead pidfs export operations will be marked as
> non-exportable in the patch that they are added in.

Can you please invert the polarity?  Marking something as not supporting
is always awkward.  Clearly marking it as supporting something (and
writing down in detail what is required for that) is much better, even
it might cause a little more churn initially.