[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <0c6a49b11150cb088f3be6a8e49fdd02@paul-moore.com>
Date: Tue, 02 Jul 2024 11:56:48 -0400
From: Paul Moore <paul@...l-moore.com>
To: Canfeng Guo <guocanfeng@...ontech.com>
Cc: stephen.smalley.work@...il.com, omosnace@...hat.com, selinux@...r.kernel.org, linux-kernel@...r.kernel.org, Canfeng Guo <guocanfeng@...ontech.com>
Subject: Re: [PATCH] selinux: Streamline type determination in security_compute_sid
On Jun 29, 2024 Canfeng Guo <guocanfeng@...ontech.com> wrote:
>
> Simplifies the logic for determining the security context type in
> security_compute_sid, enhancing readability and efficiency.
>
> Consolidates default type assignment logic next to type transition
> checks, removing redundancy and improving code flow.
>
> Signed-off-by: Canfeng Guo <guocanfeng@...ontech.com>
> ---
> security/selinux/ss/services.c | 32 ++++++++++++++++----------------
> 1 file changed, 16 insertions(+), 16 deletions(-)
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index e33e55384b75..0c07ebf0b1e7 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -1804,21 +1804,7 @@ static int security_compute_sid(u32 ssid,
> newcontext.role = OBJECT_R_VAL;
> }
>
> - /* Set the type to default values. */
> - if (cladatum && cladatum->default_type == DEFAULT_SOURCE) {
> - newcontext.type = scontext->type;
> - } else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
> - newcontext.type = tcontext->type;
> - } else {
> - if ((tclass == policydb->process_class) || sock) {
> - /* Use the type of process. */
> - newcontext.type = scontext->type;
> - } else {
> - /* Use the type of the related object. */
> - newcontext.type = tcontext->type;
> - }
> - }
> -
> + /* Set the type. */
> /* Look for a type transition/member/change rule. */
> avkey.source_type = scontext->type;
> avkey.target_type = tcontext->type;
> @@ -1837,9 +1823,23 @@ static int security_compute_sid(u32 ssid,
> }
> }
>
> + /* If a permanent rule is found, use the type from */
> + /* the type transition/member/change rule. Otherwise, */
> + /* set the type to its default values. */
In general this patch looks fine with the exception of the comment
block above, you can either follow the multi-line comment used elsewhere
in this source file, example:
/* line one
line two
line three */
... or you can follow the generally accepted style for multi-line
comments in the Linux kernel:
/* line one
* line two
* line three
*/
See the link below for more information:
* https://docs.kernel.org/process/coding-style.html#commenting
> if (avnode) {
> - /* Use the type from the type transition/member/change rule. */
> newcontext.type = avnode->datum.u.data;
> + } else if (cladatum && cladatum->default_type == DEFAULT_SOURCE) {
> + newcontext.type = scontext->type;
> + } else if (cladatum && cladatum->default_type == DEFAULT_TARGET) {
> + newcontext.type = tcontext->type;
> + } else {
> + if ((tclass == policydb->process_class) || sock) {
> + /* Use the type of process. */
> + newcontext.type = scontext->type;
> + } else {
> + /* Use the type of the related object. */
> + newcontext.type = tcontext->type;
> + }
> }
>
> /* if we have a objname this is a file trans check so check those rules */
> --
> 2.20.1
--
paul-moore.com
Powered by blists - more mailing lists