[patch] cifs: accidentally creating empty files

[Steve French]

On Fri, Oct 28, 2011 at 10:44 PM, Jeff Layton <jlayton at samba.org> wrote:
> On Fri, 28 Oct 2011 10:27:04 -0500
> Steve French <smfrench at gmail.com> wrote:
>
>> On Fri, Oct 28, 2011 at 2:20 AM, Dan Carpenter <dan.carpenter at oracle.com> wrote:
>> > On Wed, Oct 19, 2011 at 09:14:37PM -0500, Steve French wrote:
>> >> Doesn't this force the create to happen later - rather than
>> >> at lookup time where it belongs?
>> >>
>> >> if the issue is just noperm ... we should let this through if the user
>> >> has local permissions.  Doubling the cost of a file create to Samba
>> >> seems like a bad idea (ie doing aQueryPathInfo AND an NTCreateX
>> >> doubles the roundrip, doubles the load on the server etc.) - the whole
>> >> point of this is to let us do an "atomic" open or create operation
>> >> and not have to split it into multiple requests.  In any case why would
>> >> we do the open and then follow it with a create?
>> >>
>> >> Can we fix this to (at least) narrow the performance penalty.
>> >>
>> >
>> > Yes, it does add another back and forth to file create...  It's hard/
>> > impossible to check the permissions first and then decide whether to
>> > pass the O_CREAT flag.  Maybe we could add an if statement to check
>> > whether noperm was used and the noperm version could use the create
>> > on lookup.
>>
>> Why is it hard to check local permissions? We have the local mode
>> for the parent.
>>
>> Also note that the "intent" flags and the atomic create is not just
>> about performance, but also making it atomic reduces some of the weird
>> failure cases.
>>
>
> We have the local mode for the parent, but we do not have the ownership
> and mode for the file that has not yet been created. Because of the
> special (ahem) way that cifs handles permissions, it's easily possible
> for the ownership of the file not to match the user doing that create.
> At that point, later operations on the file can easily fail.
>
> This was the primary reason for the multiuser patch series, and why I
> still say that doing permissions checking on the client is a broken
> model.

We don't have much choice - we have to allow permission
checking on the client when client security context
can't match security context on server - but I would like
to make multiuser the default (especially if we
can figure out a way to integrate winbind-like
upcall for ntlmv2 auth) at least for the future (smb2 etc.)
even if we can't change the default for cifs mounts.



-- 
Thanks,

Steve