On Thu, Oct 13, 2011 at 12:27 PM, Jeff Layton <[email protected]> wrote:
>
> On Wed, 28 Sep 2011 00:28:10 +0300
> Dan Carpenter <[email protected]> wrote:
>
> > This solves a problem for where a user without write permissions
> > was creating empty files.  This function was supposed to do a lookup
> > only, the create happens later.
> >
>
> Not quite. This uses the open-intent goop in the VFS layer that Al Viro
> is trying to get rid of. The idea here is that doing a lookup just to
> do an open is a waste, when you could just attempt the open. There's no
> real reason to exempt creates from that if cifs used a sane permissions
> model...
>
> > Signed-off-by: Dan Carpenter <[email protected]>
> >
> > diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> > index 72d448b..8515afe 100644
> > --- a/fs/cifs/dir.c
> > +++ b/fs/cifs/dir.c
> > @@ -566,11 +566,13 @@ cifs_lookup(struct inode *parent_dir_inode, struct 
> > dentry *direntry,
> >               if (nd && !(nd->flags & LOOKUP_DIRECTORY) &&
> >                    (nd->flags & LOOKUP_OPEN) && !pTcon->broken_posix_open &&
> >                    (nd->intent.open.file->f_flags & O_CREAT)) {
> > +                     unsigned int f_flags;
> > +
> > +                     f_flags = (nd->intent.open.file->f_flags & ~O_CREAT);
> >                       rc = cifs_posix_open(full_path, &newInode,
> >                                       parent_dir_inode->i_sb,
> >                                       nd->intent.open.create_mode,
> > -                                     nd->intent.open.file->f_flags, 
> > &oplock,
> > -                                     &fileHandle, xid);
> > +                                     f_flags, &oplock, &fileHandle, xid);
>
>        ...so this makes it only do the open at lookup time if the
>        file already exists.
>
>        I suspect the real problem here is that cifs is trying to
>        enforce permissions on the client, which happens after the
>        lookup.
>
>        If the client simply allowed the server to handle the
>        permissions (and used the right credentials for each user),
>        then this would probably work fine. Another nail in the coffin
>        for the whole model of client side permissions enforcement,
>        IMO...

so it works with "noperm" on.  Presumably we should be recommending
turning noperm and multiuser on (and probably cifsacl if the server is
not samba).

I still would much prefer being able to hook in to the create and open
"closer" to the syscall itself rather than after it is broken down
into (unneeded) lower level fs entry points (for getattr/setattr and
create vs. open).

--
Thanks,

Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to