[PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Jan Kara]

According to specification
mkdir d; ln -s d a; open("a/", O_NOFOLLOW | O_RDONLY)
should return success but currently it did return ELOOP. Fix the code to ignore
O_NOFOLLOW in case the provided path has trailing slashes. This is a regression
caused by path lookup cleanup patch series.

CC: stable@kernel.org
CC: Andrew Morton <akpm@linux-foundation.org>
CC: Al Viro <viro@zeniv.linux.org.uk>
Reported-by: tolzmann@molgen.mpg.de
Acked-by: Miklos Szeredi <mszeredi@suse.cz>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 Since Al doesn't seem to react for a few days and this is kind of nasty bug,
I'm sending it directly...

 fs/namei.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index a7dce91..16df727 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1641,7 +1641,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 	if (nd->last.name[nd->last.len]) {
 		if (open_flag & O_CREAT)
 			goto exit;
-		nd->flags |= LOOKUP_DIRECTORY;
+		nd->flags |= LOOKUP_DIRECTORY | LOOKUP_FOLLOW;
 	}
 
 	/* just plain open? */
@@ -1830,6 +1830,8 @@ reval:
 	}
 	if (open_flag & O_DIRECTORY)
 		nd.flags |= LOOKUP_DIRECTORY;
+	if (!(open_flag & O_NOFOLLOW))
+		nd.flags |= LOOKUP_FOLLOW;
 	filp = do_last(&nd, &path, open_flag, acc_mode, mode, pathname);
 	while (unlikely(!filp)) { /* trailing symlink */
 		struct path holder;
@@ -1837,7 +1839,7 @@ reval:
 		void *cookie;
 		error = -ELOOP;
 		/* S_ISDIR part is a temporary automount kludge */
-		if ((open_flag & O_NOFOLLOW) && !S_ISDIR(inode->i_mode))
+		if (!(nd.flags & LOOKUP_FOLLOW) && !S_ISDIR(inode->i_mode))
 			goto exit_dput;
 		if (count++ == 32)
 			goto exit_dput;
-- 
1.6.4.2

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Linus Torvalds]

On Thu, 13 May 2010, Jan Kara wrote:
>
> According to specification
> mkdir d; ln -s d a; open("a/", O_NOFOLLOW | O_RDONLY)
> should return success but currently it did return ELOOP. Fix the code to ignore
> O_NOFOLLOW in case the provided path has trailing slashes. This is a regression
> caused by path lookup cleanup patch series.
> 
> CC: stable@kernel.org

Hmm? Is this correct? Isn't the bug introduced in this merge window, and 
thus not relevant for stable?

		Linus

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Jan Kara]

On Thu 13-05-10 08:24:59, Linus Torvalds wrote:
> 
> 
> On Thu, 13 May 2010, Jan Kara wrote:
> >
> > According to specification
> > mkdir d; ln -s d a; open("a/", O_NOFOLLOW | O_RDONLY)
> > should return success but currently it did return ELOOP. Fix the code to ignore
> > O_NOFOLLOW in case the provided path has trailing slashes. This is a regression
> > caused by path lookup cleanup patch series.
> > 
> > CC: stable@kernel.org
> 
> Hmm? Is this correct? Isn't the bug introduced in this merge window, and 
> thus not relevant for stable?
  Ah, you're right! I've seen dates in the patches around December so I
automatically thought the series went to 2.6.33 but checking git logs and
the actual source code of 2.6.33 it went in later. I'm sorry for the
confusion.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Andrew Morton]

On Thu, 13 May 2010 17:41:35 +0200 Jan Kara <jack@suse.cz> wrote:

> On Thu 13-05-10 08:24:59, Linus Torvalds wrote:
> > 
> > 
> > On Thu, 13 May 2010, Jan Kara wrote:
> > >
> > > According to specification
> > > mkdir d; ln -s d a; open("a/", O_NOFOLLOW | O_RDONLY)
> > > should return success but currently it did return ELOOP. Fix the code to ignore
> > > O_NOFOLLOW in case the provided path has trailing slashes. This is a regression
> > > caused by path lookup cleanup patch series.
> > > 
> > > CC: stable@kernel.org
> > 
> > Hmm? Is this correct? Isn't the bug introduced in this merge window, and 
> > thus not relevant for stable?
>   Ah, you're right! I've seen dates in the patches around December so I
> automatically thought the series went to 2.6.33 but checking git logs and
> the actual source code of 2.6.33 it went in later. I'm sorry for the
> confusion.

Yes, it's a bit tricky (for me, at least) to work out "which kernel version did
that patch go into" via git.

I just keep lots of kernel trees around and poke about with `patch
--dry-run'.  PITA.

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Christoph Hellwig]

On Thu, May 13, 2010 at 08:43:35AM -0400, Andrew Morton wrote:
> Yes, it's a bit tricky (for me, at least) to work out "which kernel version did
> that patch go into" via git.

git-describe <revid>

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[James Bottomley]

On Thu, 2010-05-13 at 11:47 -0400, Christoph Hellwig wrote:
> On Thu, May 13, 2010 at 08:43:35AM -0400, Andrew Morton wrote:
> > Yes, it's a bit tricky (for me, at least) to work out "which kernel version did
> > that patch go into" via git.
> 
> git-describe <revid>

Actually, that will print the version it was applied to.  To find out
the version it was pulled into, you need git-describe --contains <revid>

James

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Jan Kara]

On Thu 13-05-10 08:43:35, Andrew Morton wrote:
> On Thu, 13 May 2010 17:41:35 +0200 Jan Kara <jack@suse.cz> wrote:
> 
> > On Thu 13-05-10 08:24:59, Linus Torvalds wrote:
> > > 
> > > 
> > > On Thu, 13 May 2010, Jan Kara wrote:
> > > >
> > > > According to specification
> > > > mkdir d; ln -s d a; open("a/", O_NOFOLLOW | O_RDONLY)
> > > > should return success but currently it did return ELOOP. Fix the code to ignore
> > > > O_NOFOLLOW in case the provided path has trailing slashes. This is a regression
> > > > caused by path lookup cleanup patch series.
> > > > 
> > > > CC: stable@kernel.org
> > > 
> > > Hmm? Is this correct? Isn't the bug introduced in this merge window, and 
> > > thus not relevant for stable?
> >   Ah, you're right! I've seen dates in the patches around December so I
> > automatically thought the series went to 2.6.33 but checking git logs and
> > the actual source code of 2.6.33 it went in later. I'm sorry for the
> > confusion.
> 
> Yes, it's a bit tricky (for me, at least) to work out "which kernel version did
> that patch go into" via git.
  Well, if you know the commit id, "git describe --contains <commit-id>"
tells what you need. But sometimes I'm too lazy to use "git describe" and
sometimes I forget "--contains" which then returns the kernel version on
which the patch was based - not quite what I'm interested in...

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Linus Torvalds]

On Thu, 13 May 2010, Andrew Morton wrote:
>
> >   Ah, you're right! I've seen dates in the patches around December so I
> > automatically thought the series went to 2.6.33 but checking git logs and
> > the actual source code of 2.6.33 it went in later. I'm sorry for the
> > confusion.
> 
> Yes, it's a bit tricky (for me, at least) to work out "which kernel version did
> that patch go into" via git.
> 
> I just keep lots of kernel trees around and poke about with `patch
> --dry-run'.  PITA.

What I did to double-check was:

	git log fs/namei.c

to find the commit series by Al (obviously, you can do it other ways too, 
but that was the easy way). Then, when you find the commit  just do

	git name-rev 1f36f774b22a0ceb7dd33eca626746c81a97b6a5

which gives us

	1f36f774b22a0ceb7dd33eca626746c81a97b6a5 tags/v2.6.34-rc1~195^2

ie that commit is reachable from v2.6.34-rc1, not from any stable kernel.

(Or alternatively, use "git describe", and get "v2.6.33-5088-g1f36f77", 
which means that it is v2.6.33 plus 5088 commits).

And as usual, there are other ways. One particularly obscure one is to say

	git log --tags --source --author=viro fs/namei.c

which basically says "show only commits by viro in fs/namei.c, start from 
all tags, and for each commit, show _which_ tag the commit was reached 
from". It's not perfect, but it does it in one go. With "--oneline", you'd 
have gotten a listing like

	3e297b6 v2.6.34-rc3 Restore LOOKUP_DIRECTORY hint handling in final lookup on open()
	781b167 v2.6.34-rc2 Fix a dumb typo - use of & instead of &&
	1f36f77 v2.6.34-rc2 Switch !O_CREAT case to use of do_last()
	def4af3 v2.6.34-rc2 Get rid of symlink body copying
	3866248 v2.6.34-rc2 Finish pulling of -ESTALE handling to upper level in do_filp_open()
	806b681 v2.6.34-rc2 Turn do_link spaghetty into a normal loop
	10fa8e6 v2.6.34-rc2 Unify exits in O_CREAT handling
	9e67f36 v2.6.34-rc2 Kill is_link argument of do_last()
	...

so you see into which -rc the different patches from Al went.

			Linus

Re: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

[Al Viro]

On Thu, May 13, 2010 at 12:52:57PM +0200, Jan Kara wrote:
>  Since Al doesn't seem to react for a few days and this is kind of nasty bug,
> I'm sending it directly...

Belated ACK.  Should've noticed it when LOOKUP_DIRECTORY crap had shown up
weeks ago ;-/  Sorry.