From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Layton Subject: Re: [patch] cifs: accidentally creating empty files Date: Fri, 28 Oct 2011 23:44:06 -0400 Message-ID: <20111028234406.01fd23ba@corrin.poochiereds.net> References: <20110927212810.GA12900@elgon.mountain> <20111013132755.6c5db87e@barsoom.rdu.redhat.com> <20111028072019.GA14900@longonot.mountain> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Dan Carpenter , Steve French , linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, samba-technical-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org To: Steve French Return-path: In-Reply-To: Sender: linux-cifs-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-ID: On Fri, 28 Oct 2011 10:27:04 -0500 Steve French wrote: > On Fri, Oct 28, 2011 at 2:20 AM, Dan Carpenter 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. =C2=A0Doubling 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 w= hole > >> point of this is to let us do an "atomic" open or create operation > >> and not have to split it into multiple requests. =C2=A0In 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... =C2=A0It'= s hard/ > > impossible to check the permissions first and then decide whether t= o > > pass the O_CREAT flag. =C2=A0Maybe we could add an if statement to = check > > whether noperm was used and the noperm version could use the create > > on lookup. >=20 > Why is it hard to check local permissions? We have the local mode > for the parent. >=20 > Also note that the "intent" flags and the atomic create is not just > about performance, but also making it atomic reduces some of the weir= d > failure cases. >=20 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. --=20 Jeff Layton