[linux-cifs-client] [PATCH] cifs: add comments explaining cifs_new_fileinfo behavior
jlayton at samba.org
Tue May 11 05:33:28 MDT 2010
On Tue, 11 May 2010 16:33:44 +0530
Suresh Jayaraman <sjayaraman at suse.de> wrote:
> On 05/11/2010 04:17 PM, Jeff Layton wrote:
> > On Mon, 10 May 2010 20:00:05 +0530
> > Suresh Jayaraman <sjayaraman at suse.de> wrote:
> >> The comments make it clear the otherwise subtle behavior of cifs_new_fileinfo().
> >> Signed-off-by: Suresh Jayaraman <sjayaraman at suse.de>
> >> --
> >> fs/cifs/dir.c | 18 ++++++++++++++++--
> >> 1 files changed, 16 insertions(+), 2 deletions(-)
> >> diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
> >> index d791d07..bd363df 100644
> >> --- a/fs/cifs/dir.c
> >> +++ b/fs/cifs/dir.c
> >> @@ -129,6 +129,12 @@ cifs_bp_rename_retry:
> >> return full_path;
> >> }
> >> +/*
> >> + * When called with struct file pointer set to NULL, there is no way we could
> >> + * update file->private_data, but getting it stuck on openFileList provides a
> >> + * way to access it from cifs_fill_filedata and thereby set file->private_data
> >> + * from cifs_open.
> >> + */
> > The comments help, but this code has bothered me for some time. Is it
> > possible for the create to return success and for something else to
> > happen such that the cifs_open is never called? I'd imagine that it is,
> > and if so, then this this open file will be "leaked".
> I asked Shirish exactly the same question while discussing in the
> #samba-technical irc channel. He does not see a leak, but thought you or
> Steve will have a better idea..
> Steve: is such a situation not possible at all?
I'm pretty sure it is possible and may even be somewhat likely.
Consider this situation. vfs_create gets called from
__open_namei_create. Something like this:
inode create operation
...after this, __open_namei_create calls may_open to check permissions
and can return an error. If that occurs, then I don't think the open op
will ever be called.
I think you can probably reproduce this by doing something like this:
Have samba export a world-writable directory. Mount up the share with a
user's credentials. Make sure that unix extensions are enabled. Have a
different user do something like this into a file on the mount:
echo foo > /path/to/share/testfile
It's probable that the file will be created, but the open-for-write
permission check will fail and the open file will be left dangling.
> > I think we should be using lookup_instantiate_filp here instead and
> > doing this similarly to the open on lookup code. There is some
> > precedent for this -- fuse_create_open does this and it gets called via
> > the .create operation.
> I thought about this briefly but was not sure whether it can be called
> from cifs_create. I'll need to dig more.
Jeff Layton <jlayton at samba.org>
More information about the linux-cifs-client