Re: [PATCH] selinux: treat genfscon lookup failure as non-fatal for booleans

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch adds `ret = 0;` to prevent an error code from propagating, but it is a dead store because `ret` is unconditionally overwritten immediately afterward.
--

commit 56d806498916952baebbc4306bdac863b09cae9c
Author: Stephen Smalley <stephen.smalley.work@gmail.com>

selinux: treat genfscon lookup failure as non-fatal for booleans

This commit aims to make genfscon lookup failures non-fatal when labeling
inodes for policy booleans. It resets the error code from
selinux_policy_genfs_sid() to zero to prevent the error from propagating.

Link: https://lore.kernel.org/selinux/RvCmRlQYQ3uewSzEeY0utQ@coker.com.au/

> diff --git a/security/selinux/selinuxfs.c b/security/selinux/selinuxfs.c
> index 5aaaf69410bb0..fb1d11e8fd371 100644
> --- a/security/selinux/selinuxfs.c
> +++ b/security/selinux/selinuxfs.c
> @@ -1278,6 +1278,7 @@ static int sel_make_bools(struct selinux_policy *newpolicy, struct dentry *bool_
>  			pr_warn_ratelimited("SELinux: no sid found, defaulting to security isid for %s\n",
>  					   page);
>  			sid = SECINITSID_SECURITY;
> +			ret = 0;

[Severity: Medium]
Is this assignment a dead store?

Looking at sel_make_bools(), ret is unconditionally overwritten just a few
lines later by the result of sel_attach_file():

		isec->sid = sid;
		isec->initialized = LABEL_INITIALIZED;
		inode->i_fop = &sel_bool_ops;
		inode->i_ino = i|SEL_BOOL_INO_OFFSET;

		ret = sel_attach_file(bool_dir, names[i], inode);

Because ret is overwritten regardless of the prior error state, the added
assignment doesn't appear to change the functional behavior of the code.

The error code from selinux_policy_genfs_sid() was already being discarded
by the sel_attach_file() assignment before the loop condition could evaluate
it.

Does sel_attach_file() need to be skipped if selinux_policy_genfs_sid()
initially failed, or is there another way the error was intended to be
handled here?

>  		}
>  
>  		isec->sid = sid;

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