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

From: Al Viro
Date: Tue May 17 2005 - 15:14:39 EST


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.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/