apparmor: domain: clean up duplicated parts of handle_onexec()

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

Posted by Leesoo Ahn 1 year, 5 months ago

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>
---
 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;
 
-- 
2.34.1

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

Posted by John Johansen 1 year, 3 months ago

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;
>