| Message ID | 20190603171227.29148-3-trond.myklebust@hammerspace.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Incremental against [exports] rootdir patchset | expand |
On Mon, Jun 03, 2019 at 01:12:26PM -0400, Trond Myklebust wrote: > When attempting to strip the root path, we should first canonicalise > the root pathname. > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > --- > support/misc/nfsd_path.c | 17 +++++++++++++---- > 1 file changed, 13 insertions(+), 4 deletions(-) > > diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c > index 2f41a793c534..9b38dd96007f 100644 > --- a/support/misc/nfsd_path.c > +++ b/support/misc/nfsd_path.c > @@ -1,6 +1,7 @@ > #include <errno.h> > #include <sys/types.h> > #include <sys/stat.h> > +#include <limits.h> > #include <stdlib.h> > #include <unistd.h> > > @@ -62,13 +63,21 @@ nfsd_path_nfsd_rootdir(void) > char * > nfsd_path_strip_root(char *pathname) > { > + char buffer[PATH_MAX]; > const char *dir = nfsd_path_nfsd_rootdir(); > char *ret; > > - ret = strstr(pathname, dir); > - if (!ret || ret != pathname) > - return pathname; > - return pathname + strlen(dir); > + if (!dir) > + goto out; > + if (realpath(dir, buffer)) { > + ret = strstr(pathname, buffer); > + if (ret == pathname) > + return pathname + strlen(dir); > + } else > + xlog(D_GENERAL, "%s: failed to resolve path %s: %m", > + __func__, dir); > +out: > + return pathname; I still don't get this. So in the case strstr doesn't find anything, it returns the path unchanged. That means that if the next_mnt() caller asks whether there are any mounts underneath /rootdir/a/b, and nextdir finds a mountpoint at /a/b/c, it can return that, right? --b. > } > > char * > -- > 2.21.0
On Tue, 2019-06-04 at 11:46 -0400, J. Bruce Fields wrote: > On Mon, Jun 03, 2019 at 01:12:26PM -0400, Trond Myklebust wrote: > > When attempting to strip the root path, we should first > > canonicalise > > the root pathname. > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > --- > > support/misc/nfsd_path.c | 17 +++++++++++++---- > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c > > index 2f41a793c534..9b38dd96007f 100644 > > --- a/support/misc/nfsd_path.c > > +++ b/support/misc/nfsd_path.c > > @@ -1,6 +1,7 @@ > > #include <errno.h> > > #include <sys/types.h> > > #include <sys/stat.h> > > +#include <limits.h> > > #include <stdlib.h> > > #include <unistd.h> > > > > @@ -62,13 +63,21 @@ nfsd_path_nfsd_rootdir(void) > > char * > > nfsd_path_strip_root(char *pathname) > > { > > + char buffer[PATH_MAX]; > > const char *dir = nfsd_path_nfsd_rootdir(); > > char *ret; > > > > - ret = strstr(pathname, dir); > > - if (!ret || ret != pathname) > > - return pathname; > > - return pathname + strlen(dir); > > + if (!dir) > > + goto out; > > + if (realpath(dir, buffer)) { > > + ret = strstr(pathname, buffer); > > + if (ret == pathname) > > + return pathname + strlen(dir); > > + } else > > + xlog(D_GENERAL, "%s: failed to resolve path %s: %m", > > + __func__, dir); > > +out: > > + return pathname; > > I still don't get this. > > So in the case strstr doesn't find anything, it returns the path > unchanged. > > That means that if the next_mnt() caller asks whether there are any > mounts underneath /rootdir/a/b, and nextdir finds a mountpoint at > /a/b/c, it can return that, right? > Ack. Sending out a v2 of these patches. Thanks Bruce!
On Tue, Jun 04, 2019 at 05:58:59PM +0000, Trond Myklebust wrote: > On Tue, 2019-06-04 at 11:46 -0400, J. Bruce Fields wrote: > > On Mon, Jun 03, 2019 at 01:12:26PM -0400, Trond Myklebust wrote: > > > When attempting to strip the root path, we should first > > > canonicalise > > > the root pathname. > > > > > > Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> > > > --- > > > support/misc/nfsd_path.c | 17 +++++++++++++---- > > > 1 file changed, 13 insertions(+), 4 deletions(-) > > > > > > diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c > > > index 2f41a793c534..9b38dd96007f 100644 > > > --- a/support/misc/nfsd_path.c > > > +++ b/support/misc/nfsd_path.c > > > @@ -1,6 +1,7 @@ > > > #include <errno.h> > > > #include <sys/types.h> > > > #include <sys/stat.h> > > > +#include <limits.h> > > > #include <stdlib.h> > > > #include <unistd.h> > > > > > > @@ -62,13 +63,21 @@ nfsd_path_nfsd_rootdir(void) > > > char * > > > nfsd_path_strip_root(char *pathname) > > > { > > > + char buffer[PATH_MAX]; > > > const char *dir = nfsd_path_nfsd_rootdir(); > > > char *ret; > > > > > > - ret = strstr(pathname, dir); > > > - if (!ret || ret != pathname) > > > - return pathname; > > > - return pathname + strlen(dir); > > > + if (!dir) > > > + goto out; > > > + if (realpath(dir, buffer)) { > > > + ret = strstr(pathname, buffer); > > > + if (ret == pathname) > > > + return pathname + strlen(dir); > > > + } else > > > + xlog(D_GENERAL, "%s: failed to resolve path %s: %m", > > > + __func__, dir); > > > +out: > > > + return pathname; > > > > I still don't get this. > > > > So in the case strstr doesn't find anything, it returns the path > > unchanged. > > > > That means that if the next_mnt() caller asks whether there are any > > mounts underneath /rootdir/a/b, and nextdir finds a mountpoint at > > /a/b/c, it can return that, right? > > > > Ack. Sending out a v2 of these patches. Oh, good, thanks, I thought I was going crazy. (Always a possibility, especially when I'm looking at code.) --b.
diff --git a/support/misc/nfsd_path.c b/support/misc/nfsd_path.c index 2f41a793c534..9b38dd96007f 100644 --- a/support/misc/nfsd_path.c +++ b/support/misc/nfsd_path.c @@ -1,6 +1,7 @@ #include <errno.h> #include <sys/types.h> #include <sys/stat.h> +#include <limits.h> #include <stdlib.h> #include <unistd.h> @@ -62,13 +63,21 @@ nfsd_path_nfsd_rootdir(void) char * nfsd_path_strip_root(char *pathname) { + char buffer[PATH_MAX]; const char *dir = nfsd_path_nfsd_rootdir(); char *ret; - ret = strstr(pathname, dir); - if (!ret || ret != pathname) - return pathname; - return pathname + strlen(dir); + if (!dir) + goto out; + if (realpath(dir, buffer)) { + ret = strstr(pathname, buffer); + if (ret == pathname) + return pathname + strlen(dir); + } else + xlog(D_GENERAL, "%s: failed to resolve path %s: %m", + __func__, dir); +out: + return pathname; } char *
When attempting to strip the root path, we should first canonicalise the root pathname. Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com> --- support/misc/nfsd_path.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)