Re: new regression from cifs open on lookup patches

On Fri, 22 May 2009 08:42:35 -0500
Shirish Pargaonkar <[email protected]> wrote:

> On Fri, May 22, 2009 at 6:48 AM, Jeff Layton <[email protected]> wrote:
> > I've run across a new regression due to the open on lookup patches when
> > working against a server with functioning posix open calls. Prior to
> > those patches, opening a directory would simply result in a
> > revalidation of the dir, at most this would mean a QPathInfo call would
> > go out on the wire.
> >
> > Since the introduction of these patches, opening a directory causes an
> > actual open call to go out on the wire for the directory. In general,
> > the server will then return a status of NT_STATUS_FILE_IS_A_DIRECTORY
> > that gets translated to -EACCES.
> >
> > The main place I've noticed this is when trying recursively remove a
> > directory (rm -rf). rm will openat(AT_FDCWD, ...) the dir, and then try
> > to walk the subdirectories. That fails. Backing out the open intent
> > patches fixes the problem.
> >
> > This will definitely need to be fixed before 2.6.30 is released, as I'm
> > certain this will break more than "rm". Attaching a testcase that can
> > be used to test this.
> >
> > --
> > Jeff Layton <[email protected]>
> >
> 
> Jeff,
> 
> Looking into this, but as I see, with this check in cifs_lookup
>  !(nd->flags & (LOOKUP_PARENT | LOOKUP_DIRECTORY))
> posix open will not work on a directory, only on a file.

In fact, this new code in cifs_lookup does not look right to me:

    if (!((nd->intent.open.flags & O_CREAT) && (nd->intent.open.flags & O_EXCL))) {

...why are we skipping this for O_EXCL? We send SMB_O_EXCL on the wire,
so if the file already exists then the server should just return an
error, correct?

-- 
Jeff Layton <[email protected]>