From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [patch v2] fix truncate inode time modification breakage Date: Thu, 3 Jun 2010 21:49:57 +1000 Message-ID: <20100603114957.GJ6822@laptop> References: <20100601133923.GT9453@laptop> <20100601134801.GA11061@lst.de> <20100601135655.GU9453@laptop> <20100602195538.GG6152@laptop> <20100603091434.GA6822@laptop> <20100603100724.GE6822@laptop> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: hch@lst.de, viro@ZenIV.linux.org.uk, linux-fsdevel@vger.kernel.org To: Miklos Szeredi Return-path: Received: from cantor.suse.de ([195.135.220.2]:41149 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751184Ab0FCLuM (ORCPT ); Thu, 3 Jun 2010 07:50:12 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Thu, Jun 03, 2010 at 12:58:22PM +0200, Miklos Szeredi wrote: > On Thu, 3 Jun 2010, Nick Piggin wrote: > > You'll have to be careful, truncate will pass other things down like > > mode to get rid of suid. > > Oh, right. > > > If you just wanted to ignore mtime changes on truncate, then masking > > it off would be the way to go I think. > > > > if (valid & ATTR_SIZE) > > valid &= ~ATTR_SIZE; > > > > Would you also want to do the same thing with suid kill bits from > > truncate, then? Mask off ATTR_MODE and just read it back from the > > server too? > > That would be logical, but no, it didn't used to do that, and now it > can't start doing it for fear of creating a security hole. OK. > Also suid removal happens very rarely compared to mtime update, so a > rare additional chmod request (which might be superfluous for some > filesystems) in addition to write and truncate is I think acceptable. Yeah no big deal. One little thing to put on the list if you ever need bump the protocol version. > (In fact I think it would be cleanest if truncate/ftruncate was a > separate operation from setattr on all levels, but that's a different > story.) Well it's possible. It is a combination of inode and address space operation really. Because you really want to update mtime/ctime and suid bits iff the truncate succeeds. We did consider a new API for it, but it didn't seem to be an obvious improvement. A filesystem can easily branch into another function for ATTR_SIZE immediately on setattr entry. > So for now something like > > if (valid & ATTR_SIZE) > valid &= ~(ATTR_MTIME | ATTR_CTIME); > > would work? That should do the trick, yes. But I think CTIME would just confuse things seeing as you ignore it everywhere else.