Re: [PATCH 2/3] mountd: Ensure nfsd_path_strip_root() uses the canonicalised path

[Trond Myklebust <trondmy@hammerspace.com>]

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!

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com