| Message ID | 1497292229.221220.1678287959937.JavaMail.zimbra@nod.at (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mountd: Possible bug in next_mnt() | expand |
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
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);