linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Ian Kent <raven@themaw.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	rui.xiang@huawei.com,
	autofs mailing list <autofs@vger.kernel.org>,
	Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/3] autofs4 - fix device ioctl mount lookup
Date: Wed, 4 Sep 2013 03:00:13 +0100	[thread overview]
Message-ID: <20130904020013.GF13318@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20130904010301.GE13318@ZenIV.linux.org.uk>

On Wed, Sep 04, 2013 at 02:03:02AM +0100, Al Viro wrote:

> Ewwww...   NAK in that form.  Just what will happen if the last component
> given to that sucker will be . or .., for starters?  Or a symlink, with
> '/' added to it to force following the damn thing?

OK, in more details:
	* we are changing a user-visible ABI here - kern_path() with
LOOKUP_FOLLOW had been there since the introduction of that misc
device (OK, it used to be path_lookup(), but that changes nothing).
IOW, we used to follow symlinks there, now we do not.
	* pathname may end on more than one slash.  Modified that
way, the code won't do anything good (not to mention that d_lookup()
on an empty string is an interesting torture test for fs/dcache.c;
probably you'll just find nothing, but normally the function is never
called with such arguments, so...).  In any case it's an ABI change.
	* pathname may end with something/. or something/..; again,
d_lookup() won't find you anything now, so we have an ABI change.

That aside, I'm really not happy with this kind of games; this stuff clearly
belongs in fs/namei.c where we can simply see the last component.  Doing that
on the level of "let's scan the pathname for slashes, etc." is just plain
wrong.  Let's step back for a minute here; what are you trying to do?
You have a pathname that should resolve to a mountpoint, without triggering
automount (or crossing into the mountpoint, for that matter) and you want
struct path for the bottom of that mount stack?  Or is it something
completely different?

  reply	other threads:[~2013-09-04  2:00 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-09-04  0:54 [PATCH 1/3] autofs4 - fix device ioctl mount lookup Ian Kent
2013-09-04  0:55 ` [PATCH 2/3] autofs: fix the return value of autofs4_fill_super Ian Kent
2013-09-04  0:55 ` [PATCH 3/3] autofs: use IS_ROOT to replace root dentry checks Ian Kent
2013-09-04  1:03 ` [PATCH 1/3] autofs4 - fix device ioctl mount lookup Al Viro
2013-09-04  2:00   ` Al Viro [this message]
2013-09-04  2:18     ` Linus Torvalds
2013-09-04  2:26       ` Al Viro
2013-09-04  2:42         ` Al Viro
2013-09-04  3:53           ` Ian Kent
2013-09-04  4:07             ` Ian Kent
2013-09-04 10:35               ` Jeff Layton
2013-09-06  8:38           ` Ian Kent
2013-09-06  8:54             ` Ian Kent
2013-09-06 10:11               ` Jeff Layton
2013-09-04  2:46       ` Ian Kent

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=20130904020013.GF13318@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=autofs@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raven@themaw.net \
    --cc=rui.xiang@huawei.com \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).