Re: [PATCH] apparmor: domain: clean up duplicated parts of handle_onexec()

[John Johansen <john.johansen@canonical.com>]

On 7/8/24 20:07, Leesoo Ahn wrote:
> Regression test of AppArmor finished without any failures.
> 
> PASSED: aa_exec access attach_disconnected at_secure introspect capabilities
> changeprofile onexec changehat changehat_fork changehat_misc chdir clone
> coredump deleted e2e environ exec exec_qual fchdir fd_inheritance fork i18n
> link link_subset mkdir mmap mount mult_mount named_pipe namespaces net_raw
> open openat pipe pivot_root posix_ipc ptrace pwrite query_label regex rename
> readdir rw socketpair swap sd_flags setattr symlink syscall sysv_ipc tcp
> unix_fd_server unix_socket_pathname unix_socket_abstract unix_socket_unnamed
> unix_socket_autobind unlink userns xattrs xattrs_profile longpath nfs
> exec_stack aa_policy_cache nnp stackonexec stackprofile
> FAILED:
> make: Leaving directory '/apparmor/tests/regression/apparmor'
> 
> Signed-off-by: Leesoo Ahn <lsahn@ooseel.net>

Acked-by: John Johansen <john.johansen@canonical.com>

this was pulled into my tree, sorry for missing the reply earlier

> ---
>   security/apparmor/domain.c | 37 +++++++++++--------------------------
>   1 file changed, 11 insertions(+), 26 deletions(-)
> 
> diff --git a/security/apparmor/domain.c b/security/apparmor/domain.c
> index 571158ec6188..b73e01b512c2 100644
> --- a/security/apparmor/domain.c
> +++ b/security/apparmor/domain.c
> @@ -822,33 +822,18 @@ static struct aa_label *handle_onexec(const struct cred *subj_cred,
>   	AA_BUG(!bprm);
>   	AA_BUG(!buffer);
>   
> -	if (!stack) {
> -		error = fn_for_each_in_ns(label, profile,
> -				profile_onexec(subj_cred, profile, onexec, stack,
> -					       bprm, buffer, cond, unsafe));
> -		if (error)
> -			return ERR_PTR(error);
> -		new = fn_label_build_in_ns(label, profile, GFP_KERNEL,
> -				aa_get_newest_label(onexec),
> -				profile_transition(subj_cred, profile, bprm,
> -						   buffer,
> -						   cond, unsafe));
> -
> -	} else {
> -		/* TODO: determine how much we want to loosen this */
> -		error = fn_for_each_in_ns(label, profile,
> -				profile_onexec(subj_cred, profile, onexec, stack, bprm,
> -					       buffer, cond, unsafe));
> -		if (error)
> -			return ERR_PTR(error);
> -		new = fn_label_build_in_ns(label, profile, GFP_KERNEL,
> -				aa_label_merge(&profile->label, onexec,
> -					       GFP_KERNEL),
> -				profile_transition(subj_cred, profile, bprm,
> -						   buffer,
> -						   cond, unsafe));
> -	}
> +	/* TODO: determine how much we want to loosen this */
> +	error = fn_for_each_in_ns(label, profile,
> +			profile_onexec(subj_cred, profile, onexec, stack,
> +				       bprm, buffer, cond, unsafe));
> +	if (error)
> +		return ERR_PTR(error);
>   
> +	new = fn_label_build_in_ns(label, profile, GFP_KERNEL,
> +			stack ? aa_label_merge(&profile->label, onexec, GFP_KERNEL)
> +			      : aa_get_newest_label(onexec),
> +			profile_transition(subj_cred, profile, bprm,
> +					   buffer, cond, unsafe));
>   	if (new)
>   		return new;
>