Re: [PATCH 6/6] selinux-policy-2.eclass: use an array when constructing commands

Sam James <[email protected]> Sat, 20 Jun 2026 22:13:19 +0100
Newsgroups	gmane.linux.gentoo.devel
Organization	Gentoo
Message-ID	<[email protected]>
Rahul Sandhu <[email protected]> writes:

> Avoids an unnecessary, trivially avoidable bash footgun.
>
> Relying on word splitting is not all that robust nor desirable. Whilst
> it indeed _can_ be fine when you can _guarentee_ that ALL arguments do

guarantee

> not contain whitespace, they add extra fragility for no real reason.
>
> Instead of building up a command string, use the much better suited
> bash arrays for constructing command arguments. This way, if we ever
> have to account for arguments containing spaces, we get that for free
> without having to worry about uprooting the way command arguments are
> constructed, or worse potentially forgetting to do all-together and
> executing a command we did not intend to execute.
>
> Signed-off-by: Rahul Sandhu <[email protected]>
> ---
>  eclass/selinux-policy-2.eclass | 30 ++++++++++++++----------------
>  1 file changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/eclass/selinux-policy-2.eclass b/eclass/selinux-policy-2.eclass
> index 911ef39781e1..41cbfc0248a8 100644
> --- a/eclass/selinux-policy-2.eclass
> +++ b/eclass/selinux-policy-2.eclass
> @@ -281,14 +281,11 @@ selinux-policy-2_src_install() {
>  # activating the policy on the system.
>  selinux-policy-2_pkg_postinst() {
>  	# Set root path and don't load policy into the kernel when cross compiling
> -	local root_opts=""
> +	local root_opts=()
>  	if [[ -n ${ROOT} ]]; then
> -		root_opts="-p ${ROOT} -n"
> +		root_opts=( "-p" "${ROOT}" "-n" )
>  	fi
>  
> -	# build up the command in the case of multiple modules
> -	local COMMAND
> -
>  	for type in targeted strict mcs mls; do
>  		if use "selinux_policy_types_${type}"; then
>  			if [[ "${type}" = "strict" && "${MODS}" = "unconfined" ]]; then
> @@ -299,19 +296,21 @@ selinux-policy-2_pkg_postinst() {
>  			einfo "Inserting the following modules into the ${type} module store: ${MODS}"
>  
>  			cd "${ROOT}/usr/share/selinux/${type}" || die "Could not enter /usr/share/selinux/${type}"
> +
> +			local module_args=()
>  			for mod in ${MODS}; do
>  				if [[ -f "${mod}.pp" ]]; then
> -					COMMAND="${mod}.pp ${COMMAND}"
> +					module_args+=( "${mod}.pp" )
>  				elif [[ -f "${mod}.cil" ]]; then
> -					COMMAND="${mod}.cil ${COMMAND}"
> +					module_args+=( "${mod}.cil" )
>  				fi
>  			done
>  
> -			semodule ${root_opts} -s ${type} -i ${COMMAND}
> +			semodule "${root_opts[@]}" -s "${type}" -i "${module_args[@]}"
>  			if [[ $? -ne 0 ]]; then
>  				ewarn "SELinux module load failed. Trying full reload..."
>  
> -				semodule ${root_opts} -s ${type} -i ./*.pp
> +				semodule "${root_opts[@]}" -s "${type}" -i ./*.pp
>  
>  				if [[ $? -ne 0 ]]; then
>  					ewarn "Failed to reload SELinux policies."
> @@ -333,7 +332,6 @@ selinux-policy-2_pkg_postinst() {
>  			else
>  				einfo "SELinux modules loaded successfully."
>  			fi
> -			COMMAND=""
>  		fi
>  	done
>  
> @@ -386,22 +384,22 @@ selinux-policy-2_pkg_postrm() {
>  	fi
>  
>  	# Set root path and don't load policy into the kernel when cross compiling
> -	local root_opts=""
> -	if [[ -n ${ROOT} ]]; then
> -		root_opts="-p ${ROOT} -n"
> +	local root_opts=()
> +	if [[ -n "${ROOT}" ]]; then
> +		root_opts=( "-p" "${ROOT}" "-n" )
>  	fi
>  
>  	# build up the command in the case of multiple modules
> -	local COMMAND
> +	local module_args=()
>  	for mod in ${MODS}; do
> -		COMMAND="-r ${mod} ${COMMAND}"
> +		module_args+=( "-r" "${mod}" )
>  	done
>  
>  	for type in targeted strict mcs mls; do
>  		if use "selinux_policy_types_${type}"; then
>  			einfo "Removing the following modules from the ${type} module store: ${MODS}"
>  
> -			semodule ${root_opts} -s ${type} ${COMMAND}
> +			semodule "${root_opts[@]}" -s "${type}" "${module_args[@]}"
>  			if [[ $? -ne 0 ]]; then

BTW, this is considered an antipattern.

It's better to do:

if cmd ... ; then
...
fi

or

if ! cmd ... ; then
...
fi

rather than inspect $? immediately after for non-zero.

>  				ewarn "SELinux module unload failed."
>  			else
signature.asc (application/pgp-signature, 418 B)