* [PATCH 1/3] NFSD: Teach nfsd_mountpoint() auto mounts
2022-11-17 19:11 [PATCH 0/3] NFS: NFSD: Allow crossing mounts when re-exporting Richard Weinberger
@ 2022-11-17 19:11 ` Richard Weinberger
2022-11-17 19:11 ` [PATCH 2/3] fs: namei: Allow follow_down() to uncover " Richard Weinberger
2022-11-17 19:11 ` [PATCH 3/3] NFS: nfs_encode_fh: Remove S_AUTOMOUNT check Richard Weinberger
2 siblings, 0 replies; 10+ messages in thread
From: Richard Weinberger @ 2022-11-17 19:11 UTC (permalink / raw)
To: linux-nfs
Cc: linux-kernel, linux-fsdevel, jlayton, chuck.lever, anna,
trond.myklebust, viro, raven, chris.chilvers, david.young,
luis.turcitu, david, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-17 19:11 [PATCH 0/3] NFS: NFSD: Allow crossing mounts when re-exporting Richard Weinberger
2022-11-17 19:11 ` [PATCH 1/3] NFSD: Teach nfsd_mountpoint() auto mounts Richard Weinberger
@ 2022-11-17 19:11 ` Richard Weinberger
2022-11-17 21:01 ` Jeff Layton
2022-11-17 19:11 ` [PATCH 3/3] NFS: nfs_encode_fh: Remove S_AUTOMOUNT check Richard Weinberger
2 siblings, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2022-11-17 19:11 UTC (permalink / raw)
To: linux-nfs
Cc: linux-kernel, linux-fsdevel, jlayton, chuck.lever, anna,
trond.myklebust, viro, raven, chris.chilvers, david.young,
luis.turcitu, david, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-17 19:11 ` [PATCH 2/3] fs: namei: Allow follow_down() to uncover " Richard Weinberger
@ 2022-11-17 21:01 ` Jeff Layton
2022-11-17 21:12 ` Richard Weinberger
2022-11-17 23:37 ` Ian Kent
0 siblings, 2 replies; 10+ messages in thread
From: Jeff Layton @ 2022-11-17 21:01 UTC (permalink / raw)
To: Richard Weinberger, linux-nfs
Cc: linux-kernel, linux-fsdevel, chuck.lever, anna, trond.myklebust,
viro, raven, chris.chilvers, david.young, luis.turcitu, david
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-17 21:01 ` Jeff Layton
@ 2022-11-17 21:12 ` Richard Weinberger
2022-11-17 21:42 ` Jeff Layton
2022-11-17 23:37 ` Ian Kent
1 sibling, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2022-11-17 21:12 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-nfs, linux-kernel, linux-fsdevel, chuck lever, anna,
trond myklebust, Al Viro, raven, chris chilvers, david young,
luis turcitu, david
----- 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-17 21:12 ` Richard Weinberger
@ 2022-11-17 21:42 ` Jeff Layton
2022-11-27 21:29 ` Richard Weinberger
0 siblings, 1 reply; 10+ messages in thread
From: Jeff Layton @ 2022-11-17 21:42 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-nfs, linux-kernel, linux-fsdevel, chuck lever, anna,
trond myklebust, Al Viro, raven, chris chilvers, david young,
luis turcitu, david
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-17 21:42 ` Jeff Layton
@ 2022-11-27 21:29 ` Richard Weinberger
2022-11-28 11:49 ` Jeff Layton
0 siblings, 1 reply; 10+ messages in thread
From: Richard Weinberger @ 2022-11-27 21:29 UTC (permalink / raw)
To: Jeff Layton
Cc: linux-nfs, linux-kernel, linux-fsdevel, chuck lever, anna,
trond myklebust, Al Viro, raven, chris chilvers, david young,
luis turcitu, david
----- 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
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-27 21:29 ` Richard Weinberger
@ 2022-11-28 11:49 ` Jeff Layton
0 siblings, 0 replies; 10+ messages in thread
From: Jeff Layton @ 2022-11-28 11:49 UTC (permalink / raw)
To: Richard Weinberger
Cc: linux-nfs, linux-kernel, linux-fsdevel, chuck lever, anna,
trond myklebust, Al Viro, raven, chris chilvers, david young,
luis turcitu, david
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>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] fs: namei: Allow follow_down() to uncover auto mounts
2022-11-17 21:01 ` Jeff Layton
2022-11-17 21:12 ` Richard Weinberger
@ 2022-11-17 23:37 ` Ian Kent
1 sibling, 0 replies; 10+ messages in thread
From: Ian Kent @ 2022-11-17 23:37 UTC (permalink / raw)
To: Jeff Layton, Richard Weinberger, linux-nfs
Cc: linux-kernel, linux-fsdevel, chuck.lever, anna, trond.myklebust,
viro, chris.chilvers, david.young, luis.turcitu, david
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] NFS: nfs_encode_fh: Remove S_AUTOMOUNT check
2022-11-17 19:11 [PATCH 0/3] NFS: NFSD: Allow crossing mounts when re-exporting Richard Weinberger
2022-11-17 19:11 ` [PATCH 1/3] NFSD: Teach nfsd_mountpoint() auto mounts Richard Weinberger
2022-11-17 19:11 ` [PATCH 2/3] fs: namei: Allow follow_down() to uncover " Richard Weinberger
@ 2022-11-17 19:11 ` Richard Weinberger
2 siblings, 0 replies; 10+ messages in thread
From: Richard Weinberger @ 2022-11-17 19:11 UTC (permalink / raw)
To: linux-nfs
Cc: linux-kernel, linux-fsdevel, jlayton, chuck.lever, anna,
trond.myklebust, viro, raven, chris.chilvers, david.young,
luis.turcitu, david, 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
^ permalink raw reply related [flat|nested] 10+ messages in thread