Re: [patch v2] fix truncate inode time modification breakage

[Nick Piggin <npiggin@suse.de>]

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.