Re: [PATCH 5/7] SELinux: Handle opening of a unioned file

[David Howells]

Stephen Smalley <sds@xxxxxxxxxxxxx> wrote:

> It looks like commit 415103f9932d45f7927f4b17e3a9a13834cdb9a1 changed
> selinux_inode_init_security()'s handling of SECURITY_FS_USE_MNTPOINT,
> and this change was never propagated to selinux_dentry_init_security().
>  However, that commit also did not update
> security/selinux/hooks.c:may_create()'s logic for computing the new file
> label when checking CREATE permission, and therefore introduced a
> potential inconsistency between the label used for the permission check
> and the label assigned to the inode.
> 
> That's why I suggested that we need a common helper for all three to
> ensure consistency there.

I think a common helper is harder than it seems.  We need the parent dir in
one of the cases the helper has to consider, but finding it is done in three
different ways, depending on the caller:

 (1) dentry_init can just use ->d_parent as there's a lock held that prevents
     it changing (I think).  This could use (2) instead, however.

 (2) file_open has to use dget_parent().

 (3) inode_init doesn't have any dentries, but rather has the object and
     parent inodes.

If we don't mind file_open() always calling dget_parent(), then the common
helper can take the dir inode.

Also, thinking ahead to the possibility of bringing unionmount into the kernel
at some point: union non-dir dentries that are not yet copied up have no inode
attached, but rather fall through to the underlying lower inode in the VFS.
This, however, gives us nowhere to hang the inode label.  How expensive is the
security_transition_sid() call?

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