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@xxxxxxx>]

On Tue 11-05-10 17:48:51, Jan Kara wrote:
> 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@xxxxxxxxxxxxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxx
> > >         ReportedBy: tolzmann@xxxxxxxxxxxxx
> > >         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@xxxxxxx>
> 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@xxxxxxxxxx
> Signed-off-by: Jan Kara <jack@xxxxxxx>
...
  BTW: It might be worthwhile to add the attached testcase to LTP?

									Honza
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR

#define _GNU_SOURCE
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/types.h>

int do_test(char *p)
{
  int err;
  char path[16];

  strcpy(path, p);
  err = open(path, O_NOFOLLOW | O_RDONLY);
  if (err >= 0) {
    fprintf(stderr, "open(\"%s\", O_NOFOLLOW | O_RDONLY) did not fail!\n", path);
    return 1;
  }
  strcat(path, "/");
  err = open(path, O_NOFOLLOW | O_RDONLY);
  if (err < 0) {
    fprintf(stderr, "open(\"%s\", O_NOFOLLOW | O_RDONLY) failed with %d\n", path, errno);
    return 1;
  }
  strcat(path, ".");
  err = open(path, O_NOFOLLOW | O_RDONLY);
  if (err < 0) {
    fprintf(stderr, "open(\"%s\", O_NOFOLLOW | O_RDONLY) failed with %d\n", path, errno);
    return 1;
  }
  return 0;
}

int main(void)
{
  if (mkdir("d", 0700) < 0) {
    perror("mkdir");
    return 1;
  }
  if (symlink("d", "a") < 0) {
    perror("link");
    return 1;
  }
  if (do_test("a"))
    return 1;
  return 0;
}