On Tue, Jul 5, 2016 at 11:50 AM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> Provide a security hook to label new file correctly when a file is copied
> up from lower layer to upper layer of a overlay/union mount.
>
> This hook can prepare and switch to a new set of creds which are suitable
> for new file creation during copy up. Caller should revert to old creds
> after file creation.
>
> In SELinux, newly copied up file gets same label as lower file for
> non-context mounts. But it gets label specified in mount option context=
> for context mounts.
>
> Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
> ---
> fs/overlayfs/copy_up.c | 8 ++++++++
> include/linux/lsm_hooks.h | 13 +++++++++++++
> include/linux/security.h | 6 ++++++
> security/security.c | 8 ++++++++
> security/selinux/hooks.c | 27 +++++++++++++++++++++++++++
> 5 files changed, 62 insertions(+)
..
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index a86d537..1b1a1e5 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -3270,6 +3270,32 @@ static void selinux_inode_getsecid(struct inode *inode, u32 *secid)
> *secid = isec->sid;
> }
>
> +static int selinux_inode_copy_up(struct dentry *src, const struct cred **old)
> +{
> + u32 sid;
> + struct cred *new_creds;
> + struct task_security_struct *tsec;
> +
> + new_creds = prepare_creds();
> + if (!new_creds)
> + return -ENOMEM;
> +
> + /* Get label from overlay inode and set it in create_sid */
> + selinux_inode_getsecid(d_inode(src), &sid);
> + tsec = new_creds->security;
> + tsec->create_sid = sid;
> + *old = override_creds(new_creds);
> +
> + /*
> + * At this point of time we have 2 refs on new_creds. One by
> + * prepare_creds and other by override_creds. Drop one reference
> + * so that as soon as caller calls revert_creds(old), this cred
> + * will be freed.
> + */
> + put_cred(new_creds);
> + return 0;
> +}
One quick point of clarification: in addition to the SELinux specific
comments in lsm_hooks.h, I think it would be a good idea if the
SELinux hook implementation, e.g. security/selinux/hooks.c, was in its
own patch and not part of the hook definition.
Beyond that, I'm not overly excited about reusing create_sid for this
purpose. I understand why you did it, but what if the process had
explicitly set create_sid? I think I would prefer the creation of a
new field (create_up_sid?) to track this new label and then add an
additional check to selinux_inode_init_security() to prefer the
existing create_sid to this new field when both are set.
Sound reasonable?
--
paul moore
security @ redhat
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
