linux-kernel - Re: [PATCH v3 01/14] selinux: avoid nontransitive comparison

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAEjxPJ7R-ZR6u6kn3ao_7AT6-QckUzoR+xiop0PumUu9p+xVJQ@mail.gmail.com>
Date: Tue, 13 May 2025 15:40:31 -0400
From: Stephen Smalley <stephen.smalley.work@...il.com>
To: cgzones@...glemail.com
Cc: selinux@...r.kernel.org, Paul Moore <paul@...l-moore.com>, 
	Ondrej Mosnacek <omosnace@...hat.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 01/14] selinux: avoid nontransitive comparison

On Sun, May 11, 2025 at 1:31 PM Christian Göttsche
<cgoettsche@...tendoof.de> wrote:
>
> From: Christian Göttsche <cgzones@...glemail.com>
>
> Avoid using nontransitive comparison to prevent unexpected sorting
> results due to (well-defined) overflows.
> See https://www.qualys.com/2024/01/30/qsort.txt for a related issue in
> glibc's qsort(3).
>
> Signed-off-by: Christian Göttsche <cgzones@...glemail.com>

I don't especially care either way about this patch, but it seems harmless.

Acked-by: Stephen Smalley <stephen.smalley.work@...il.com>

> ---
> v3: rename macro to cmp_int()
> ---
>  security/selinux/ss/policydb.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/security/selinux/ss/policydb.c b/security/selinux/ss/policydb.c
> index 9ea971943713..dc701a7f8652 100644
> --- a/security/selinux/ss/policydb.c
> +++ b/security/selinux/ss/policydb.c
> @@ -37,6 +37,8 @@
>  #include "mls.h"
>  #include "services.h"
>
> +#define cmp_int(a, b) (((a) > (b)) - ((a) < (b)))
> +
>  #ifdef CONFIG_SECURITY_SELINUX_DEBUG
>  /* clang-format off */
>  static const char *const symtab_name[SYM_NUM] = {
> @@ -424,11 +426,11 @@ static int filenametr_cmp(const void *k1, const void *k2)
>         const struct filename_trans_key *ft2 = k2;
>         int v;
>
> -       v = ft1->ttype - ft2->ttype;
> +       v = cmp_int(ft1->ttype, ft2->ttype);
>         if (v)
>                 return v;
>
> -       v = ft1->tclass - ft2->tclass;
> +       v = cmp_int(ft1->tclass, ft2->tclass);
>         if (v)
>                 return v;
>
> @@ -459,15 +461,15 @@ static int rangetr_cmp(const void *k1, const void *k2)
>         const struct range_trans *key1 = k1, *key2 = k2;
>         int v;
>
> -       v = key1->source_type - key2->source_type;
> +       v = cmp_int(key1->source_type, key2->source_type);
>         if (v)
>                 return v;
>
> -       v = key1->target_type - key2->target_type;
> +       v = cmp_int(key1->target_type, key2->target_type);
>         if (v)
>                 return v;
>
> -       v = key1->target_class - key2->target_class;
> +       v = cmp_int(key1->target_class, key2->target_class);
>
>         return v;
>  }
> @@ -496,15 +498,15 @@ static int role_trans_cmp(const void *k1, const void *k2)
>         const struct role_trans_key *key1 = k1, *key2 = k2;
>         int v;
>
> -       v = key1->role - key2->role;
> +       v = cmp_int(key1->role, key2->role);
>         if (v)
>                 return v;
>
> -       v = key1->type - key2->type;
> +       v = cmp_int(key1->type, key2->type);
>         if (v)
>                 return v;
>
> -       return key1->tclass - key2->tclass;
> +       return cmp_int(key1->tclass, key2->tclass);
>  }
>
>  static const struct hashtab_key_params roletr_key_params = {
> --
> 2.49.0
>