Re: [Bugme-new] [Bug 15909] New: open("a/", O_NOFOLLOW) fails with ELOOP if "a" is a symbolic link to a directory.

[Jan Kara <jack@suse.cz>]

On Thu 06-05-10 14:30:02, Andrew Morton wrote:
> 
> (switched to email.  Please respond via emailed reply-to-all, not via the
> bugzilla web interface).
> 
> On Wed, 5 May 2010 13:01:22 GMT
> bugzilla-daemon@bugzilla.kernel.org wrote:
> 
> > https://bugzilla.kernel.org/show_bug.cgi?id=15909
> > 
> >            Summary: open("a/",O_NOFOLLOW) fails with ELOOP if "a" is a
> >                     symbolic link to a directory.
> >            Product: File System
> >            Version: 2.5
> >     Kernel Version: 2.6.34-rc6
> >           Platform: All
> >         OS/Version: Linux
> >               Tree: Mainline
> >             Status: NEW
> >           Severity: high
> >           Priority: P1
> >          Component: Other
> >         AssignedTo: fs_other@kernel-bugs.osdl.org
> >         ReportedBy: tolzmann@molgen.mpg.de
> >         Regression: No
> > 
> > 
> > mkdir c
> > ln -s c a
> > 
> > f=open("a/",O_RDONLY+O_NOFOLLOW)
> > 
> > fails with ELOOP. However, this open should behave like open("a/.") not like
> > open("a") according to path_resolution(7). In kernel version 2.6.32 the open
> > worked as documented. 
> > 
> > On a higher level this bug makes
> > 
> > find a/
> > 
> > to fail.
> > 
> 
> It sounds like this 2.6.32->2.6.34-rc6 regression could have pretty
> serious ramifications for some users.  Does anyone know whcih commit
> might have caused it?
  The patch below fixes the issue for me but someone should have a look
at it because I'm not really an expert in that code and the code paths are so
twisted that my mind is currently tied into a knot ;).

									Honza
---

>From d53d3cc6488d9135bb69c3ff7e034b3b624866ed Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Tue, 11 May 2010 16:34:25 +0200
Subject: [PATCH] vfs: Fix O_NOFOLLOW behavior for paths with trailing slashes

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
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/namei.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index a7dce91..4cc7fbc 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? */
@@ -1722,7 +1722,7 @@ static struct file *do_last(struct nameidata *nd, struct path *path,
 
 	if (__follow_mount(path)) {
 		error = -ELOOP;
-		if (open_flag & O_NOFOLLOW)
+		if (!(nd->flags & LOOKUP_FOLLOW))
 			goto exit_dput;
 	}
 
@@ -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