Re: [PATCH v4] selinux: implement namespace_init and namespace_install hooks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] selinux: missing SECINITSID_KERNEL fallback for initial namespaces in setns
- [Medium] selinux: audit logs report zero ns_id during namespace creation
--

commit 7cfaed9361f973ecdd7fd2e074c01074e97b7cd5
Author: Daniel Durning <danieldurning.work@gmail.com>

selinux: implement namespace_init and namespace_install hooks
    
This commit adds implementations for the namespace_init and
namespace_install hooks to control namespace creation and setns
operations via SELinux. It defines new security classes for different
namespace types and saves the creator's SID in the namespace security
blob for validation.

> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index f2394cf6646e5..cd7ec2c90d1d0 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c

[ ... ]

> @@ -7184,6 +7211,41 @@ static int selinux_inode_getsecctx(struct inode *inode, struct lsm_context *cp)
>  	cp->id = LSM_ID_SELINUX;
>  	return 0;
>  }
> +
> +static int selinux_namespace_init(struct ns_common *ns)
> +{
> +	struct common_audit_data ad;
> +	struct ns_security_struct *nssec = selinux_ns(ns);
> +	u16 sclass = ns_type_to_security_class(ns);
> +	u32 sid = current_sid();
> +
> +	ad.type = LSM_AUDIT_DATA_NS;
> +	ad.u.ns.ns_type = ns->ns_type;
> +	ad.u.ns.ns_id = ns->ns_id;

[Severity: Medium]
Will this audit log record an ns_id of 0 for all namespace creation events?

During namespace creation, ns_common_init() initializes ns->ns_id to 0 and
immediately calls security_namespace_init(). The actual namespace ID isn't
assigned until ns_tree_gen_id() is called later in the lifecycle.

Because selinux_namespace_init() reads ns->ns_id before it is assigned, does
this leave the audit logs ineffective for correlating namespace lifecycle
events?

> +
> +	nssec->sid = sid;
> +	if (ns->inum == MNT_NS_ANON_INO)
> +		return avc_has_perm(sid, sid, sclass,
> +					MNT_NAMESPACE__CREATE_ANON, &ad);
> +
> +	return avc_has_perm(sid, sid, sclass, NAMESPACE__CREATE, &ad);
> +}
> +
> +static int selinux_namespace_install(const struct nsset *nsset,
> +				  struct ns_common *ns)
> +{
> +	struct common_audit_data ad;
> +	struct ns_security_struct *nssec = selinux_ns(ns);
> +	u16 sclass = ns_type_to_security_class(ns);
> +	u32 sid = current_sid();
> +
> +	ad.type = LSM_AUDIT_DATA_NS;
> +	ad.u.ns.ns_type = ns->ns_type;
> +	ad.u.ns.ns_id = ns->ns_id;
> +
> +	return avc_has_perm(sid, nssec->sid, sclass, NAMESPACE__SETNS, &ad);

[Severity: High]
Is the fallback to SECINITSID_KERNEL missing here?

The commit message states: "We fall back to SECINITSID_KERNEL if ns_common is
pointing to an init namespace, since these are created before policy load and
will be lacking a security blob."

However, this line unconditionally dereferences nssec->sid. If setns() targets
an initial namespace (where ns->ns_security might be uninitialized or NULL),
could this result in a NULL pointer dereference or an evaluation against
SECINITSID_UNLABELED instead of SECINITSID_KERNEL?

> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921132825.17526-1-stephen.smalley.work@gmail.com?part=1