Hi!
next_mnt() finds submounts below a given path p.
While investigating into an issue in my crossmount patches for nfs-utils I noticed
that it does not work when fsid=root, rootdir=/some/path/ and then "/" is being exported.
In this case next_mnt() is asked to find submounts of "/" but returns none.
In my opinion this wrong because every mount is a submount of "/".
The following change fixes the problem on my side but I'm not sure whether
"/" is a special case in mountd where next_mnt() has to bail out.
diff --git a/support/export/cache.c b/support/export/cache.c
index 2497d4f48df3..be20cb34adcb 100644
--- a/support/export/cache.c
+++ b/support/export/cache.c
@@ -410,13 +410,13 @@ static char *next_mnt(void **v, char *p)
*v = f;
} else
f = *v;
- while ((me = getmntent(f)) != NULL && l > 1) {
+ while ((me = getmntent(f)) != NULL && l >= 1) {
char *mnt_dir = nfsd_path_strip_root(me->mnt_dir);
if (!mnt_dir)
continue;
- if (strncmp(mnt_dir, p, l) == 0 && mnt_dir[l] == '/')
+ if (strncmp(mnt_dir, p, l) == 0 && (l == 1 || mnt_dir[l] == '/'))
return mnt_dir;
}
endmntent(f);
Comments? :-)
Thanks,
//richard
Hello,
My apologies... Eating some PTO...
On 3/8/23 10:05 AM, Richard Weinberger wrote:
> Hi!
>
> next_mnt() finds submounts below a given path p.
> While investigating into an issue in my crossmount patches for nfs-utils I noticed
> that it does not work when fsid=root, rootdir=/some/path/ and then "/" is being exported.
> In this case next_mnt() is asked to find submounts of "/" but returns none.
I'm not clear as what you are saying... "rootdir=/some/path/" is not an
export option.
>
> In my opinion this wrong because every mount is a submount of "/".
>
> The following change fixes the problem on my side but I'm not sure whether
> "/" is a special case in mountd where next_mnt() has to bail out.
>
> diff --git a/support/export/cache.c b/support/export/cache.c
> index 2497d4f48df3..be20cb34adcb 100644
> --- a/support/export/cache.c
> +++ b/support/export/cache.c
> @@ -410,13 +410,13 @@ static char *next_mnt(void **v, char *p)
> *v = f;
> } else
> f = *v;
> - while ((me = getmntent(f)) != NULL && l > 1) {
> + while ((me = getmntent(f)) != NULL && l >= 1) {
> char *mnt_dir = nfsd_path_strip_root(me->mnt_dir);
>
> if (!mnt_dir)
> continue;
>
> - if (strncmp(mnt_dir, p, l) == 0 && mnt_dir[l] == '/')
> + if (strncmp(mnt_dir, p, l) == 0 && (l == 1 || mnt_dir[l] == '/'))
> return mnt_dir;
> }
> endmntent(f);
>
> Comments? :-)
Putting this is the correct patch format including a Signed-off-by
signature... would help.
steved.
>
> Thanks,
> //richard
>
----- Ursprüngliche Mail -----
>> next_mnt() finds submounts below a given path p.
>> While investigating into an issue in my crossmount patches for nfs-utils I
>> noticed
>> that it does not work when fsid=root, rootdir=/some/path/ and then "/" is being
>> exported.
>> In this case next_mnt() is asked to find submounts of "/" but returns none.
> I'm not clear as what you are saying... "rootdir=/some/path/" is not an
> export option.
Sorry for being imprecise.
rootdir= is an nfs.conf exports option.
Thanks,
//richard
On 3/11/23 11:52 AM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>>> next_mnt() finds submounts below a given path p.
>>> While investigating into an issue in my crossmount patches for nfs-utils I
>>> noticed
>>> that it does not work when fsid=root, rootdir=/some/path/ and then "/" is being
>>> exported.
>>> In this case next_mnt() is asked to find submounts of "/" but returns none.
>> I'm not clear as what you are saying... "rootdir=/some/path/" is not an
>> export option.
>
> Sorry for being imprecise.
> rootdir= is an nfs.conf exports option.Point. But I still need the patch in the correct
format with the Signed-off-by...
steved.
>
> Thanks,
> //richard
>
----- Ursprüngliche Mail -----
> On 3/11/23 11:52 AM, Richard Weinberger wrote:
>> ----- Ursprüngliche Mail -----
>>>> next_mnt() finds submounts below a given path p.
>>>> While investigating into an issue in my crossmount patches for nfs-utils I
>>>> noticed
>>>> that it does not work when fsid=root, rootdir=/some/path/ and then "/" is being
>>>> exported.
>>>> In this case next_mnt() is asked to find submounts of "/" but returns none.
>>> I'm not clear as what you are saying... "rootdir=/some/path/" is not an
>>> export option.
>>
>> Sorry for being imprecise.
>> rootdir= is an nfs.conf exports option.Point. But I still need the patch in the
>> correct
> format with the Signed-off-by...
Well, the goal of my mail was not sending a ready-to-apply patch.
It was a question. To me next_mnt() looks wrong but I'm not sure whether
the current handling of "/" is desired for some special case I'm not aware of.
I'll happily send a patch after we agree that next_mnt() is wrong.
Thanks,
//richard
On 3/12/23 9:36 AM, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
>> On 3/11/23 11:52 AM, Richard Weinberger wrote:
>>> ----- Ursprüngliche Mail -----
>>>>> next_mnt() finds submounts below a given path p.
>>>>> While investigating into an issue in my crossmount patches for nfs-utils I
>>>>> noticed
>>>>> that it does not work when fsid=root, rootdir=/some/path/ and then "/" is being
>>>>> exported.
>>>>> In this case next_mnt() is asked to find submounts of "/" but returns none.
>>>> I'm not clear as what you are saying... "rootdir=/some/path/" is not an
>>>> export option.
>>>
>>> Sorry for being imprecise.
>>> rootdir= is an nfs.conf exports option.Point. But I still need the patch in the
>>> correct
>> format with the Signed-off-by...
>
> Well, the goal of my mail was not sending a ready-to-apply patch.
> It was a question. To me next_mnt() looks wrong but I'm not sure whether
> the current handling of "/" is desired for some special case I'm not aware of.
>
> I'll happily send a patch after we agree that next_mnt() is wrong.
I'm still trying to reproduce problem... I have
/etc/nfs.conf: rootdir=/export
/etc/exports:
/home *(rw,sec=sys:krb5:krb5i:krb5p)
/tmp *(rw,fsid=666,all_squash)
/ *(rw,fsid=root,all_squash)
I'm not seeing the problem... Where does the crossmount come in?
steved.
----- Ursprüngliche Mail -----
>> Well, the goal of my mail was not sending a ready-to-apply patch.
>> It was a question. To me next_mnt() looks wrong but I'm not sure whether
>> the current handling of "/" is desired for some special case I'm not aware of.
>>
>> I'll happily send a patch after we agree that next_mnt() is wrong.
> I'm still trying to reproduce problem... I have
>
> /etc/nfs.conf: rootdir=/export
>
> /etc/exports:
> /home *(rw,sec=sys:krb5:krb5i:krb5p)
> /tmp *(rw,fsid=666,all_squash)
> / *(rw,fsid=root,all_squash)
>
> I'm not seeing the problem... Where does the crossmount come in?
Chris reported the problem to me while he was testing my re-export/crossmount patches.
I can try reproducing without re-exporting later.
In theory you should see the problem as follows:
1. Have rootdir=/export in your nfs.conf
2. /export is some filesystem that contains more mounts
3. /export/fs1 is a different filesytem
4. /export/fs2 is a different filesytem too
5. /etc/exports contains: / *(rw,fsid=root,all_squash,crossmount)
Client mounts / to /nfs and then tries to access /nfs/fs1.
Then nfsd_fh() iterates over all exports, finds one with NFSEXP_CROSSMOUNT set.
Using next_mnt() it finds possible sub-mounts. But for the "/" case next_mnt()
returns none -> nfsd_fh() fails -> client cannot enter /nfs/fs1.
Thanks,
//richard
----- Ursprüngliche Mail -----
>> I'm still trying to reproduce problem... I have
>>
>> /etc/nfs.conf: rootdir=/export
>>
>> /etc/exports:
>> /home *(rw,sec=sys:krb5:krb5i:krb5p)
>> /tmp *(rw,fsid=666,all_squash)
>> / *(rw,fsid=root,all_squash)
>>
>> I'm not seeing the problem... Where does the crossmount come in?
>
> Chris reported the problem to me while he was testing my re-export/crossmount
> patches.
> I can try reproducing without re-exporting later.
Finally I found some cycles to reproduce without my re-exporting setup.
1. /etc/exports contains:
/ *(rw,crossmnt,no_subtree_check,fsid=root)
2. /etc/nfs.conf contains:
[exports]
rootdir=/nfs_srv
3. Mounts:
/root/fs1.ext4 on /nfs_srv type ext4 (rw,relatime)
/root/fs2.ext4 on /nfs_srv/fs2 type ext4 (rw,relatime)
4. On the client:
# ls /nfs_client/fs2
ls: cannot open directory '/nfs_client/fs2': Stale file handle
I'll send a proper patch ASAP.
next_mnt() has to deal with "/" too.
Thanks,
//richard