[PATCH 0/3] NFS: NFSD: Allow crossing mounts when re-exporting

[PATCH 0/3] NFS: NFSD: Allow crossing mounts when re-exporting

[Richard Weinberger]

Currently when re-exporting a NFS share the NFS cross mount feature does
not work [0].
This patch series outlines an approach to address the problem.

Crossing mounts does not work for two reasons:

1. As soon the NFS client (on the re-exporting server) sees a different
filesystem id, it installs an automount. That way the other filesystem
will be mounted automatically when someone enters the directory.
But the cross mount logic of KNFS does not know about automount.
This patch series addresses the problem and teach both KNFSD
and the exportfs logic of NFS to deal with automount.

2. When KNFSD detects crossing of a mount point, it asks rpc.mountd to install
a new export for the target mount point. Beside of authentication rpc.mountd
also has to find a filesystem id for the new export. Is the to be exported
filesystem a NFS share, rpc.mountd cannot derive a filesystem id from it and
refuses to export. In the logs you'll see errors such as:

mountd: Cannot export /srv/nfs/vol0, possibly unsupported filesystem or fsid= required

To deal with that I've changed rpc.mountd to use generate and store fsids [1].
Since the kernel side of my changes did change for a long time I decided to
try upstreaming it first.
A 3rd iteration of my rpc.mountd will happen soon.

[0] https://marc.info/?l=linux-nfs&m=161653016627277&w=2
[1] https://lore.kernel.org/linux-nfs/20220217131531.2890-1-richard@nod.at/

Richard Weinberger (3):
  NFSD: Teach nfsd_mountpoint() auto mounts
  fs: namei: Allow follow_down() to uncover auto mounts
  NFS: nfs_encode_fh: Remove S_AUTOMOUNT check

 fs/namei.c      | 2 +-
 fs/nfs/export.c | 2 +-
 fs/nfsd/vfs.c   | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.26.2

[PATCH 1/3] NFSD: Teach nfsd_mountpoint() auto mounts

[Richard Weinberger]

Currently nfsd_mountpoint() tests for mount points using d_mountpoint(),
this works only when a mount point is already uncovered.
In our case the mount point is of type auto mount and can be coverted.
i.e. ->d_automount() was not called.

Using d_managed() nfsd_mountpoint() can test whether a mount point is
either already uncovered or can be uncovered later.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/nfsd/vfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f650afedd67f..157f0df0e93a 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -160,7 +160,7 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
 		return 1;
 	if (nfsd4_is_junction(dentry))
 		return 1;
-	if (d_mountpoint(dentry))
+	if (d_managed(dentry))
 		/*
 		 * Might only be a mountpoint in a different namespace,
 		 * but we need to check.
-- 
2.26.2

[PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Richard Weinberger]

This function is only used by NFSD to cross mount points.
If a mount point is of type auto mount, follow_down() will
not uncover it. Add LOOKUP_AUTOMOUNT to the lookup flags
to have ->d_automount() called when NFSD walks down the
mount tree.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/namei.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/namei.c b/fs/namei.c
index 578c2110df02..000c4b84e6be 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1462,7 +1462,7 @@ int follow_down(struct path *path)
 {
 	struct vfsmount *mnt = path->mnt;
 	bool jumped;
-	int ret = traverse_mounts(path, &jumped, NULL, 0);
+	int ret = traverse_mounts(path, &jumped, NULL, LOOKUP_AUTOMOUNT);
 
 	if (path->mnt != mnt)
 		mntput(mnt);
-- 
2.26.2

[PATCH 3/3] NFS: nfs_encode_fh: Remove S_AUTOMOUNT check

[Richard Weinberger]

Now with NFSD being able to cross into auto mounts,
the check can be removed.

Signed-off-by: Richard Weinberger <richard@nod.at>
---
 fs/nfs/export.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/export.c b/fs/nfs/export.c
index 01596f2d0a1e..0a5ee1754d50 100644
--- a/fs/nfs/export.c
+++ b/fs/nfs/export.c
@@ -42,7 +42,7 @@ nfs_encode_fh(struct inode *inode, __u32 *p, int *max_len, struct inode *parent)
 	dprintk("%s: max fh len %d inode %p parent %p",
 		__func__, *max_len, inode, parent);
 
-	if (*max_len < len || IS_AUTOMOUNT(inode)) {
+	if (*max_len < len) {
 		dprintk("%s: fh len %d too small, required %d\n",
 			__func__, *max_len, len);
 		*max_len = len;
-- 
2.26.2

Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Jeff Layton]

On Thu, 2022-11-17 at 20:11 +0100, Richard Weinberger wrote:
> This function is only used by NFSD to cross mount points.
> If a mount point is of type auto mount, follow_down() will
> not uncover it. Add LOOKUP_AUTOMOUNT to the lookup flags
> to have ->d_automount() called when NFSD walks down the
> mount tree.
> 
> Signed-off-by: Richard Weinberger <richard@nod.at>
> ---
>  fs/namei.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 578c2110df02..000c4b84e6be 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1462,7 +1462,7 @@ int follow_down(struct path *path)
>  {
>  	struct vfsmount *mnt = path->mnt;
>  	bool jumped;
> -	int ret = traverse_mounts(path, &jumped, NULL, 0);
> +	int ret = traverse_mounts(path, &jumped, NULL, LOOKUP_AUTOMOUNT);
>  
>  	if (path->mnt != mnt)
>  		mntput(mnt);


What happens when CROSSMOUNT isn't enabled and someone tries to stroll
into an automount point? I'm guessing the automount happens but the
export is denied? It seems like LOOKUP_AUTOMOUNT ought to be conditional
on the parent export having CROSSMOUNT set.

There's also another caller of follow_down too, the UNIX98 pty code.
This may be harmless for it, but it'd be best not to perturb that if we
can help it.

Maybe follow_down can grow a lookupflags argument?
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Jeff Layton" <jlayton@kernel.org>
> What happens when CROSSMOUNT isn't enabled and someone tries to stroll
> into an automount point? I'm guessing the automount happens but the
> export is denied? 

Exactly.

On the other hand, why should knfsd not trigger automounts?
Almost any userspace interaction would also do so.

> It seems like LOOKUP_AUTOMOUNT ought to be conditional
> on the parent export having CROSSMOUNT set.
> 
> There's also another caller of follow_down too, the UNIX98 pty code.
> This may be harmless for it, but it'd be best not to perturb that if we
> can help it.
> 
> Maybe follow_down can grow a lookupflags argument?

So, in nfsd_cross_mnt() the follow_down() helper should use LOOKUP_AUTOMOUNT only
if exp->ex_flags & NFSEXP_CROSSMOUNT is true?
Sounds sane, thanks for the pointer.

Thanks,
//richard

Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Jeff Layton]

On Thu, 2022-11-17 at 22:12 +0100, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Jeff Layton" <jlayton@kernel.org>
> > What happens when CROSSMOUNT isn't enabled and someone tries to stroll
> > into an automount point? I'm guessing the automount happens but the
> > export is denied? 
> 
> Exactly.
> 
> On the other hand, why should knfsd not trigger automounts?
> Almost any userspace interaction would also do so.
> 

I have no issue with knfsd activity triggering an automount, but I think
it'd be best if we don't do that when knfsd can't do anything with the
resulting filesystem. Automounts can be expensive.

> > It seems like LOOKUP_AUTOMOUNT ought to be conditional
> > on the parent export having CROSSMOUNT set.
> > 
> > There's also another caller of follow_down too, the UNIX98 pty code.
> > This may be harmless for it, but it'd be best not to perturb that if we
> > can help it.
> > 
> > Maybe follow_down can grow a lookupflags argument?
> 
> So, in nfsd_cross_mnt() the follow_down() helper should use LOOKUP_AUTOMOUNT only
> if exp->ex_flags & NFSEXP_CROSSMOUNT is true?
> Sounds sane, thanks for the pointer.
> 

Yeah, I think so. I do wonder if we ought to make any provision for
"nohide" exports, but since you have to enumerate those explicitly, it
shouldn't be a huge problem for someone to just ensure that they're
mounted beforehand.

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Ian Kent]

On 18/11/22 05:01, Jeff Layton wrote:
> On Thu, 2022-11-17 at 20:11 +0100, Richard Weinberger wrote:
>> This function is only used by NFSD to cross mount points.
>> If a mount point is of type auto mount, follow_down() will
>> not uncover it. Add LOOKUP_AUTOMOUNT to the lookup flags
>> to have ->d_automount() called when NFSD walks down the
>> mount tree.
>>
>> Signed-off-by: Richard Weinberger <richard@nod.at>
>> ---
>>   fs/namei.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 578c2110df02..000c4b84e6be 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -1462,7 +1462,7 @@ int follow_down(struct path *path)
>>   {
>>   	struct vfsmount *mnt = path->mnt;
>>   	bool jumped;
>> -	int ret = traverse_mounts(path, &jumped, NULL, 0);
>> +	int ret = traverse_mounts(path, &jumped, NULL, LOOKUP_AUTOMOUNT);
>>   
>>   	if (path->mnt != mnt)
>>   		mntput(mnt);
>
> What happens when CROSSMOUNT isn't enabled and someone tries to stroll
> into an automount point? I'm guessing the automount happens but the
> export is denied? It seems like LOOKUP_AUTOMOUNT ought to be conditional
> on the parent export having CROSSMOUNT set.
>
> There's also another caller of follow_down too, the UNIX98 pty code.
> This may be harmless for it, but it'd be best not to perturb that if we
> can help it.
>
> Maybe follow_down can grow a lookupflags argument?es, I think that's needed too.


Changing the core VFS unconditionally ricks breaking things.


For example this:

         if (!(lookup_flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY |
                            LOOKUP_OPEN | LOOKUP_CREATE | 
LOOKUP_AUTOMOUNT)) &&
             dentry->d_inode)

will never be true now so that, at the least, the handling of this case

will change for automount(8). I don't remember now the reasons behind

doing this but I do remember there was a special case that needed to

be handled by it.


Ian

Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "Jeff Layton" <jlayton@kernel.org>
>> So, in nfsd_cross_mnt() the follow_down() helper should use LOOKUP_AUTOMOUNT
>> only
>> if exp->ex_flags & NFSEXP_CROSSMOUNT is true?
>> Sounds sane, thanks for the pointer.
>> 
> 
> Yeah, I think so. I do wonder if we ought to make any provision for
> "nohide" exports, but since you have to enumerate those explicitly, it
> shouldn't be a huge problem for someone to just ensure that they're
> mounted beforehand.

TBH, I didn't invest much into the nohide feature wrt. NFS re-exporting.
What problem do you have in mind?

I wonder also what NFS client folks think about my changes before I send
the next revision (with Jeff's comments addressed).

Thanks,
//richard

Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts

[Jeff Layton]

On Sun, 2022-11-27 at 22:29 +0100, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Jeff Layton" <jlayton@kernel.org>
> > > So, in nfsd_cross_mnt() the follow_down() helper should use LOOKUP_AUTOMOUNT
> > > only
> > > if exp->ex_flags & NFSEXP_CROSSMOUNT is true?
> > > Sounds sane, thanks for the pointer.
> > > 
> > 
> > Yeah, I think so. I do wonder if we ought to make any provision for
> > "nohide" exports, but since you have to enumerate those explicitly, it
> > shouldn't be a huge problem for someone to just ensure that they're
> > mounted beforehand.
> 
> TBH, I didn't invest much into the nohide feature wrt. NFS re-exporting.
> What problem do you have in mind?
> 

nohide is sort of complimentary to crossmnt. You can achieve the same
effect as crossmnt by adding explicit exports for all the children and
marking them "nohide".

The point here is that you have to explicitly create exports for the
child mounts in that case, and if you're doing that then it's not a
burden for the admin to make sure they're mounted before exporting.

So, I don't think we need to worry about nohide here after all.

> I wonder also what NFS client folks think about my changes before I send
> the next revision (with Jeff's comments addressed).
-- 
Jeff Layton <jlayton@kernel.org>