Re: [patch 2/7] BSD Secure Levels: move bd claim from inode to filp

[Al Viro <viro@parcelfarce.linux.theplanet.co.uk>]

On Tue, May 17, 2005 at 02:46:17PM -0500, Michael Halcrow wrote:
> In my tests, utime() does not cause file_permission() to be called.

> > Use of current in setting/checking ->i_security is a bad joke.

OK, these applied to the original.  Now to the patched version:

	a) ->file_permission() is called on every write(2).  So you get
an open() for each write().  And only one matching close() (aka. blkdev_put())
	b) ->file_permission() is *not* called on mmap() path.  So much for
protection, exclusion, etc.
	c) you get bd_claim() on each write(); subsequent ones work just
fine, since you use the same holder.  On close() you undo one of those.
Leaving the rest there, with holder pointing to freed-and-soon-to-be-reused
struct file.
 
> > d) cargo-cult programming: ->f_dentry and ->f_dentry->d_inode are
> > *not* NULL, TYVM.
> 
> Exactly what code are you refering to here?

seclvl_file_free_security(), but I have to lift that objection in part - this
crap gets called from put_filp(), so ->f_dentry might be NULL.  If it is not
NULL, though, we are guaranteed that ->f_dentry->d_inode will *not* be NULL.
BTW, all your checks in that path can be replaced with a single check for
f->f_security being non-NULL.  Not that it helped, though - see (a) and (c)
above.