Re: [Security] time_attrs argument for security_path_truncate

[Nick Piggin <npiggin@suse.de>]

On Tue, Jun 01, 2010 at 10:04:13AM -0700, Linus Torvalds wrote:
> 
> 
> On Wed, 2 Jun 2010, Nick Piggin wrote:
> >
> > It appears like the time_attrs parameter of security_path_truncate is
> > unused by any security module, so I wonder if we can remove it?
> 
> I think we should aggressively remove interfaces that aren't used. The VFS 
> side has been complicated several times because of the security hooks, and 
> if they ended up not getting used and just complicates logic, we should 
> remove it.

OK, good. I'll put a patch at the head of my queue.

> > We cannot really get it right for truncate(2) calls anyway without
> > holding i_mutex over the call (because ATTR_MTIME|ATTR_CTIME is
> > effectively set iff size changes). So the meaning of this parameter
> > today is misleading anyway.
> 
> Why do we pass it in to do_truncate() at all, btw? Especially as we seem 
> to be a bit confused about it. A regular 'truncate()' passes in a 
> time_attrs of '0', while a 'ftruncate()' passes ATTR_MTIME|ATTR_CTIME.

Yes, and then it relies on the filesystems to update MTIME and CTIME
if size has changed. Many get it wrong (and in my confusion I also
just broke a couple more). So I'm working on fixing it for everyone.

http://marc.info/?l=linux-fsdevel&m=127539956932537&w=2

 
> That doesn't sound right. And if it is (because I can well imagine some 
> strange and subtle POSIX rule wrt ftruncate() and mtime/ctime), I think we 
> should add a comment on it, since I've clearly forgotten the reason.

Yeah a comment would have really helped, since it seems stupid it
must be a typo in the standards document.

6e656be899993f450a765056cdc8d87e58906508

 
> Btw, right now we pass in ATTR_OPEN too to the security_path_truncate() 
> call. Which again doesn't seem to be _used_ by any security thing, and 
> which also violates the naming of that argument. So there's some 
> additional strangeness going on there.
> 
> (That ATTR_OPEN case was introduced in commit be6d3e56a6 ("introduce new 
> LSM hooks where vfsmount is available"), for what its worth. The ATTR_OPEN 
> bit seems to have been copied from the do_truncate() call below it, I 
> think it's just a copy-paste error).

Good point. That'll get nuked when we remove the parameter.