All of lore.kernel.org
 help / color / mirror / Atom feed
From: "J. Bruce Fields" <bfields@fieldses.org>
To: NeilBrown <neilb@suse.de>
Cc: NFS <linux-nfs@vger.kernel.org>
Subject: Re: Should exportfs/mountd cope with case-insensitive directory names.
Date: Thu, 3 Apr 2014 09:09:09 -0400	[thread overview]
Message-ID: <20140403130909.GA24700@fieldses.org> (raw)
In-Reply-To: <20140403164652.5d7770ad@notabene.brown>

On Thu, Apr 03, 2014 at 04:46:52PM +1100, NeilBrown wrote:
> 
> Hi,
>  I came across an interesting issue recently.
> 
> Suppose you have a filesystem which supports case-insensitive file names
> (like VFAT, but in this particular case 'NSS' - Novell Storage Services).
> 
> And support you export some subdirectory (or sub-volume) using a
> non-canonical name.
> i.e. you "mkdir /path/export", the put "/path/EXPORT" in /etc/exports and
> mount  server:/path/EXPORT from some client.
> 
> This all works until the export cache times out and the kernel asks mountd
> if "/path/export" is exported.  mountd says "no" and suddenly all accesses
> fail.
> 
> I don't think much of case-insensitive file names, but I suspect we should
> either make this work, it issue a warning as to why it is failing.
> A simple work around is to export the canonical name and use it when
> mounting.  But if the sysadmin doesn't know they need to, they are unlikely
> to guess.
> 
> I don't think there is any API to test if a name is canonical, or to get the
> canonical name, so I cannot see any way in advance to see if this problem
> situation has arisen.  So the only option I can think of is to fix it.
> 
> The following patch (or something much like it for an older nfs-utils) seems
> to do the trick.
> 
> What do people think?  Is this a reasonable thing to do?  Is it likely to
> have negative consequences that I haven't thought of?

Neat-o.

Are we adding a stat of every export in the export table where there
wasn't one before?  But those should typically be cached, so maybe
there's no problem.

I'm not super-fond of the combining the comparison of two paths and
truncating of one into the same function, though I see why it's
convenient.

--b.

> 
> Thanks,
> NeilBrown
> 
> 
> 
> diff --git a/utils/mountd/cache.c b/utils/mountd/cache.c
> index 9a1bb2767ac2..2d91db76b867 100644
> --- a/utils/mountd/cache.c
> +++ b/utils/mountd/cache.c
> @@ -377,6 +377,28 @@ static char *next_mnt(void **v, char *p)
>  	return me->mnt_dir;
>  }
>  
> +static int same_path(char *child, char *parent, int len)
> +{
> +	static char p[PATH_MAX];
> +	struct stat sc, sp;
> +	if (len <= 0)
> +		len = strlen(child);
> +	strncpy(p, child, len);
> +	p[len] = 0;
> +	if (strcmp(p, parent) == 0)
> +		return 1;
> +
> +	if (lstat(p, &sc) != 0)
> +		return 0;
> +	if (lstat(parent, &sp) != 0)
> +		return 0;
> +	if (sc.st_dev != sp.st_dev)
> +		return 0;
> +	if (sc.st_ino != sp.st_ino)
> +		return 0;
> +	return 1;
> +}
> +
>  static int is_subdirectory(char *child, char *parent)
>  {
>  	/* Check is child is strictly a subdirectory of
> @@ -387,7 +409,7 @@ static int is_subdirectory(char *child, char *parent)
>  	if (strcmp(parent, "/") == 0 && child[1] != 0)
>  		return 1;
>  
> -	return (strncmp(child, parent, l) == 0 && child[l] == '/');
> +	return (same_path(child, parent, l) && child[l] == '/');
>  }
>  
>  static int path_matches(nfs_export *exp, char *path)
> @@ -396,7 +418,7 @@ static int path_matches(nfs_export *exp, char *path)
>  	 * exact match, or does the export have CROSSMOUNT, and path
>  	 * is a descendant?
>  	 */
> -	return strcmp(path, exp->m_export.e_path) == 0
> +	return same_path(path, exp->m_export.e_path, 0)
>  		|| ((exp->m_export.e_flags & NFSEXP_CROSSMOUNT)
>  		    && is_subdirectory(path, exp->m_export.e_path));
>  }



  reply	other threads:[~2014-04-03 13:09 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-03  5:46 Should exportfs/mountd cope with case-insensitive directory names NeilBrown
2014-04-03 13:09 ` J. Bruce Fields [this message]
2014-04-03 16:02   ` Boaz Harrosh
2014-04-03 17:16     ` J. Bruce Fields
2014-04-03 17:24       ` J. Bruce Fields
2014-04-03 21:21         ` NeilBrown
2014-04-03 21:36           ` J. Bruce Fields
2014-04-03 22:13             ` NeilBrown
2014-04-04 14:34               ` J. Bruce Fields

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140403130909.GA24700@fieldses.org \
    --to=bfields@fieldses.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.