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

From: Stephen Smalley
Date: Tue Oct 06 2015 - 17:31:08 EST


On 10/05/2015 05:56 PM, Andreas Gruenbacher wrote:
> On Mon, Oct 5, 2015 at 5:08 PM, Stephen Smalley <sds@xxxxxxxxxxxxx> 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.






--
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/