Re: [PATCH v2 1/2] security: Add hook to invalidate inode security labels

[Stephen Smalley <sds@tycho.nsa.gov>]

On 10/05/2015 05:56 PM, Andreas Gruenbacher wrote:
> On Mon, Oct 5, 2015 at 5:08 PM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
>> Not fond of these magic initialized values.
> 
> That should be a solvable problem.
> 
>> Is it always safe to call inode_doinit() from all callers of
>> inode_has_perm()?
> 
> As long as inode_has_perm is only used in contexts in which a file
> permission check / acl check would be possible, I don't see why not.
> 
>> What about the cases where isec->sid is used without going through
>> inode_has_perm()?
> 
> inode_has_perm seems to be called frequently and invalid labels seem
> to be reload quickly, so this change may make SELinux work well enough
> to be useful on top of gfs2 or similar. More checks would of course be
> better. The ideal case would be to always reload invalid labels, but
> that currently won't be possible because we don't have dentries
> everywhere.
> 
> I can't tell if this is this good enough to provide a useful level of
> protection. In any case, without a patch like this, on gfs2 and
> similar file systems, SELinux currently doesn't work at all.
> 
> How we can make progress with this problem?

I think we'd need to wrap all uses of inode->i_security with a helper that
applies this test.  FWIW, many/most of them seem to have a dentry
available, including all callers of inode_has_perm itself, so you could
just use inode_doinit_with_dentry() for all of those cases.  Maybe just
inline inode_has_perm() and get rid of it.

Need to deal appropriately with situations like selinux_inode_permission with
MAY_NOT_BLOCK.