linux-kernel - Re: [PATCH] apparmor: domain: clean up duplicated parts of handle

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <41b11740-65aa-4015-86d1-c98f8354846e@canonical.com>
Date: Mon, 9 Sep 2024 23:37:34 -0700
From: John Johansen <john.johansen@...onical.com>
To: Leesoo Ahn <lsahn@...eel.net>
Cc: Leesoo Ahn <lsahn@...akecorp.com>, Paul Moore <paul@...l-moore.com>,
 James Morris <jmorris@...ei.org>, "Serge E. Hallyn" <serge@...lyn.com>,
 apparmor@...ts.ubuntu.com, linux-security-module@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH] apparmor: domain: clean up duplicated parts of
 handle_onexec()

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@...eel.net>

Acked-by: John Johansen <john.johansen@...onical.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;
>