[linux-cifs-client] [PATCH] cifs: add comments explaining cifs_new_fileinfo behavior
Suresh Jayaraman
sjayaraman at suse.de
Tue May 11 06:48:08 MDT 2010
On 05/11/2010 05:03 PM, Jeff Layton wrote:
> On Tue, 11 May 2010 16:33:44 +0530
> Suresh Jayaraman <sjayaraman at suse.de> wrote:
>
>>>>
>>>> +/*
>>>> + * 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:
>
> do_last
> __open_namei_create
> vfs_create
> 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.
>
A quick test shows it is not leaking atleast in this case. What happens is:
cifs_lookup() to the file returns NULL
cifs_posix_open()
CIFSPOSIXCreate() (file gets created)
cifs_new_fileinfo() (updated the openFileList)
lookup_instantiate_filp (gets the filep, calls cifs_open)
followed by a cifs_close
On the wire, I see SET_PATH_INFO with Level Of Interest set to Set File
Posix Open call followed by a Close.
Thanks,
--
Suresh Jayaraman
More information about the linux-cifs-client
mailing list