Attachment 18325 Details for Bug 15625 – backport to 4.17 (v2)

[patch] backport to 4.17 (v2)

v4-17-bug-15625-sorting-v2.patch (text/plain), 101.36 KB, created by Douglas Bagnall on 2024-06-05 04:20:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Douglas Bagnall

Created: 2024-06-05 04:20:58 UTC

Size: 101.36 KB

patch

obsolete

>From 91349f1f51ed1e88706dd9b15e39e62737509227 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 11:06:00 +1300
>Subject: [PATCH 01/65] ldb: avoid out of bounds read and write in ldb_qsort()
>
>If a compare function is non-transitive (for example, if it evaluates
>A > B and B > C, but A < C), this implementation of qsort could access
>out-of-bounds memory. This was found in glibc's qsort by Qualys, and
>their write-up for OSS-Security explains it very well:
>
> https://www.openwall.com/lists/oss-security/2024/01/30/7
>
>An example of a non-transitive compare is one in which does this
>
> int cmp(const void *_a, const void *_b)
> {
>        int a = *(int *)_a;
>        int b = *(int *)_b;
>        return a - b;
> }
>
>which does the right thing when the magnitude of the numbers is small,
>but which will go wrong if a is INT_MIN and b is INT_MAX. Likewise, if
>a and b are e.g. uint32_t, the value can wrap when cast to int.
>
>We have functions that are non-transitive regardless of subtraction.
>For example, here (which is not used with ldb_qsort):
>
> int codepoint_cmpi(codepoint_t c1, codepoint_t c2)
>        if (c1 == c2 ||
>            toupper_m(c1) == toupper_m(c2)) {
>                return 0;
>        }
>        return c1 - c2;
> }
>
>The toupper_m() is only called on equality case. Consider {'a', 'A', 'B'}.
>     'a' == 'A'
>     'a' >  'B'  (lowercase letters come after upper)
>     'A' <  'B'
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15569
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 73e4f6026ad04b73074b413bd8c838ca48ffde7f)
>---
> lib/ldb/common/qsort.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/qsort.c b/lib/ldb/common/qsort.c
>index 012aaf3c403..bae35e6b1b1 100644
>--- a/lib/ldb/common/qsort.c
>+++ b/lib/ldb/common/qsort.c
>@@ -227,7 +227,7 @@ void ldb_qsort (void *const pbase, size_t total_elems, size_t size,
>     while ((run_ptr += size) <= end_ptr)
>       {
> 	tmp_ptr = run_ptr - size;
>-	while ((*cmp) ((void *) run_ptr, (void *) tmp_ptr, opaque) < 0)
>+	while (tmp_ptr > base_ptr && (*cmp) ((void *) run_ptr, (void *) tmp_ptr, opaque) < 0)
> 	  tmp_ptr -= size;
> 
> 	tmp_ptr += size;
>-- 
>2.34.1
>
>
>From b947b5675831dc1cdb2797ed7f4b7b08dc6ec297 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 28 Mar 2024 12:57:54 +1300
>Subject: [PATCH 02/65] lib/fuzzing/decode_ndr_X_crash: guess the pipe from
> filename
>
>Usually we are dealing with a filename that tells you what the pipe is,
>and there is no reason for this debug helper not to be convenient
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 8b6a584170eeb5082a188879be88e5f414b0be81)
>---
> lib/fuzzing/decode_ndr_X_crash | 12 ++++++++++--
> 1 file changed, 10 insertions(+), 2 deletions(-)
>
>diff --git a/lib/fuzzing/decode_ndr_X_crash b/lib/fuzzing/decode_ndr_X_crash
>index 63c3cd747d7..d90e7efe122 100755
>--- a/lib/fuzzing/decode_ndr_X_crash
>+++ b/lib/fuzzing/decode_ndr_X_crash
>@@ -61,8 +61,9 @@ def process_one_file(f):
> 
> def main():
>     parser = argparse.ArgumentParser()
>-    parser.add_argument('-p', '--pipe', default='$PIPE',
>-                        help='pipe name (for output command line)')
>+    parser.add_argument('-p', '--pipe', default=None,
>+                        help=('pipe name (for output command line, '
>+                              'default is a guess or "$PIPE")'))
>     parser.add_argument('-t', '--type', default=None, choices=TYPES,
>                         help='restrict to this type')
>     parser.add_argument('-o', '--opnum', default=None, type=int,
>@@ -91,6 +92,13 @@ def main():
>         sys.exit(1)
> 
>     for fn in args.FILES:
>+        if pipe is None:
>+            m = re.search(r'clusterfuzz-testcase.+-fuzz_ndr_([a-z]+)', fn)
>+            if m is None:
>+                pipe = '$PIPE'
>+            else:
>+                pipe = m.group(1)
>+
>         if args.crash_filter is not None:
>             if not re.search(args.crash_filter, fn):
>                 print_if_verbose(f"skipping {fn}")
>-- 
>2.34.1
>
>
>From e98d60c2e6b7130d4c2d4eebf7bc0adbf0ac288e Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:43:27 +1300
>Subject: [PATCH 03/65] util:tsort.h: add a macro for safely comparing numbers
>
>In many places we use `return a - b;` in a comparison function. This can
>be problematic if the comparison is used in a sort, as `a - b` is not
>guaranteed to do what we expect. For example:
>
>* if a and b are 2s-complement ints, a is INT_MIN and b is INT_MAX, then
>  a - b = 1, which is wrong.
>
>* if a and b are 64 bit pointers, a - b could wrap around many times in
>  a cmp function returning 32 bit ints. (We do this often).
>
>The issue is not just that a sort could go haywire.
>Due to a bug in glibc, this could result in out-of-bounds access:
>
>https://www.openwall.com/lists/oss-security/2024/01/30/7
>
>(We have replicated this bug in ldb_qsort).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5ab93f48c575db1a3c5a707258cc44f707a5eeb0)
>---
> lib/util/tsort.h | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
>diff --git a/lib/util/tsort.h b/lib/util/tsort.h
>index 811d6cd2f77..18e82d6c9fe 100644
>--- a/lib/util/tsort.h
>+++ b/lib/util/tsort.h
>@@ -37,4 +37,23 @@ do { \
> } while (0)
> #endif
> 
>+
>+#ifndef NUMERIC_CMP
>+/*
>+ * NUMERIC_CMP is a safe replacement for `a - b` in comparison
>+ * functions. It will work on integers, pointers, and floats.
>+ *
>+ * Rather than
>+ *
>+ *      return a - b;
>+ *
>+ * use
>+ *
>+ *     return NUMERIC_CMP(a, b);
>+ *
>+ * and you won't have any troubles if a - b would overflow.
>+ */
>+#define NUMERIC_CMP(a, b) (((a) > (b)) - ((a) < (b)))
>+#endif
>+
> #endif
>-- 
>2.34.1
>
>
>From 47e8fa12883c01b284eac8d985d0391ba4990b3a Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 17:53:39 +1300
>Subject: [PATCH 04/65] ldb: add NUMERIC_CMP macro to ldb.h
>
>In other places we tend to include tsort.h, which also has TYPESAFE_QSORT.
>
>ldb.h already has TYPESAFE_QSORT, so it might as well have NUMERIC_CMP.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit de1b94f79ea8694ecdddab4b455d539caa7e77e2)
>---
> lib/ldb/include/ldb.h | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
>diff --git a/lib/ldb/include/ldb.h b/lib/ldb/include/ldb.h
>index 63d8aedd672..223686a3d83 100644
>--- a/lib/ldb/include/ldb.h
>+++ b/lib/ldb/include/ldb.h
>@@ -2311,6 +2311,22 @@ do { \
> } while (0)
> #endif
> 
>+#ifndef NUMERIC_CMP
>+/*
>+ * NUMERIC_CMP is a safe replacement for `a - b` in comparison
>+ * functions. It will work on integers, pointers, and floats.
>+ *
>+ * Rather than
>+ *
>+ *      return a - b;
>+ *
>+ * use
>+ *
>+ *     return NUMERIC_CMP(a, b);
>+ */
>+#define NUMERIC_CMP(a, b) (((a) > (b)) - ((a) < (b)))
>+#endif
>+
> 
> 
> /**
>-- 
>2.34.1
>
>
>From 5914d8aea5d1e212ce79ea33ac68da89e05aab3f Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:50:47 +1300
>Subject: [PATCH 05/65] ldb:ldb_dn: use safe NUMERIC_CMP in
> ldb_dn_compare_base()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5150b318f4894a8036b2a394c446afd513f8cb60)
>---
> lib/ldb/common/ldb_dn.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
>index fb7a95bf04f..740b099e6be 100644
>--- a/lib/ldb/common/ldb_dn.c
>+++ b/lib/ldb/common/ldb_dn.c
>@@ -1115,7 +1115,7 @@ int ldb_dn_compare_base(struct ldb_dn *base, struct ldb_dn *dn)
> 
> 		/* compare attr.cf_value. */
> 		if (b_vlen != dn_vlen) {
>-			return b_vlen - dn_vlen;
>+			return NUMERIC_CMP(b_vlen, dn_vlen);
> 		}
> 		ret = strncmp(b_vdata, dn_vdata, b_vlen);
> 		if (ret != 0) return ret;
>-- 
>2.34.1
>
>
>From bbb50cb1ad23fec75e5b57a4e01beebb95cb6278 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:51:04 +1300
>Subject: [PATCH 06/65] ldb:ldb_dn: use safe NUMERIC_CMP in ldb_dn_compare()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 75e51bd99b7a029afd98b55283eddad835319ed6)
>---
> lib/ldb/common/ldb_dn.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
>index 740b099e6be..c86a7f6692a 100644
>--- a/lib/ldb/common/ldb_dn.c
>+++ b/lib/ldb/common/ldb_dn.c
>@@ -1194,7 +1194,7 @@ int ldb_dn_compare(struct ldb_dn *dn0, struct ldb_dn *dn1)
> 
> 		/* compare attr.cf_value. */
> 		if (dn0_vlen != dn1_vlen) {
>-			return dn0_vlen - dn1_vlen;
>+			return NUMERIC_CMP(dn0_vlen, dn1_vlen);
> 		}
> 		ret = strncmp(dn0_vdata, dn1_vdata, dn0_vlen);
> 		if (ret != 0) {
>-- 
>2.34.1
>
>
>From 743987800bea1bf7c756f7a187a5188edc8a9a57 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:52:50 +1300
>Subject: [PATCH 07/65] s4:ntvfs: use NUMERIC_CMP in stream_name_cmp
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit a6d76d6ee9f7cfcabe2c20b872b8b1cb598928a6)
>---
> source4/ntvfs/posix/pvfs_streams.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source4/ntvfs/posix/pvfs_streams.c b/source4/ntvfs/posix/pvfs_streams.c
>index cacd8c19952..ae1ec113e8a 100644
>--- a/source4/ntvfs/posix/pvfs_streams.c
>+++ b/source4/ntvfs/posix/pvfs_streams.c
>@@ -22,6 +22,7 @@
> #include "includes.h"
> #include "vfs_posix.h"
> #include "librpc/gen_ndr/xattr.h"
>+#include "lib/util/tsort.h"
> 
> /*
>   normalise a stream name, removing a :$DATA suffix if there is one 
>@@ -51,7 +52,7 @@ static int stream_name_cmp(const char *name1, const char *name2)
> 	l1 = c1?(c1 - name1):strlen(name1);
> 	l2 = c2?(c2 - name2):strlen(name2);
> 	if (l1 != l2) {
>-		return l1 - l2;
>+		return NUMERIC_CMP(l1, l2);
> 	}
> 	ret = strncasecmp_m(name1, name2, l1);
> 	if (ret != 0) {
>-- 
>2.34.1
>
>
>From 4cd2b0198ddf13ca0c7d7b2ce851cb3ca074fd10 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:55:27 +1300
>Subject: [PATCH 08/65] s4:dsdb:mod:operational: use NUMERIC_CMP in pso_compare
>
>prec_{1,2} are uint32_t, and if one is not set we are defaulting to
>0xffffffff (a.k.a UINT32_MAX), so an overflow when cast to int seems
>extremely likely.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 623adcf4aae00ac06e82d98a75ce4644890501e6)
>---
> source4/dsdb/samdb/ldb_modules/operational.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/operational.c b/source4/dsdb/samdb/ldb_modules/operational.c
>index 214079c0917..cd2e6fea9b9 100644
>--- a/source4/dsdb/samdb/ldb_modules/operational.c
>+++ b/source4/dsdb/samdb/ldb_modules/operational.c
>@@ -1062,7 +1062,7 @@ static int pso_compare(struct ldb_message **m1, struct ldb_message **m2)
> 
> 		return ndr_guid_compare(&guid1, &guid2);
> 	} else {
>-		return prec1 - prec2;
>+		return NUMERIC_CMP(prec1, prec2);
> 	}
> }
> 
>-- 
>2.34.1
>
>
>From fa6da9cabaef27b9549e5fddb16b79b1e3f98ac0 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:55:54 +1300
>Subject: [PATCH 09/65] s4: use numeric_cmp in dns_common_sort_zones()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit ee4ebcccd7d9d89dda59615b3653df2632fb1a5d)
>---
> source4/dns_server/dnsserver_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
>index 0481b0715c7..0bbba7ff6de 100644
>--- a/source4/dns_server/dnsserver_common.c
>+++ b/source4/dns_server/dnsserver_common.c
>@@ -1402,7 +1402,7 @@ static int dns_common_sort_zones(struct ldb_message **m1, struct ldb_message **m
> 	/* If the string lengths are not equal just sort by length */
> 	if (l1 != l2) {
> 		/* If m1 is the larger zone name, return it first */
>-		return l2 - l1;
>+		return NUMERIC_CMP(l2, l1);
> 	}
> 
> 	/*TODO: We need to compare DNs here, we want the DomainDNSZones first */
>-- 
>2.34.1
>
>
>From 66d6123319ad97d7c5ba721dfec524ead3b012ee Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 15:47:10 +1300
>Subject: [PATCH 10/65] util:binsearch: user NUMERIC_CMP()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 09c98ff1263eb05933f1956e201655dd41e28a0c)
>---
> lib/util/tests/binsearch.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/lib/util/tests/binsearch.c b/lib/util/tests/binsearch.c
>index b3ecda165f3..24840156c73 100644
>--- a/lib/util/tests/binsearch.c
>+++ b/lib/util/tests/binsearch.c
>@@ -23,17 +23,19 @@
> 
> #include "includes.h"
> #include "lib/util/binsearch.h"
>+#include "lib/util/tsort.h"
> #include "torture/torture.h"
> #include "torture/local/proto.h"
> 
> static int int_cmp(int a, int b)
> {
>-	return a - b;
>+	return NUMERIC_CMP(a, b);
> }
> 
> static int int_cmp_p(int a, int *b)
> {
>-	return a - *b;
>+	int _b = *b;
>+	return NUMERIC_CMP(a, _b);
> }
> 
> static bool test_binsearch_v(struct torture_context *tctx)
>-- 
>2.34.1
>
>
>From 9ac53f4c47722e4d56973e3ae38e1fa4f61e9502 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 5 Apr 2024 13:14:38 +1300
>Subject: [PATCH 11/65] torture:charset: use < and > assertions for
> strcasecmp_m
>
>strcasecmp_m is supposed to return a negative, zero, or positive
>number, depending on whether the first argument is less than, equal to,
>or greater than the second argument (respectively).
>
>We have been asserting that it returns exactly the difference between
>the codepoints in the first character that differs.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(backported from commit ac0a8cd92ca4497bfcfad30e2b4d47547b582b92)
>[dbagnall@samba.org: master patch fixed 32 bit tests not present in 4.17]
>---
> lib/util/charset/tests/charset.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/lib/util/charset/tests/charset.c b/lib/util/charset/tests/charset.c
>index 547dc51e59d..94bf76c010d 100644
>--- a/lib/util/charset/tests/charset.c
>+++ b/lib/util/charset/tests/charset.c
>@@ -72,16 +72,16 @@ static bool test_strcasecmp_m(struct torture_context *tctx)
> 	const char file_iso8859_1[7] = { 0x66, 0x69, 0x6c, 0x65, 0x2d, 0xe9, 0 };
> 	/* file.{accented e} in utf8 */
> 	const char file_utf8[8] =      { 0x66, 0x69, 0x6c, 0x65, 0x2d, 0xc3, 0xa9, 0 };
>-	torture_assert_int_equal(tctx, strcasecmp_m("foo", "bar"), 4, "different strings both lower");
>-	torture_assert_int_equal(tctx, strcasecmp_m("foo", "Bar"), 4, "different strings lower/upper");
>-	torture_assert_int_equal(tctx, strcasecmp_m("Foo", "bar"), 4, "different strings upper/lower");
>-	torture_assert_int_equal(tctx, strcasecmp_m("AFoo", "_bar"), 2, "different strings upper/lower");
>+	torture_assert_int_greater(tctx, strcasecmp_m("foo", "bar"), 0, "different strings both lower");
>+	torture_assert_int_greater(tctx, strcasecmp_m("foo", "Bar"), 0, "different strings lower/upper");
>+	torture_assert_int_greater(tctx, strcasecmp_m("Foo", "bar"), 0, "different strings upper/lower");
>+	torture_assert_int_greater(tctx, strcasecmp_m("AFoo", "_bar"), 0, "different strings upper/lower");
> 	torture_assert_int_equal(tctx, strcasecmp_m("foo", "foo"), 0, "same case strings");
> 	torture_assert_int_equal(tctx, strcasecmp_m("foo", "Foo"), 0, "different case strings");
>-	torture_assert_int_equal(tctx, strcasecmp_m(NULL, "Foo"),  -1, "one NULL");
>-	torture_assert_int_equal(tctx, strcasecmp_m("foo", NULL),  1, "other NULL");
>+	torture_assert_int_less(tctx, strcasecmp_m(NULL, "Foo"),  0, "one NULL");
>+	torture_assert_int_greater(tctx, strcasecmp_m("foo", NULL),  0, "other NULL");
> 	torture_assert_int_equal(tctx, strcasecmp_m(NULL, NULL),   0, "both NULL");
>-	torture_assert_int_equal(tctx, strcasecmp_m(file_iso8859_1, file_utf8), 38,
>+	torture_assert_int_greater(tctx, strcasecmp_m(file_iso8859_1, file_utf8), 0,
> 		"file.{accented e} should differ");
> 	return true;
> }
>-- 
>2.34.1
>
>
>From 39dae146a40a9f65415138bcd20324086e74d66d Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 5 Apr 2024 14:43:42 +1300
>Subject: [PATCH 12/65] torture:charset: use < and > assertions for
> strncasecmp_m
>
>strncasecmp_m is supposed to return a negative, zero, or positive
>number, not necessarily the difference between the codepoints in
>the first  character that differs, which we have been asserting up to
>now.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(backported from commit dda0bb6fc71bae91f3158f69462cb79fdad210fb)
>[dbagnall@samba.org: master patch fixed 32 bit tests not present in 4.17]
>---
> lib/util/charset/tests/charset.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
>diff --git a/lib/util/charset/tests/charset.c b/lib/util/charset/tests/charset.c
>index 94bf76c010d..6fca3f36c19 100644
>--- a/lib/util/charset/tests/charset.c
>+++ b/lib/util/charset/tests/charset.c
>@@ -151,19 +151,19 @@ static bool test_strncasecmp_m(struct torture_context *tctx)
> 	const char file_iso8859_1[7] = { 0x66, 0x69, 0x6c, 0x65, 0x2d, 0xe9, 0 };
> 	/* file.{accented e} in utf8 */
> 	const char file_utf8[8] =      { 0x66, 0x69, 0x6c, 0x65, 0x2d, 0xc3, 0xa9, 0 };
>-	torture_assert_int_equal(tctx, strncasecmp_m("foo", "bar", 3), 4, "different strings both lower");
>-	torture_assert_int_equal(tctx, strncasecmp_m("foo", "Bar", 3), 4, "different strings lower/upper");
>-	torture_assert_int_equal(tctx, strncasecmp_m("Foo", "bar", 3), 4, "different strings upper/lower");
>-	torture_assert_int_equal(tctx, strncasecmp_m("AFoo", "_bar", 4), 2, "different strings upper/lower");
>+	torture_assert_int_greater(tctx, strncasecmp_m("foo", "bar", 3), 0, "different strings both lower");
>+	torture_assert_int_greater(tctx, strncasecmp_m("foo", "Bar", 3), 0, "different strings lower/upper");
>+	torture_assert_int_greater(tctx, strncasecmp_m("Foo", "bar", 3), 0, "different strings upper/lower");
>+	torture_assert_int_greater(tctx, strncasecmp_m("AFoo", "_bar", 4), 0, "different strings upper/lower");
> 	torture_assert_int_equal(tctx, strncasecmp_m("foo", "foo", 3), 0, "same case strings");
> 	torture_assert_int_equal(tctx, strncasecmp_m("foo", "Foo", 3), 0, "different case strings");
> 	torture_assert_int_equal(tctx, strncasecmp_m("fool", "Foo", 3),0, "different case strings");
> 	torture_assert_int_equal(tctx, strncasecmp_m("fool", "Fool", 40), 0, "over size");
> 	torture_assert_int_equal(tctx, strncasecmp_m("BLA", "Fool", 0),0, "empty");
>-	torture_assert_int_equal(tctx, strncasecmp_m(NULL, "Foo", 3),  -1, "one NULL");
>-	torture_assert_int_equal(tctx, strncasecmp_m("foo", NULL, 3),  1, "other NULL");
>+	torture_assert_int_less(tctx, strncasecmp_m(NULL, "Foo", 3),  0, "one NULL");
>+	torture_assert_int_greater(tctx, strncasecmp_m("foo", NULL, 3),  0, "other NULL");
> 	torture_assert_int_equal(tctx, strncasecmp_m(NULL, NULL, 3),   0, "both NULL");
>-	torture_assert_int_equal(tctx, strncasecmp_m(file_iso8859_1, file_utf8, 6), 38,
>+	torture_assert_int_greater(tctx, strncasecmp_m(file_iso8859_1, file_utf8, 6), 0,
> 		"file.{accented e} should differ");
> 	return true;
> }
>-- 
>2.34.1
>
>
>From 461c501566974e205d1a8bbfbd9acf9166a772e3 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 5 Apr 2024 14:46:48 +1300
>Subject: [PATCH 13/65] torture:charset: test more of strcasecmp_m
>
>We now test cases:
>
>1. where the first string compares less
>2. one of the strings ends before the other
>3. the strings differ on a character other than the first.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit a512759d7b216cacc0a780b3304549b7945f919c)
>---
> lib/util/charset/tests/charset.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/lib/util/charset/tests/charset.c b/lib/util/charset/tests/charset.c
>index 6fca3f36c19..bca5449c579 100644
>--- a/lib/util/charset/tests/charset.c
>+++ b/lib/util/charset/tests/charset.c
>@@ -73,11 +73,14 @@ static bool test_strcasecmp_m(struct torture_context *tctx)
> 	/* file.{accented e} in utf8 */
> 	const char file_utf8[8] =      { 0x66, 0x69, 0x6c, 0x65, 0x2d, 0xc3, 0xa9, 0 };
> 	torture_assert_int_greater(tctx, strcasecmp_m("foo", "bar"), 0, "different strings both lower");
>+	torture_assert_int_less(tctx, strcasecmp_m("bar", "foo"), 0, "different strings both lower");
> 	torture_assert_int_greater(tctx, strcasecmp_m("foo", "Bar"), 0, "different strings lower/upper");
> 	torture_assert_int_greater(tctx, strcasecmp_m("Foo", "bar"), 0, "different strings upper/lower");
> 	torture_assert_int_greater(tctx, strcasecmp_m("AFoo", "_bar"), 0, "different strings upper/lower");
> 	torture_assert_int_equal(tctx, strcasecmp_m("foo", "foo"), 0, "same case strings");
> 	torture_assert_int_equal(tctx, strcasecmp_m("foo", "Foo"), 0, "different case strings");
>+	torture_assert_int_greater(tctx, strcasecmp_m("food", "Foo"), 0, "strings differ towards the end");
>+	torture_assert_int_less(tctx, strcasecmp_m("food", "Fool"), 0, "strings differ towards the end");
> 	torture_assert_int_less(tctx, strcasecmp_m(NULL, "Foo"),  0, "one NULL");
> 	torture_assert_int_greater(tctx, strcasecmp_m("foo", NULL),  0, "other NULL");
> 	torture_assert_int_equal(tctx, strcasecmp_m(NULL, NULL),   0, "both NULL");
>-- 
>2.34.1
>
>
>From b5289e51e431192bb35b8edb1938db26ccef2474 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 15:49:55 +1300
>Subject: [PATCH 14/65] util:charset:util_str: use NUMERIC_CMP in
> strcasecmp_m_handle
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit f788a399996a73b2aa206ec2b15f5943b06660e0)
>---
> lib/util/charset/util_str.c                                | 5 +++--
> selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard | 1 +
> 2 files changed, 4 insertions(+), 2 deletions(-)
> create mode 100644 selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard
>
>diff --git a/lib/util/charset/util_str.c b/lib/util/charset/util_str.c
>index 2a4ccd93673..ed0a7f3bee1 100644
>--- a/lib/util/charset/util_str.c
>+++ b/lib/util/charset/util_str.c
>@@ -26,6 +26,7 @@
> #include "system/locale.h"
> #include "charset.h"
> #include "lib/util/fault.h"
>+#include "lib/util/tsort.h"
> 
> #ifdef strcasecmp
> #undef strcasecmp
>@@ -79,10 +80,10 @@ _PUBLIC_ int strcasecmp_m_handle(struct smb_iconv_handle *iconv_handle,
> 			continue;
> 		}
> 
>-		return l1 - l2;
>+		return NUMERIC_CMP(l1, l2);
> 	}
> 
>-	return *s1 - *s2;
>+	return NUMERIC_CMP(*s1, *s2);
> }
> 
> /**
>diff --git a/selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard b/selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard
>new file mode 100644
>index 00000000000..fe0d14e83e2
>--- /dev/null
>+++ b/selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard
>@@ -0,0 +1 @@
>+^samba.unittests.ms_fnmatch.test_ms_fn_match_protocol_no_wildcard
>-- 
>2.34.1
>
>
>From 6a7488a93cca17cf1021b38b3240bb7e54df921c Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 17:23:15 +1300
>Subject: [PATCH 15/65] util:test: test_ms_fn_match_protocol_no_wildcard: allow
> -1
>
>We have changed strcasecmp_m() to return -1 in a place where it used
>to return -3. This upset a test, but it shouldn't have: the exact
>value of the negative int is not guaranteed by the function.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit d4ce8231f986a359dc657cd1a6b416270a53c7d3)
>---
> lib/util/tests/test_ms_fnmatch.c                           | 2 +-
> selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard | 1 -
> 2 files changed, 1 insertion(+), 2 deletions(-)
> delete mode 100644 selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard
>
>diff --git a/lib/util/tests/test_ms_fnmatch.c b/lib/util/tests/test_ms_fnmatch.c
>index d11c7bed4be..2261f9bb111 100644
>--- a/lib/util/tests/test_ms_fnmatch.c
>+++ b/lib/util/tests/test_ms_fnmatch.c
>@@ -36,7 +36,7 @@ static void test_ms_fn_match_protocol_no_wildcard(void **state)
> 	/* no wildcards in pattern, a simple strcasecmp_m */
> 	cmp = ms_fnmatch_protocol("pattern", "string", PROTOCOL_COREPLUS,
> 				  true);	/* case sensitive */
>-	assert_int_equal(cmp, -3);
>+	assert_true(cmp < 0);
> }
> 
> static void test_ms_fn_match_protocol_pattern_upgraded(void **state)
>diff --git a/selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard b/selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard
>deleted file mode 100644
>index fe0d14e83e2..00000000000
>--- a/selftest/knownfail.d/test_ms_fn_match_protocol_no_wildcard
>+++ /dev/null
>@@ -1 +0,0 @@
>-^samba.unittests.ms_fnmatch.test_ms_fn_match_protocol_no_wildcard
>-- 
>2.34.1
>
>
>From f9658a62f47b063e5f469e4ee1253dcf74c41620 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 15:53:29 +1300
>Subject: [PATCH 16/65] util:charset:codepoints: condepoint_cmpi uses
> NUMERIC_CMP()
>
>If these are truly unicode codepoints (< ~2m) there is no overflow,
>but the type is defined as uint32_t.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 675fdeee3d6570fdf5a055890dc3386a8db5fd88)
>---
> lib/util/charset/codepoints.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c
>index c35241e2983..faffd28cd25 100644
>--- a/lib/util/charset/codepoints.c
>+++ b/lib/util/charset/codepoints.c
>@@ -26,6 +26,7 @@
> #include "dynconfig/dynconfig.h"
> #include "lib/util/debug.h"
> #include "lib/util/byteorder.h"
>+#include "lib/util/tsort.h"
> 
> #ifdef strcasecmp
> #undef strcasecmp
>@@ -16483,7 +16484,7 @@ _PUBLIC_ int codepoint_cmpi(codepoint_t c1, codepoint_t c2)
> 	    toupper_m(c1) == toupper_m(c2)) {
> 		return 0;
> 	}
>-	return c1 - c2;
>+	return NUMERIC_CMP(c1, c2);
> }
> 
> 
>-- 
>2.34.1
>
>
>From f3ed3e0518c4de7912b2248e0d37a2864b4e8695 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:56:16 +1300
>Subject: [PATCH 17/65] util:charset:codepoints: codepoint_cmpi warning about
> non-transitivity
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit f07ae6990702f8806c0c815454b80a5596b7219a)
>---
> lib/util/charset/codepoints.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
>diff --git a/lib/util/charset/codepoints.c b/lib/util/charset/codepoints.c
>index faffd28cd25..b3abfa99938 100644
>--- a/lib/util/charset/codepoints.c
>+++ b/lib/util/charset/codepoints.c
>@@ -16480,6 +16480,18 @@ _PUBLIC_ bool isupper_m(codepoint_t val)
> */
> _PUBLIC_ int codepoint_cmpi(codepoint_t c1, codepoint_t c2)
> {
>+	/*
>+	 * FIXME: this is unsuitable for use in a sort, as the
>+	 * comparison is intransitive.
>+	 *
>+	 * The problem is toupper_m() is only called on equality case,
>+	 * which has strange effects.
>+	 *
>+	 *    Consider {'a', 'A', 'B'}.
>+	 *     'a' == 'A'
>+	 *     'a' >  'B'  (lowercase letters come after upper)
>+	 *     'A' <  'B'
>+	 */
> 	if (c1 == c2 ||
> 	    toupper_m(c1) == toupper_m(c2)) {
> 		return 0;
>-- 
>2.34.1
>
>
>From 4fce590d3fdd90b7aea97514a85c8dbe76a51b98 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 16:10:38 +1300
>Subject: [PATCH 18/65] s3:libsmb:namequery: note intransitivity in
> addr_compare()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 4a9d274d43b1adac113419c649bbf530d180229d)
>---
> source3/libsmb/namequery.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c
>index 047fff22ad9..a6bd53bc8eb 100644
>--- a/source3/libsmb/namequery.c
>+++ b/source3/libsmb/namequery.c
>@@ -1082,8 +1082,15 @@ bool name_status_find(const char *q_name,
> }
> 
> /*
>-  comparison function used by sort_addr_list
>-*/
>+ * comparison function used by sort_addr_list
>+ *
>+ * This comparison is intransitive in sort if a socket has an invalid
>+ * family (i.e., not IPv4 or IPv6), or an interface doesn't support
>+ * the family. Say we have sockaddrs with IP versions {4,5,6}, of
>+ * which 5 is invalid. By this function, 4 == 5 and 6 == 5, but 4 !=
>+ * 6. This is of course a consequence of cmp() being unable to
>+ * communicate error.
>+ */
> 
> static int addr_compare(const struct sockaddr_storage *ss1,
> 			const struct sockaddr_storage *ss2)
>-- 
>2.34.1
>
>
>From efaaf936702532ed4e38839702a4cf57dd544e10 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 16:13:07 +1300
>Subject: [PATCH 19/65] s3:libsmb:namequery: use NUMERIC_CMP in addr_compare
>
>This one was OK, as the numbers are tightly bound, but there is no
>real reason not to do it safely.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 3414a894ad6640fa8e282d650b1cc5319991545f)
>---
> source3/libsmb/namequery.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c
>index a6bd53bc8eb..10acabad279 100644
>--- a/source3/libsmb/namequery.c
>+++ b/source3/libsmb/namequery.c
>@@ -34,6 +34,7 @@
> #include "lib/gencache.h"
> #include "librpc/gen_ndr/dns.h"
> #include "lib/util/util_net.h"
>+#include "lib/util/tsort.h"
> #include "lib/util/string_wrappers.h"
> 
> /* nmbd.c sets this to True. */
>@@ -1178,7 +1179,7 @@ static int addr_compare(const struct sockaddr_storage *ss1,
> 			max_bits2 += 128;
> 		}
> 	}
>-	return max_bits2 - max_bits1;
>+	return NUMERIC_CMP(max_bits2, max_bits1);
> }
> 
> /*
>-- 
>2.34.1
>
>
>From 63a2e968c5497b0818c4d0c90747575b448db5f8 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 5 Apr 2024 14:22:11 +1300
>Subject: [PATCH 20/65] lib/torture: add assert_int_{less,greater} macros
>
>In some situations, like comparison functions for qsort, we don't care
>about the actual value, just whethger it was greater or less than
>zero.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 6159b098cf35a8043682bfd4c4ea17ef0da6e8ee)
>---
> lib/torture/torture.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
>diff --git a/lib/torture/torture.h b/lib/torture/torture.h
>index ea34c15e0e9..154b225db58 100644
>--- a/lib/torture/torture.h
>+++ b/lib/torture/torture.h
>@@ -521,6 +521,26 @@ static inline void torture_dump_data_str_cb(const char *buf, void *private_data)
> 	} \
> 	} while(0)
> 
>+#define torture_assert_int_less(torture_ctx,got,limit,cmt)\
>+	do { int __got = (got), __limit = (limit); \
>+	if (__got >= __limit) { \
>+		torture_result(torture_ctx, TORTURE_FAIL, \
>+			__location__": "#got" was %d (0x%X), expected < %d (0x%X): %s", \
>+			__got, __got, __limit, __limit, cmt); \
>+		return false; \
>+	} \
>+	} while(0)
>+
>+#define torture_assert_int_greater(torture_ctx,got,limit,cmt)\
>+	do { int __got = (got), __limit = (limit); \
>+	if (__got <= __limit) { \
>+		torture_result(torture_ctx, TORTURE_FAIL, \
>+			__location__": "#got" was %d (0x%X), expected > %d (0x%X): %s", \
>+			__got, __got, __limit, __limit, cmt); \
>+		return false; \
>+	} \
>+	} while(0)
>+
> #define torture_assert_int_equal_goto(torture_ctx,got,expected,ret,label,cmt)\
> 	do { int __got = (got), __expected = (expected); \
> 	if (__got != __expected) { \
>-- 
>2.34.1
>
>
>From 08e0f459b28d9a7f0d2ab3998bba02bdbb0a6585 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 16:16:44 +1300
>Subject: [PATCH 21/65] util: charset:util_str: use NUMERIC_CMP in
> strncasecmp_m_handle
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 997b72d79e651ddbc20e67006ae176229528dc6f)
>---
> lib/util/charset/util_str.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/lib/util/charset/util_str.c b/lib/util/charset/util_str.c
>index ed0a7f3bee1..163ba459010 100644
>--- a/lib/util/charset/util_str.c
>+++ b/lib/util/charset/util_str.c
>@@ -156,14 +156,14 @@ _PUBLIC_ int strncasecmp_m_handle(struct smb_iconv_handle *iconv_handle,
> 			continue;
> 		}
> 
>-		return l1 - l2;
>+		return NUMERIC_CMP(l1, l2);
> 	}
> 
> 	if (n == 0) {
> 		return 0;
> 	}
> 
>-	return *s1 - *s2;
>+	return NUMERIC_CMP(*s1, *s2);
> }
> 
> /**
>-- 
>2.34.1
>
>
>From 63713ea0304a3e1b0d5bb965e54f2d406ef26853 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 17:32:48 +1300
>Subject: [PATCH 22/65] ldb:attrib_handlers: ldb_comparison_Boolean uses
> NUMERIC_CMP()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit f78b964cd81db11097e78099c0699f571f20e126)
>---
> lib/ldb/common/attrib_handlers.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
>index febf2f414ca..4c60432b035 100644
>--- a/lib/ldb/common/attrib_handlers.c
>+++ b/lib/ldb/common/attrib_handlers.c
>@@ -287,7 +287,7 @@ static int ldb_comparison_Boolean(struct ldb_context *ldb, void *mem_ctx,
> 			   const struct ldb_val *v1, const struct ldb_val *v2)
> {
> 	if (v1->length != v2->length) {
>-		return v1->length - v2->length;
>+		return NUMERIC_CMP(v1->length, v2->length);
> 	}
> 	return strncasecmp((char *)v1->data, (char *)v2->data, v1->length);
> }
>-- 
>2.34.1
>
>
>From 498920a7be1d8aa60fb0fecb848c0a2ac2bf7ee2 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 17:43:03 +1300
>Subject: [PATCH 23/65] ldb:attrib_handlers: ldb_comparison_binary uses
> NUMERIC_CMP()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 838c68470299045c5b1c9bdbd527edbeedebf2d6)
>---
> lib/ldb/common/attrib_handlers.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
>index 4c60432b035..7e174366c53 100644
>--- a/lib/ldb/common/attrib_handlers.c
>+++ b/lib/ldb/common/attrib_handlers.c
>@@ -300,7 +300,7 @@ int ldb_comparison_binary(struct ldb_context *ldb, void *mem_ctx,
> 			  const struct ldb_val *v1, const struct ldb_val *v2)
> {
> 	if (v1->length != v2->length) {
>-		return v1->length - v2->length;
>+		return NUMERIC_CMP(v1->length, v2->length);
> 	}
> 	return memcmp(v1->data, v2->data, v1->length);
> }
>-- 
>2.34.1
>
>
>From 06159506118cc9ddc97fd8230c275e69ed7b1139 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 11:07:06 +1300
>Subject: [PATCH 24/65] util:datablob: avoid non-transitive comparison in
> data_blob_cmp()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(backported from commit e1519c3667841ce27b15983eae378799ef9936f7)
>[dbagnall@samba.org: changed in master for conditional ACEs]
>---
> lib/util/data_blob.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/lib/util/data_blob.c b/lib/util/data_blob.c
>index 677f7c19211..687b6c8d3d5 100644
>--- a/lib/util/data_blob.c
>+++ b/lib/util/data_blob.c
>@@ -22,6 +22,7 @@
> #include "attr.h"
> #include "data_blob.h"
> #include "lib/util/samba_util.h"
>+#include "lib/util/tsort.h"
> 
> const DATA_BLOB data_blob_null = { NULL, 0 };
> 
>@@ -121,11 +122,11 @@ _PUBLIC_ int data_blob_cmp(const DATA_BLOB *d1, const DATA_BLOB *d2)
> 		return 1;
> 	}
> 	if (d1->data == d2->data) {
>-		return d1->length - d2->length;
>+		return NUMERIC_CMP(d1->length, d2->length);
> 	}
> 	ret = memcmp(d1->data, d2->data, MIN(d1->length, d2->length));
> 	if (ret == 0) {
>-		return d1->length - d2->length;
>+		return NUMERIC_CMP(d1->length, d2->length);
> 	}
> 	return ret;
> }
>-- 
>2.34.1
>
>
>From d6b37fef44667943934838cce4fb1aedbb09c8bd Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 11:22:58 +1300
>Subject: [PATCH 25/65] ldb: avoid non-transitive comparison in ldb_val_cmp()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5c36bc82415b246fccec9eae693da82b7aa45b81)
>---
> lib/ldb/common/ldb_msg.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
>index 2ea2cce2e83..f715b7cdcf3 100644
>--- a/lib/ldb/common/ldb_msg.c
>+++ b/lib/ldb/common/ldb_msg.c
>@@ -93,7 +93,7 @@ struct ldb_val *ldb_msg_find_val(const struct ldb_message_element *el,
> static int ldb_val_cmp(const struct ldb_val *v1, const struct ldb_val *v2)
> {
> 	if (v1->length != v2->length) {
>-		return v1->length - v2->length;
>+		return NUMERIC_CMP(v1->length, v2->length);
> 	}
> 	return memcmp(v1->data, v2->data, v1->length);
> }
>-- 
>2.34.1
>
>
>From 70e5e4549ed8f23a0fe51fd2bbc78676b7435d00 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 11:26:25 +1300
>Subject: [PATCH 26/65] ldb: reduce non-transitive comparisons in
> ldb_msg_element_compare()
>
>We can still have inconsistent comparisons, because two elements with
>the same number of values will always return -1 if they are unequal,
>which means they will sort differently depending on the order in which
>they are compared.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 21a071e4864dd739840c2ad4adb0c71ec33f8427)
>---
> lib/ldb/common/ldb_msg.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
>diff --git a/lib/ldb/common/ldb_msg.c b/lib/ldb/common/ldb_msg.c
>index f715b7cdcf3..196a644f245 100644
>--- a/lib/ldb/common/ldb_msg.c
>+++ b/lib/ldb/common/ldb_msg.c
>@@ -753,9 +753,16 @@ int ldb_msg_element_compare(struct ldb_message_element *el1,
> 	unsigned int i;
> 
> 	if (el1->num_values != el2->num_values) {
>-		return el1->num_values - el2->num_values;
>+		return NUMERIC_CMP(el1->num_values, el2->num_values);
> 	}
>-
>+	/*
>+	 * Note this is an inconsistent comparison, unsuitable for
>+	 * sorting. If A has values {a, b} and B has values {b, c},
>+	 * then
>+	 *
>+	 * ldb_msg_element_compare(A, B) returns -1, meaning A < B
>+	 * ldb_msg_element_compare(B, A) returns -1, meaning B < A
>+	 */
> 	for (i=0;i<el1->num_values;i++) {
> 		if (!ldb_msg_find_val(el2, &el1->values[i])) {
> 			return -1;
>-- 
>2.34.1
>
>
>From aa410306d113fd645530fe540f6fe8dd4cc17583 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 13:43:47 +1300
>Subject: [PATCH 27/65] libcli/security: use NUMERIC_CMP in dom_sid_compare()
>
>sid->num_auths is always small (int8 < 16), so this is cosmetic only.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(backported from commit cb94202c1cf990e871ee2e8e43c577a0e4b9ee6f)
>[dbagnall@samba.org: file changed in master]
>---
> libcli/security/dom_sid.c | 18 ++++++++++++------
> 1 file changed, 12 insertions(+), 6 deletions(-)
>
>diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
>index 32bc3e187a7..b2ed1531df0 100644
>--- a/libcli/security/dom_sid.c
>+++ b/libcli/security/dom_sid.c
>@@ -28,6 +28,7 @@
> #include "librpc/gen_ndr/security.h"
> #include "dom_sid.h"
> #include "lib/util/smb_strtox.h"
>+#include "lib/util/tsort.h"
> 
> /*****************************************************************
>  Compare the auth portion of two sids.
>@@ -71,12 +72,17 @@ int dom_sid_compare(const struct dom_sid *sid1, const struct dom_sid *sid2)
> 		return 1;
> 
> 	/* Compare most likely different rids, first: i.e start at end */
>-	if (sid1->num_auths != sid2->num_auths)
>-		return sid1->num_auths - sid2->num_auths;
>-
>-	for (i = sid1->num_auths-1; i >= 0; --i)
>-		if (sid1->sub_auths[i] != sid2->sub_auths[i])
>-			return sid1->sub_auths[i] - sid2->sub_auths[i];
>+	if (sid1->num_auths != sid2->num_auths) {
>+		return NUMERIC_CMP(sid1->num_auths, sid2->num_auths);
>+	}
>+	for (i = sid1->num_auths-1; i >= 0; --i) {
>+		if (sid1->sub_auths[i] < sid2->sub_auths[i]) {
>+			return -1;
>+		}
>+		if (sid1->sub_auths[i] > sid2->sub_auths[i]) {
>+			return 1;
>+		}
>+	}
> 
> 	return dom_sid_compare_auth(sid1, sid2);
> }
>-- 
>2.34.1
>
>
>From eeaf4eb644fb388b866c5d38ba8cb0b68f1221af Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 13:53:58 +1300
>Subject: [PATCH 28/65] libcli/security: use NUMERIC_CMP in
> dom_sid_compare_auth()
>
>These numbers are all 8 bit, so overflow is unlikely.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 4641a97151783c2ae825582e91b4676d66dcb713)
>---
> libcli/security/dom_sid.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
>diff --git a/libcli/security/dom_sid.c b/libcli/security/dom_sid.c
>index b2ed1531df0..711655ba326 100644
>--- a/libcli/security/dom_sid.c
>+++ b/libcli/security/dom_sid.c
>@@ -47,11 +47,12 @@ int dom_sid_compare_auth(const struct dom_sid *sid1,
> 		return 1;
> 
> 	if (sid1->sid_rev_num != sid2->sid_rev_num)
>-		return sid1->sid_rev_num - sid2->sid_rev_num;
>+		return NUMERIC_CMP(sid1->sid_rev_num, sid2->sid_rev_num);
> 
> 	for (i = 0; i < 6; i++)
>-		if (sid1->id_auth[i] != sid2->id_auth[i])
>-			return sid1->id_auth[i] - sid2->id_auth[i];
>+		if (sid1->id_auth[i] != sid2->id_auth[i]) {
>+			return NUMERIC_CMP(sid1->id_auth[i], sid2->id_auth[i]);
>+		}
> 
> 	return 0;
> }
>-- 
>2.34.1
>
>
>From 2435a6a01efa52188b425676eb05a70820a1e54f Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:01:24 +1300
>Subject: [PATCH 29/65] s3:lib:util_tdb: use NUMERIC_CMP() in tdb_data_cmp()
>
>Although these are size_t, in practice TDB data is limited to 32 bit.
>Even so, overflow of a signed int is possible.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit dd4a0c276813b2c8516061110a7e580aa9afcf40)
>---
> source3/lib/util_tdb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source3/lib/util_tdb.c b/source3/lib/util_tdb.c
>index 943847f04a3..c5ccf748ee4 100644
>--- a/source3/lib/util_tdb.c
>+++ b/source3/lib/util_tdb.c
>@@ -324,11 +324,11 @@ int tdb_data_cmp(TDB_DATA t1, TDB_DATA t2)
> 		return 1;
> 	}
> 	if (t1.dptr == t2.dptr) {
>-		return t1.dsize - t2.dsize;
>+		return NUMERIC_CMP(t1.dsize, t2.dsize);
> 	}
> 	ret = memcmp(t1.dptr, t2.dptr, MIN(t1.dsize, t2.dsize));
> 	if (ret == 0) {
>-		return t1.dsize - t2.dsize;
>+		return NUMERIC_CMP(t1.dsize, t2.dsize);
> 	}
> 	return ret;
> }
>-- 
>2.34.1
>
>
>From cb384979da6a0862e4f0074e9a310768fcdbf9f2 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:10:45 +1300
>Subject: [PATCH 30/65] s4:rpc_server: compare_SamEntry() uses NUMERIC_CMP()
>
>SamEntry.idx is uint32_t.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit ed3ab87bdb0f6c6a9ea6323ed240fe267220b759)
>---
> source4/rpc_server/samr/dcesrv_samr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
>index b1342cbfe84..98cda8ad4b9 100644
>--- a/source4/rpc_server/samr/dcesrv_samr.c
>+++ b/source4/rpc_server/samr/dcesrv_samr.c
>@@ -1166,7 +1166,7 @@ static NTSTATUS dcesrv_samr_CreateDomainGroup(struct dcesrv_call_state *dce_call
> */
> static int compare_SamEntry(struct samr_SamEntry *e1, struct samr_SamEntry *e2)
> {
>-	return e1->idx - e2->idx;
>+	return NUMERIC_CMP(e1->idx, e2->idx);
> }
> 
> static int compare_msgRid(struct ldb_message **m1, struct ldb_message **m2) {
>-- 
>2.34.1
>
>
>From b3a7b12ba017a52a5fa9d0b69b9f6d377ecc642d Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:22:24 +1300
>Subject: [PATCH 31/65] s4:dns_server: use NUMERIC_CMP in rec_cmp()
>
>dnsp_DnssrvRpcRecord.dwTimeStamp is uint32_t, making overflow possible.
>
>dnsp_DnssrvRpcRecord.wType is an enum, which has the size of an int,
>though it may be hard to set it to overflowing values.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 42ead213484840121ce6bc0db22941ea0a019105)
>---
> source4/dns_server/dnsserver_common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/source4/dns_server/dnsserver_common.c b/source4/dns_server/dnsserver_common.c
>index 0bbba7ff6de..79468dbb8fe 100644
>--- a/source4/dns_server/dnsserver_common.c
>+++ b/source4/dns_server/dnsserver_common.c
>@@ -642,7 +642,7 @@ static int rec_cmp(const struct dnsp_DnssrvRpcRecord *r1,
> 		 * The records are sorted with higher types first,
> 		 * which puts tombstones (type 0) last.
> 		 */
>-		return r2->wType - r1->wType;
>+		return NUMERIC_CMP(r2->wType, r1->wType);
> 	}
> 	/*
> 	 * Then we need to sort from the oldest to newest timestamp.
>@@ -650,7 +650,7 @@ static int rec_cmp(const struct dnsp_DnssrvRpcRecord *r1,
> 	 * Note that dwTimeStamp == 0 (never expiring) records come first,
> 	 * then the ones whose expiry is soonest.
> 	 */
>-	return r1->dwTimeStamp - r2->dwTimeStamp;
>+	return NUMERIC_CMP(r1->dwTimeStamp, r2->dwTimeStamp);
> }
> 
> /*
>-- 
>2.34.1
>
>
>From 4464e756d0360f2e8795beb8c8ff38b510ba17a3 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:54:09 +1300
>Subject: [PATCH 32/65] s4:wins: use NUMERIC_CMP in winsdb_addr_sort_list()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 66d47537e42caa528c7fab670d9c35d27c513cce)
>---
> source4/nbt_server/wins/winsdb.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source4/nbt_server/wins/winsdb.c b/source4/nbt_server/wins/winsdb.c
>index 2a05e96bca4..eb554fc5bd3 100644
>--- a/source4/nbt_server/wins/winsdb.c
>+++ b/source4/nbt_server/wins/winsdb.c
>@@ -32,6 +32,7 @@
> #include "lib/socket/netif.h"
> #include "param/param.h"
> #include "lib/util/smb_strtox.h"
>+#include "lib/util/tsort.h"
> 
> #undef strcasecmp
> 
>@@ -360,7 +361,7 @@ static int winsdb_addr_sort_list (struct winsdb_addr **p1, struct winsdb_addr **
> 		a1_owned = true;
> 	}
> 
>-	return a2_owned - a1_owned;
>+	return NUMERIC_CMP(a2_owned, a1_owned);
> }
> 
> struct winsdb_addr **winsdb_addr_list_add(struct winsdb_handle *h, const struct winsdb_record *rec,
>-- 
>2.34.1
>
>
>From 059f27f17991a8791a56cdfba45e430d4ab8e7f7 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:16:21 +1300
>Subject: [PATCH 33/65] s4:wins: winsdb_addr_sort_list() uses NUMERIC_CMP()
>
>expire_time is time_t, which is at least int-sized, so overflow is
>possible (if this code ever runs).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit d8b97649ef4d3ccaf53878021be0e2d4824b982c)
>---
> source4/nbt_server/wins/winsdb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source4/nbt_server/wins/winsdb.c b/source4/nbt_server/wins/winsdb.c
>index eb554fc5bd3..7df40c33135 100644
>--- a/source4/nbt_server/wins/winsdb.c
>+++ b/source4/nbt_server/wins/winsdb.c
>@@ -350,7 +350,7 @@ static int winsdb_addr_sort_list (struct winsdb_addr **p1, struct winsdb_addr **
> 	 * then the replica addresses with the newest to the oldest address
> 	 */
> 	if (a2->expire_time != a1->expire_time) {
>-		return a2->expire_time - a1->expire_time;
>+		return NUMERIC_CMP(a2->expire_time, a1->expire_time);
> 	}
> 
> 	if (strcmp(a2->wins_owner, h->local_owner) == 0) {
>-- 
>2.34.1
>
>
>From 87da4196f7f41bbe37c1a52946f7bd06ab75a0ab Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:53:32 +1300
>Subject: [PATCH 34/65] s4:wins: use NUMERIC_CMP in
> nbtd_wins_randomize1Clist_sort()
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit a197be2003d7e248b1e1294f4ad5473f48762bce)
>---
> source4/nbt_server/wins/winsserver.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/source4/nbt_server/wins/winsserver.c b/source4/nbt_server/wins/winsserver.c
>index a9f3ecd7d29..6679961dc03 100644
>--- a/source4/nbt_server/wins/winsserver.c
>+++ b/source4/nbt_server/wins/winsserver.c
>@@ -36,6 +36,7 @@
> #include "param/param.h"
> #include "libcli/resolve/resolve.h"
> #include "lib/util/util_net.h"
>+#include "lib/util/tsort.h"
> 
> /*
>   work out the ttl we will use given a client requested ttl
>@@ -653,7 +654,7 @@ static int nbtd_wins_randomize1Clist_sort(void *p1,/* (const char **) */
> 	match_bits1 = ipv4_match_bits(interpret_addr2(a1), interpret_addr2(src->addr));
> 	match_bits2 = ipv4_match_bits(interpret_addr2(a2), interpret_addr2(src->addr));
> 
>-	return match_bits2 - match_bits1;
>+	return NUMERIC_CMP(match_bits2, match_bits1);
> }
> 
> static void nbtd_wins_randomize1Clist(struct loadparm_context *lp_ctx,
>-- 
>2.34.1
>
>
>From f1c5d0075050020f24ad5f234848f13bb2ea43d5 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:25:54 +1300
>Subject: [PATCH 35/65] s3:util:net_registry: registry_value_cmp() uses
> NUMERIC_CMP()
>
>v->type is an int-sized enum, so overflow might be possible if it could
>be arbitrarily set.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(backported from commit 5e99262aaf5fc6601f3859c8b060b680b11bf6ea)
>[dbagnall@samba.org: function has moved in master]
>---
> source3/registry/reg_api_util.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/registry/reg_api_util.c b/source3/registry/reg_api_util.c
>index eb83fa8f597..9424799a397 100644
>--- a/source3/registry/reg_api_util.c
>+++ b/source3/registry/reg_api_util.c
>@@ -243,5 +243,5 @@ int registry_value_cmp(const struct registry_value* v1, const struct registry_va
> 	if (v1->type == v2->type) {
> 		return data_blob_cmp(&v1->data, &v2->data);
> 	}
>-	return v1->type - v2->type;
>+	return NUMERIC_CMP(v1->type, v2->type);
> }
>-- 
>2.34.1
>
>
>From ba3be64db7726c154e5a63cf648944e5f6212b7c Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 3 Apr 2024 12:56:48 +1300
>Subject: [PATCH 36/65] s3:smbcacls: use NUMERIC_CMP in ace_compare
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 31101a9fa1503be9d8137e42466f57d85136a156)
>---
> source3/utils/smbcacls.c | 15 ++++++++-------
> 1 file changed, 8 insertions(+), 7 deletions(-)
>
>diff --git a/source3/utils/smbcacls.c b/source3/utils/smbcacls.c
>index 7ce7d2415dc..30b25d02d0b 100644
>--- a/source3/utils/smbcacls.c
>+++ b/source3/utils/smbcacls.c
>@@ -553,22 +553,23 @@ static int ace_compare(struct security_ace *ace1, struct security_ace *ace2)
> 		return -1;
> 	if ((ace1->flags & SEC_ACE_FLAG_INHERITED_ACE) &&
> 			(ace2->flags & SEC_ACE_FLAG_INHERITED_ACE))
>-		return ace1 - ace2;
>-
>-	if (ace1->type != ace2->type)
>-		return ace2->type - ace1->type;
>+		return NUMERIC_CMP(ace1, ace2);
> 
>+	if (ace1->type != ace2->type) {
>+		/* note the reverse order */
>+		return NUMERIC_CMP(ace2->type, ace1->type);
>+	}
> 	if (dom_sid_compare(&ace1->trustee, &ace2->trustee))
> 		return dom_sid_compare(&ace1->trustee, &ace2->trustee);
> 
> 	if (ace1->flags != ace2->flags)
>-		return ace1->flags - ace2->flags;
>+		return NUMERIC_CMP(ace1->flags, ace2->flags);
> 
> 	if (ace1->access_mask != ace2->access_mask)
>-		return ace1->access_mask - ace2->access_mask;
>+		return NUMERIC_CMP(ace1->access_mask, ace2->access_mask);
> 
> 	if (ace1->size != ace2->size)
>-		return ace1->size - ace2->size;
>+		return NUMERIC_CMP(ace1->size, ace2->size);
> 
> 	return memcmp(ace1, ace2, sizeof(struct security_ace));
> }
>-- 
>2.34.1
>
>
>From 628bb066cb26334532ebdd387fe1cbe542e39f24 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:08:02 +1300
>Subject: [PATCH 37/65] s3:util:sharesec ace_compare() uses NUMERIC_CMP()
>
>ace->access_mask is uint32_t, so can overflow a signed int.
>This would be easy to trigger, as it is a flags field rather than an
>allocation count.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit e35d54fd4d381df67ab9b4f8390e2109b2142678)
>---
> source3/utils/sharesec.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/source3/utils/sharesec.c b/source3/utils/sharesec.c
>index 9b8064de702..8f6117209f4 100644
>--- a/source3/utils/sharesec.c
>+++ b/source3/utils/sharesec.c
>@@ -119,19 +119,19 @@ static int ace_compare(struct security_ace *ace1, struct security_ace *ace2)
> 		return 0;
> 
> 	if (ace1->type != ace2->type)
>-		return ace2->type - ace1->type;
>+		return NUMERIC_CMP(ace2->type, ace1->type);
> 
> 	if (dom_sid_compare(&ace1->trustee, &ace2->trustee))
> 		return dom_sid_compare(&ace1->trustee, &ace2->trustee);
> 
> 	if (ace1->flags != ace2->flags)
>-		return ace1->flags - ace2->flags;
>+		return NUMERIC_CMP(ace1->flags, ace2->flags);
> 
> 	if (ace1->access_mask != ace2->access_mask)
>-		return ace1->access_mask - ace2->access_mask;
>+		return NUMERIC_CMP(ace1->access_mask, ace2->access_mask);
> 
> 	if (ace1->size != ace2->size)
>-		return ace1->size - ace2->size;
>+		return NUMERIC_CMP(ace1->size, ace2->size);
> 
> 	return memcmp(ace1, ace2, sizeof(struct security_ace));
> }
>-- 
>2.34.1
>
>
>From 6af04f38fd2cf7e0d9bb4d28849cfd2f7f52c1a7 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 4 Apr 2024 14:33:47 +1300
>Subject: [PATCH 38/65] s3:libsmb_xattr: ace_compare() uses NUMERIC_CMP()
>
>the access_mask is the easiest to overflow with subtraction -- other
>fields are 8 or 16 bit.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>
>Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
>Autobuild-Date(master): Wed Apr 10 23:58:12 UTC 2024 on atb-devel-224
>
>(cherry picked from commit 81598b42455d6758941da532c668b6d4e969cc40)
>---
> source3/libsmb/libsmb_xattr.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
>diff --git a/source3/libsmb/libsmb_xattr.c b/source3/libsmb/libsmb_xattr.c
>index c8aa0743f54..d56279dbf73 100644
>--- a/source3/libsmb/libsmb_xattr.c
>+++ b/source3/libsmb/libsmb_xattr.c
>@@ -121,7 +121,13 @@ ace_compare(struct security_ace *ace1,
>          */
> 
> 	if (ace1->type != ace2->type) {
>-		return ace2->type - ace1->type;
>+		/*
>+		 * ace2 and ace1 are reversed here, so that
>+		 * ACCESS_DENIED_ACE_TYPE (1) sorts before
>+		 * ACCESS_ALLOWED_ACE_TYPE (0), which is the order you
>+		 * usually want.
>+		 */
>+		return NUMERIC_CMP(ace2->type, ace1->type);
>         }
> 
> 	if (dom_sid_compare(&ace1->trustee, &ace2->trustee)) {
>@@ -129,15 +135,15 @@ ace_compare(struct security_ace *ace1,
>         }
> 
> 	if (ace1->flags != ace2->flags) {
>-		return ace1->flags - ace2->flags;
>+		return NUMERIC_CMP(ace1->flags, ace2->flags);
>         }
> 
> 	if (ace1->access_mask != ace2->access_mask) {
>-		return ace1->access_mask - ace2->access_mask;
>+		return NUMERIC_CMP(ace1->access_mask, ace2->access_mask);
>         }
> 
> 	if (ace1->size != ace2->size) {
>-		return ace1->size - ace2->size;
>+		return NUMERIC_CMP(ace1->size, ace2->size);
>         }
> 
> 	return memcmp(ace1, ace2, sizeof(struct security_ace));
>-- 
>2.34.1
>
>
>From 183bf959e4e372a7cc283e9fb35531e0354e28fc Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 14:54:34 +1200
>Subject: [PATCH 39/65] ldb:mod:sort: rearrange NULL checks
>
>There are further changes coming here.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit d785c1991c922150bab38c36cef3a799448ac304)
>---
> lib/ldb/modules/sort.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
>diff --git a/lib/ldb/modules/sort.c b/lib/ldb/modules/sort.c
>index cb6f8df440f..8487c7003b6 100644
>--- a/lib/ldb/modules/sort.c
>+++ b/lib/ldb/modules/sort.c
>@@ -121,15 +121,18 @@ static int sort_compare(struct ldb_message **msg1, struct ldb_message **msg2, vo
> 	el1 = ldb_msg_find_element(*msg1, ac->attributeName);
> 	el2 = ldb_msg_find_element(*msg2, ac->attributeName);
> 
>-	if (!el1 && el2) {
>+	/*
>+	 * NULL elements sort at the end (regardless of ac->reverse flag).
>+	 */
>+	if (el1 == NULL && el2 == NULL) {
>+		return 0;
>+	}
>+	if (el1 == NULL) {
> 		return 1;
> 	}
>-	if (el1 && !el2) {
>+	if (el2 == NULL) {
> 		return -1;
> 	}
>-	if (!el1 && !el2) {
>-		return 0;
>-	}
> 
> 	if (ac->reverse)
> 		return ac->a->syntax->comparison_fn(ldb, ac, &el2->values[0], &el1->values[0]);
>-- 
>2.34.1
>
>
>From acb2ad6b45e8d56465c5cb0638de8abc86bd860b Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 14:55:27 +1200
>Subject: [PATCH 40/65] ldb:sort: check that elements have values
>
>We assume no values is unlikely, since we have been dereferencing
>->values[0] forever, with no known reports of trouble.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit d4e69734c65ade0bbb398447012513a7f27e98bd)
>---
> lib/ldb/modules/sort.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ldb/modules/sort.c b/lib/ldb/modules/sort.c
>index 8487c7003b6..a4a77329cee 100644
>--- a/lib/ldb/modules/sort.c
>+++ b/lib/ldb/modules/sort.c
>@@ -122,7 +122,8 @@ static int sort_compare(struct ldb_message **msg1, struct ldb_message **msg2, vo
> 	el2 = ldb_msg_find_element(*msg2, ac->attributeName);
> 
> 	/*
>-	 * NULL elements sort at the end (regardless of ac->reverse flag).
>+	 * NULL and empty elements sort at the end (regardless of ac->reverse flag).
>+	 * NULL elements come after empty ones.
> 	 */
> 	if (el1 == NULL && el2 == NULL) {
> 		return 0;
>@@ -133,6 +134,15 @@ static int sort_compare(struct ldb_message **msg1, struct ldb_message **msg2, vo
> 	if (el2 == NULL) {
> 		return -1;
> 	}
>+	if (unlikely(el1->num_values == 0 && el2->num_values == 0)) {
>+		return 0;
>+	}
>+	if (unlikely(el1->num_values == 0)) {
>+		return 1;
>+	}
>+	if (unlikely(el2->num_values == 0)) {
>+		return -1;
>+	}
> 
> 	if (ac->reverse)
> 		return ac->a->syntax->comparison_fn(ldb, ac, &el2->values[0], &el1->values[0]);
>-- 
>2.34.1
>
>
>From 08ab3c2246013204ed78525475462382bb33aebd Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 14:58:48 +1200
>Subject: [PATCH 41/65] ldb:sort: generalise both-NULL check to equality check
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 531f31df99341b2cb1afc42538022451ca771983)
>---
> lib/ldb/modules/sort.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/modules/sort.c b/lib/ldb/modules/sort.c
>index a4a77329cee..72c60fc894a 100644
>--- a/lib/ldb/modules/sort.c
>+++ b/lib/ldb/modules/sort.c
>@@ -125,7 +125,7 @@ static int sort_compare(struct ldb_message **msg1, struct ldb_message **msg2, vo
> 	 * NULL and empty elements sort at the end (regardless of ac->reverse flag).
> 	 * NULL elements come after empty ones.
> 	 */
>-	if (el1 == NULL && el2 == NULL) {
>+	if (el1 == el2) {
> 		return 0;
> 	}
> 	if (el1 == NULL) {
>-- 
>2.34.1
>
>
>From f151ea2521032aed25b423ab5e0d970fe76c0b68 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:04:43 +1200
>Subject: [PATCH 42/65] ldb:dn: make ldb_dn_compare() self-consistent
>
>We were returning -1 in all these cases:
>
>   ldb_dn_compare(dn, NULL);
>   ldb_dn_compare(NULL, dn);
>   ldb_dn_compare(NULL, NULL);
>
>which would give strange results in sort, where this is often used.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5fe488d515a8bb719bdeafb8b64d8479732b5ac8)
>---
> lib/ldb/common/ldb_dn.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
>index c86a7f6692a..2674094d7f6 100644
>--- a/lib/ldb/common/ldb_dn.c
>+++ b/lib/ldb/common/ldb_dn.c
>@@ -1136,8 +1136,22 @@ int ldb_dn_compare(struct ldb_dn *dn0, struct ldb_dn *dn1)
> {
> 	unsigned int i;
> 	int ret;
>+	/*
>+	 * If used in sort, we shift NULL and invalid DNs to the end.
>+	 *
>+	 * If ldb_dn_casefold_internal() fails, that goes to the end too, so
>+	 * we end up with:
>+	 *
>+	 * | normal DNs, sorted | casefold failed DNs | invalid DNs | NULLs |
>+	 */
> 
>-	if (( ! dn0) || dn0->invalid || ! dn1 || dn1->invalid) {
>+	if (dn0 == dn1 || (dn0->invalid && dn1->invalid)) {
>+		return 0;
>+	}
>+	if (dn0 == NULL || dn0->invalid) {
>+		return 1;
>+	}
>+	if (dn1 == NULL || dn1->invalid) {
> 		return -1;
> 	}
> 
>-- 
>2.34.1
>
>
>From 6e8638d982971a8c917a49b9dd951d411b465d05 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:07:20 +1200
>Subject: [PATCH 43/65] s3:brlock: use NUMERIC_CMP in #ifdef-zeroed
> lock_compare
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 9b73235d4957a487fbb3214fdfda6461a2cf0b21)
>---
> source3/locking/brlock.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
>diff --git a/source3/locking/brlock.c b/source3/locking/brlock.c
>index 68ade6b6d59..a91216f2e72 100644
>--- a/source3/locking/brlock.c
>+++ b/source3/locking/brlock.c
>@@ -413,12 +413,9 @@ static int lock_compare(const struct lock_struct *lck1,
> 			 const struct lock_struct *lck2)
> {
> 	if (lck1->start != lck2->start) {
>-		return (lck1->start - lck2->start);
>+		return NUMERIC_CMP(lck1->start, lck2->start);
> 	}
>-	if (lck2->size != lck1->size) {
>-		return ((int)lck1->size - (int)lck2->size);
>-	}
>-	return 0;
>+	return NUMERIC_CMP(lck1->size, lck2->size);
> }
> #endif
> 
>-- 
>2.34.1
>
>
>From f08a59f6e3c7d4dda9d5aa23e5fdf7285f5b242e Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:12:56 +1200
>Subject: [PATCH 44/65] s3:mod:posixacl_xattr: use NUMERIC_CMP in
> posixacl_xattr_entry_compare
>
>The first subtraction was between uint16_t, so is safe with 32 bit
>int, but the second compared uint32_t, so was not safe.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 8b2605a5d9cc14f9e6ddf2db704cdca2f523d74e)
>---
> source3/modules/posixacl_xattr.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source3/modules/posixacl_xattr.c b/source3/modules/posixacl_xattr.c
>index 365cdc79973..5d0516cf754 100644
>--- a/source3/modules/posixacl_xattr.c
>+++ b/source3/modules/posixacl_xattr.c
>@@ -226,14 +226,14 @@ static int posixacl_xattr_entry_compare(const void *left, const void *right)
> 	tag_left = SVAL(left, 0);
> 	tag_right = SVAL(right, 0);
> 
>-	ret = (tag_left - tag_right);
>-	if (!ret) {
>+	ret = NUMERIC_CMP(tag_left, tag_right);
>+	if (ret == 0) {
> 		/* ID is the third element in the entry, after two short
> 		   integers (tag and perm), i.e at offset 4.
> 		*/
> 		id_left = IVAL(left, 4);
> 		id_right = IVAL(right, 4);
>-		ret = id_left - id_right;
>+		ret = NUMERIC_CMP(id_left, id_right);
> 	}
> 
> 	return ret;
>-- 
>2.34.1
>
>
>From cf2d2281d2328728d48f68536841f9b191e1c767 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:17:22 +1200
>Subject: [PATCH 45/65] s3:mod:vfs_vxfs: use NUMERIC_CMP in vxfs_ace_cmp
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 386216d4a158d8bafb0879a0a753da096a939b93)
>---
> source3/modules/vfs_vxfs.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
>diff --git a/source3/modules/vfs_vxfs.c b/source3/modules/vfs_vxfs.c
>index ddd34ba812a..efd40849eb7 100644
>--- a/source3/modules/vfs_vxfs.c
>+++ b/source3/modules/vfs_vxfs.c
>@@ -92,13 +92,13 @@ static int vxfs_ace_cmp(const void *ace1, const void *ace2)
> 	type_a1 = SVAL(ace1, 0);
> 	type_a2 = SVAL(ace2, 0);
> 
>-	ret = (type_a1 - type_a2);
>-	if (!ret) {
>+	ret = NUMERIC_CMP(type_a1, type_a2);
>+	if (ret == 0) {
> 		/* Compare ID under type */
> 		/* skip perm thus take offset as 4*/
> 		id_a1 = IVAL(ace1, 4);
> 		id_a2 = IVAL(ace2, 4);
>-		ret = id_a1 - id_a2;
>+		ret = NUMERIC_CMP(id_a1, id_a2);
> 	}
> 
> 	return ret;
>-- 
>2.34.1
>
>
>From 83ed1e7666a2d454b637cbd56d1ed740afe78d23 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:36:06 +1200
>Subject: [PATCH 46/65] dsdb:schema: use NUMERIC_CMP in place of uint32_cmp
>
>uint32_cmp (introduced in 0c362597c0f933b3612bb17328c0a13b73d72e43
>"fixed the sorting of schema attributes") was doing what NUMERIC_CMP
>does, but it was adding an extra function call. This results in less
>code.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 8317a6173646d425dc99e08bbf3d6086b0086bc5)
>---
> source4/dsdb/schema/schema_set.c | 14 ++++----------
> 1 file changed, 4 insertions(+), 10 deletions(-)
>
>diff --git a/source4/dsdb/schema/schema_set.c b/source4/dsdb/schema/schema_set.c
>index 03cf2405595..42a8657f1b7 100644
>--- a/source4/dsdb/schema/schema_set.c
>+++ b/source4/dsdb/schema/schema_set.c
>@@ -436,19 +436,13 @@ static void dsdb_setup_attribute_shortcuts(struct ldb_context *ldb, struct dsdb_
> 	}
> }
> 
>-static int uint32_cmp(uint32_t c1, uint32_t c2)
>-{
>-	if (c1 == c2) return 0;
>-	return c1 > c2 ? 1 : -1;
>-}
>-
> static int dsdb_compare_class_by_lDAPDisplayName(struct dsdb_class **c1, struct dsdb_class **c2)
> {
> 	return strcasecmp((*c1)->lDAPDisplayName, (*c2)->lDAPDisplayName);
> }
> static int dsdb_compare_class_by_governsID_id(struct dsdb_class **c1, struct dsdb_class **c2)
> {
>-	return uint32_cmp((*c1)->governsID_id, (*c2)->governsID_id);
>+	return NUMERIC_CMP((*c1)->governsID_id, (*c2)->governsID_id);
> }
> static int dsdb_compare_class_by_governsID_oid(struct dsdb_class **c1, struct dsdb_class **c2)
> {
>@@ -465,11 +459,11 @@ static int dsdb_compare_attribute_by_lDAPDisplayName(struct dsdb_attribute **a1,
> }
> static int dsdb_compare_attribute_by_attributeID_id(struct dsdb_attribute **a1, struct dsdb_attribute **a2)
> {
>-	return uint32_cmp((*a1)->attributeID_id, (*a2)->attributeID_id);
>+	return NUMERIC_CMP((*a1)->attributeID_id, (*a2)->attributeID_id);
> }
> static int dsdb_compare_attribute_by_msDS_IntId(struct dsdb_attribute **a1, struct dsdb_attribute **a2)
> {
>-	return uint32_cmp((*a1)->msDS_IntId, (*a2)->msDS_IntId);
>+	return NUMERIC_CMP((*a1)->msDS_IntId, (*a2)->msDS_IntId);
> }
> static int dsdb_compare_attribute_by_attributeID_oid(struct dsdb_attribute **a1, struct dsdb_attribute **a2)
> {
>@@ -477,7 +471,7 @@ static int dsdb_compare_attribute_by_attributeID_oid(struct dsdb_attribute **a1,
> }
> static int dsdb_compare_attribute_by_linkID(struct dsdb_attribute **a1, struct dsdb_attribute **a2)
> {
>-	return uint32_cmp((*a1)->linkID, (*a2)->linkID);
>+	return NUMERIC_CMP((*a1)->linkID, (*a2)->linkID);
> }
> 
> /**
>-- 
>2.34.1
>
>
>From 7908cfa80f377e604fdadb5f91c20d8b6ff7b4b1 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:47:12 +1200
>Subject: [PATCH 47/65] s3:rpc:wkssvc_nt: dom_user_cmp uses NUMERIC_CMP
>
>usr->login_time is time_t, which is often bigger than int.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 75682e397b9cf22d04a5d80252554c6b2e376793)
>---
> source3/rpc_server/wkssvc/srv_wkssvc_nt.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/rpc_server/wkssvc/srv_wkssvc_nt.c b/source3/rpc_server/wkssvc/srv_wkssvc_nt.c
>index 592e8668cd3..89cdeb6daa1 100644
>--- a/source3/rpc_server/wkssvc/srv_wkssvc_nt.c
>+++ b/source3/rpc_server/wkssvc/srv_wkssvc_nt.c
>@@ -140,7 +140,7 @@ static int dom_user_cmp(const struct dom_usr *usr1, const struct dom_usr *usr2)
> 	/* Called from qsort to compare two domain users in a dom_usr_t array
> 	 * for sorting by login time. Return >0 if usr1 login time was later
> 	 * than usr2 login time, <0 if it was earlier */
>-	return (usr1->login_time - usr2->login_time);
>+	return NUMERIC_CMP(usr1->login_time, usr2->login_time);
> }
> 
> /*******************************************************************
>-- 
>2.34.1
>
>
>From f2ab1b6c9c8b537d5f92917b95fd9a14045017f2 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Sun, 7 Apr 2024 15:54:02 +1200
>Subject: [PATCH 48/65] gensec: sort_gensec uses NUMERIC_CMP
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit acaa1323d0337ae9339dfff9f856ea54725a86ac)
>---
> auth/gensec/gensec_start.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/auth/gensec/gensec_start.c b/auth/gensec/gensec_start.c
>index bd5b7259d3f..7c0d63564c2 100644
>--- a/auth/gensec/gensec_start.c
>+++ b/auth/gensec/gensec_start.c
>@@ -1103,7 +1103,7 @@ _PUBLIC_ const struct gensec_critical_sizes *gensec_interface_version(void)
> }
> 
> static int sort_gensec(const struct gensec_security_ops **gs1, const struct gensec_security_ops **gs2) {
>-	return (*gs2)->priority - (*gs1)->priority;
>+	return NUMERIC_CMP((*gs2)->priority, (*gs1)->priority);
> }
> 
> int gensec_setting_int(struct gensec_settings *settings, const char *mechanism, const char *name, int default_value)
>-- 
>2.34.1
>
>
>From db3ce1120fa4c282439c38f733cb25c4179543a4 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 8 Apr 2024 17:06:57 +1200
>Subject: [PATCH 49/65] lib/socket: rearrange iface_comp() to use NUMERIC_CMP
>
>We rearrange rather than just replacing the subtraction, because that
>would call ntohl() more than necessary, and I think the flow is a bit
>clearer this way.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 7ba6fcb93656e5e88e1d5bcd6002747aa64f0a3a)
>---
> lib/socket/interfaces.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
>diff --git a/lib/socket/interfaces.c b/lib/socket/interfaces.c
>index 3157e0cef12..b9c9f65a5ba 100644
>--- a/lib/socket/interfaces.c
>+++ b/lib/socket/interfaces.c
>@@ -386,18 +386,18 @@ static int iface_comp(struct iface_struct *i1, struct iface_struct *i2)
> 	if (((struct sockaddr *)&i1->ip)->sa_family == AF_INET) {
> 		struct sockaddr_in *s1 = (struct sockaddr_in *)&i1->ip;
> 		struct sockaddr_in *s2 = (struct sockaddr_in *)&i2->ip;
>-
>-		r = ntohl(s1->sin_addr.s_addr) -
>-			ntohl(s2->sin_addr.s_addr);
>-		if (r) {
>-			return r;
>+		uint32_t a1 = ntohl(s1->sin_addr.s_addr);
>+		uint32_t a2 = ntohl(s2->sin_addr.s_addr);
>+		r = NUMERIC_CMP(a1, a2);
>+		if (r == 0) {
>+			/* compare netmasks as a tiebreaker */
>+			s1 = (struct sockaddr_in *)&i1->netmask;
>+			s2 = (struct sockaddr_in *)&i2->netmask;
>+			a1 = ntohl(s1->sin_addr.s_addr);
>+			a2 = ntohl(s2->sin_addr.s_addr);
>+			r = NUMERIC_CMP(a1, a2);
> 		}
>-
>-		s1 = (struct sockaddr_in *)&i1->netmask;
>-		s2 = (struct sockaddr_in *)&i2->netmask;
>-
>-		return ntohl(s1->sin_addr.s_addr) -
>-			ntohl(s2->sin_addr.s_addr);
>+		return r;
> 	}
> 	return 0;
> }
>-- 
>2.34.1
>
>
>From 006e7f5648d0b51bda8d243a348ea03b186e24c1 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 8 Apr 2024 17:08:03 +1200
>Subject: [PATCH 50/65] s3:libsmb:nmblib: use NUMERIC_CMP in status_compare
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 31c322874b8b65518cec945e05a42fd014e6390b)
>---
> source3/libsmb/nmblib.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
>diff --git a/source3/libsmb/nmblib.c b/source3/libsmb/nmblib.c
>index 607470f7e4e..4cfca7f369d 100644
>--- a/source3/libsmb/nmblib.c
>+++ b/source3/libsmb/nmblib.c
>@@ -1229,8 +1229,10 @@ static unsigned char sort_ip[4];
> 
> static int name_query_comp(unsigned char *p1, unsigned char *p2)
> {
>-	return matching_len_bits(p2+2, sort_ip, 4) -
>-		matching_len_bits(p1+2, sort_ip, 4);
>+	int a = matching_len_bits(p1+2, sort_ip, 4);
>+	int b = matching_len_bits(p2+2, sort_ip, 4);
>+	/* reverse sort -- p2 derived value comes first */
>+	return NUMERIC_CMP(b, a);
> }
> 
> /****************************************************************************
>-- 
>2.34.1
>
>
>From 30209598c21da0d5fe9858c8e57baa6bc6af5f0c Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 8 Apr 2024 22:54:49 +1200
>Subject: [PATCH 51/65] s4:rpcsrv:dnsserver: make dns_name_compare transitive
> with NULLs
>
>Returning 0 on `(name1 == NULL || name2 == NULL)` made NULL equal to
>everything, which confuses a sort (consider {A, B, NULL} where A > B,
>but A == NULL == B).
>
>The only caller is dnsserver_enumerate_records() which fails if it
>finds a NULL in the sorted list. We make the happen more quickly by
>sorting NULLs to the front.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 7be535315a5eed5d5b7eaea025ecf9f55e772e8e)
>---
> source4/rpc_server/dnsserver/dnsdata.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
>diff --git a/source4/rpc_server/dnsserver/dnsdata.c b/source4/rpc_server/dnsserver/dnsdata.c
>index 002d9e622cc..4b2c04a089e 100644
>--- a/source4/rpc_server/dnsserver/dnsdata.c
>+++ b/source4/rpc_server/dnsserver/dnsdata.c
>@@ -1075,9 +1075,23 @@ int dns_name_compare(struct ldb_message * const *m1, struct ldb_message * const
> 
> 	name1 = ldb_msg_find_attr_as_string(*m1, "name", NULL);
> 	name2 = ldb_msg_find_attr_as_string(*m2, "name", NULL);
>-	if (name1 == NULL || name2 == NULL) {
>+	/*
>+	 * We sort NULL names to the start of the list, because the only
>+	 * caller of this function, dnsserver_enumerate_records() will call
>+	 * dns_build_tree() with the sorted list, which will always return an
>+	 * error when it hits a NULL, so we might as well make that happen
>+	 * quickly.
>+	 */
>+	if (name1 == name2) {
>+		/* this includes the both NULL case */
> 		return 0;
> 	}
>+	if (name1 == NULL) {
>+		return -1;
>+	}
>+	if (name2 == NULL) {
>+		return 1;
>+	}
> 
> 	/* Compare the last components of names.
> 	 * If search_name is not NULL, compare the second last components of names */
>-- 
>2.34.1
>
>
>From 1ab6fcb9a10b01671d84336fef766f4ceac7abc8 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Mon, 8 Apr 2024 22:55:50 +1200
>Subject: [PATCH 52/65] s4:rpcsrv:samr: improve a comment in compare_msgRid
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 6229feab74a734190c302ee9b1cc36960669743d)
>---
> source4/rpc_server/samr/dcesrv_samr.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/source4/rpc_server/samr/dcesrv_samr.c b/source4/rpc_server/samr/dcesrv_samr.c
>index 98cda8ad4b9..18c15ad2632 100644
>--- a/source4/rpc_server/samr/dcesrv_samr.c
>+++ b/source4/rpc_server/samr/dcesrv_samr.c
>@@ -1197,8 +1197,9 @@ static int compare_msgRid(struct ldb_message **m1, struct ldb_message **m2) {
> 	}
> 
> 	/*
>-	 * Get and compare the rids, if we fail to extract a rid treat it as a
>-	 * missing SID and sort to the end of the list
>+	 * Get and compare the rids. If we fail to extract a rid (because
>+	 * there are no subauths) the msg goes to the end of the list, but
>+	 * before the NULL SIDs.
> 	 */
> 	status = dom_sid_split_rid(NULL, sid1, NULL, &rid1);
> 	if (!NT_STATUS_IS_OK(status)) {
>-- 
>2.34.1
>
>
>From 7f4df2cfaaa5409937656c0ccc8ee592aeead98c Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 10 Apr 2024 10:54:31 +1200
>Subject: [PATCH 53/65] ldb-samba: ldif-handlers: make
> ldif_comparison_objectSid() accurate
>
>This function compares blobs that might be SID strings or might be SID
>structures. Until now, if they were both (seemingly) strings, they were
>compared as strings, otherwise if either was a string it was converted to
>a structure blob, then the blobs were compared. This had two big problems:
>
>1. There is variety in the way a SID can be stringified. For example,
>   "s-1-02-3" means the same SID as "S-1-2-3", but those wouldn't compare
>   equal.
>
>2. SID comparison was crazily non-transitive. Consider the three values
>     a = "S-1-2-3-4-5",
>     b = "S-1-9-1",
>     c = SID("S-1-11-1"), where c is a struct and the others are string.
>
>   then we had,
>     a < b, because the 5th character '2' < '9'.
>     a > c, because when converted to a structure, the number of sub-auths
>            is the first varying byte. a has 3, c has 0.
>     b < c, because after the sub-auth count comes the id_auth value
>            (big-endian, which doesn't matter in this case).
>
>That made the function unreliable for sorting, AND for simple equality
>tests. Also it leaked.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 6722e80d1b3a252a1ed714be4a35185cd99971e3)
>---
> lib/ldb-samba/ldif_handlers.c | 59 +++++++++++++++++++++--------------
> 1 file changed, 35 insertions(+), 24 deletions(-)
>
>diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
>index 78a433748bb..a3ea70c83b3 100644
>--- a/lib/ldb-samba/ldif_handlers.c
>+++ b/lib/ldb-samba/ldif_handlers.c
>@@ -150,36 +150,47 @@ bool ldif_comparision_objectSid_isString(const struct ldb_val *v)
> 
> /*
>   compare two objectSids
>+
>+  If the SIDs seem to be strings, they are converted to binary form.
> */
> static int ldif_comparison_objectSid(struct ldb_context *ldb, void *mem_ctx,
> 				    const struct ldb_val *v1, const struct ldb_val *v2)
> {
>-	if (ldif_comparision_objectSid_isString(v1) && ldif_comparision_objectSid_isString(v2)) {
>-		return ldb_comparison_binary(ldb, mem_ctx, v1, v2);
>-	} else if (ldif_comparision_objectSid_isString(v1)
>-		   && !ldif_comparision_objectSid_isString(v2)) {
>-		struct ldb_val v;
>-		int ret;
>-		if (ldif_read_objectSid(ldb, mem_ctx, v1, &v) != 0) {
>-			/* Perhaps not a string after all */
>-			return ldb_comparison_binary(ldb, mem_ctx, v1, v2);
>+	bool v1_is_string = ldif_comparision_objectSid_isString(v1);
>+	bool v2_is_string = ldif_comparision_objectSid_isString(v2);
>+	struct ldb_val parsed_1 = {};
>+	struct ldb_val parsed_2 = {};
>+	int ret;
>+	/*
>+	 * If the ldb_vals look like SID strings (i.e. start with "S-"
>+	 * or "s-"), we try to parse them as such. If that fails, we
>+	 * assume they are binary SIDs, even though that's not really
>+	 * possible -- the first two bytes of a struct dom_sid are the
>+	 * version (1), and the number of sub-auths (<= 15), neither
>+	 * of which are close to 'S' or '-'.
>+	 */
>+	if (v1_is_string) {
>+		int r = ldif_read_objectSid(ldb, mem_ctx, v1, &parsed_1);
>+		if (r == 0) {
>+			v1 = &parsed_1;
> 		}
>-		ret = ldb_comparison_binary(ldb, mem_ctx, &v, v2);
>-		talloc_free(v.data);
>-		return ret;
>-	} else if (!ldif_comparision_objectSid_isString(v1)
>-		   && ldif_comparision_objectSid_isString(v2)) {
>-		struct ldb_val v;
>-		int ret;
>-		if (ldif_read_objectSid(ldb, mem_ctx, v2, &v) != 0) {
>-			/* Perhaps not a string after all */
>-			return ldb_comparison_binary(ldb, mem_ctx, v1, v2);
>-		}
>-		ret = ldb_comparison_binary(ldb, mem_ctx, v1, &v);
>-		talloc_free(v.data);
>-		return ret;
> 	}
>-	return ldb_comparison_binary(ldb, mem_ctx, v1, v2);
>+	if (v2_is_string) {
>+		int r = ldif_read_objectSid(ldb, mem_ctx, v2, &parsed_2);
>+		if (r == 0) {
>+			v2 = &parsed_2;
>+		}
>+	}
>+
>+	ret = ldb_comparison_binary(ldb, mem_ctx, v1, v2);
>+
>+	if (v1_is_string) {
>+		TALLOC_FREE(parsed_1.data);
>+	}
>+	if (v2_is_string) {
>+		TALLOC_FREE(parsed_2.data);
>+	}
>+	return ret;
> }
> 
> /*
>-- 
>2.34.1
>
>
>From faf570ef202b63ed876b72071b80c97589170f34 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 11 Apr 2024 16:25:02 +1200
>Subject: [PATCH 54/65] ldb-samba:ldif_handlers: dn_link_comparison semi-sorts
> deleted objects
>
>We were always returning -1 for a deleted object, which works for an
>equality test, but not a relative comparison.
>
>This sorts deleted DNs toward the end of the list -- except when both
>DNs are deleted. What should happen there is yet to be determined.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit db963b1674ede357d4edba578e0e0372dcb2f287)
>---
> lib/ldb-samba/ldif_handlers.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
>diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
>index a3ea70c83b3..8c8370c0bac 100644
>--- a/lib/ldb-samba/ldif_handlers.c
>+++ b/lib/ldb-samba/ldif_handlers.c
>@@ -1152,13 +1152,15 @@ static int samba_ldb_dn_link_comparison(struct ldb_context *ldb, void *mem_ctx,
> 	struct ldb_dn *dn1 = NULL, *dn2 = NULL;
> 	int ret;
> 
>+	/*
>+	 * In a sort context, Deleted DNs get shifted to the end.
>+	 * They never match in an equality
>+	 */
> 	if (dsdb_dn_is_deleted_val(v1)) {
>-		/* If the DN is deleted, then we can't search for it */
>-		return -1;
>+		return 1;
> 	}
> 
> 	if (dsdb_dn_is_deleted_val(v2)) {
>-		/* If the DN is deleted, then we can't search for it */
> 		return -1;
> 	}
> 
>-- 
>2.34.1
>
>
>From b4e9cce21e13a94938239a903a81705ff4b5461c Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 11 Apr 2024 16:26:03 +1200
>Subject: [PATCH 55/65] ldb-samba:ldif_handlers: dn_link_comparison semi-sorts
> invalid DNs
>
>these tend to go to the end of the sorted array.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 11d5a809325369b48d14023adf109e418bb1c7af)
>---
> lib/ldb-samba/ldif_handlers.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
>index 8c8370c0bac..78d712dc305 100644
>--- a/lib/ldb-samba/ldif_handlers.c
>+++ b/lib/ldb-samba/ldif_handlers.c
>@@ -1165,7 +1165,9 @@ static int samba_ldb_dn_link_comparison(struct ldb_context *ldb, void *mem_ctx,
> 	}
> 
> 	dn1 = ldb_dn_from_ldb_val(mem_ctx, ldb, v1);
>-	if ( ! ldb_dn_validate(dn1)) return -1;
>+	if ( ! ldb_dn_validate(dn1)) {
>+		return 1;
>+	}
> 
> 	dn2 = ldb_dn_from_ldb_val(mem_ctx, ldb, v2);
> 	if ( ! ldb_dn_validate(dn2)) {
>-- 
>2.34.1
>
>
>From 2e6951467ccc7d48ee1a709f6831bca7f6317d88 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 11 Apr 2024 16:53:03 +1200
>Subject: [PATCH 56/65] ldb-samba:ldif_handlers: dn_link_comparison correctly
> sorts deleted objects
>
>This changes the behaviour of the DN syntax .comparison_fn when being
>used in a search, if the search key is a deleted DN.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 70356592563bf758dbe509413445b77bb0d7da14)
>---
> lib/ldb-samba/ldif_handlers.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
>diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
>index 78d712dc305..a1256a9fce7 100644
>--- a/lib/ldb-samba/ldif_handlers.c
>+++ b/lib/ldb-samba/ldif_handlers.c
>@@ -1157,10 +1157,17 @@ static int samba_ldb_dn_link_comparison(struct ldb_context *ldb, void *mem_ctx,
> 	 * They never match in an equality
> 	 */
> 	if (dsdb_dn_is_deleted_val(v1)) {
>-		return 1;
>-	}
>-
>-	if (dsdb_dn_is_deleted_val(v2)) {
>+		if (! dsdb_dn_is_deleted_val(v2)) {
>+			return 1;
>+		}
>+		/*
>+		 * They are both deleted!
>+		 *
>+		 * The soundest thing to do at this point is carry on
>+		 * and compare the DNs normally. This matches the
>+		 * behaviour of samba_dn_extended_match() below.
>+		 */
>+	} else if (dsdb_dn_is_deleted_val(v2)) {
> 		return -1;
> 	}
> 
>-- 
>2.34.1
>
>
>From 2a5591b2922e2b1005a86211002b785faf5f0bab Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 11 Apr 2024 16:59:50 +1200
>Subject: [PATCH 57/65] ldb-samba:ldif_handlers: dn_link_comparison leaks less
>
>dn1 and dn2 can be invalid but still occupying memory.
>(ldb_dn_validate(dn2) does contain a NULL check, but a lot more besides).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 341b8fb60e291ad598fafd7a09a75e9b249de07f)
>---
> lib/ldb-samba/ldif_handlers.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
>index a1256a9fce7..8daf2e576f7 100644
>--- a/lib/ldb-samba/ldif_handlers.c
>+++ b/lib/ldb-samba/ldif_handlers.c
>@@ -1173,12 +1173,14 @@ static int samba_ldb_dn_link_comparison(struct ldb_context *ldb, void *mem_ctx,
> 
> 	dn1 = ldb_dn_from_ldb_val(mem_ctx, ldb, v1);
> 	if ( ! ldb_dn_validate(dn1)) {
>+		TALLOC_FREE(dn1);
> 		return 1;
> 	}
> 
> 	dn2 = ldb_dn_from_ldb_val(mem_ctx, ldb, v2);
> 	if ( ! ldb_dn_validate(dn2)) {
>-		talloc_free(dn1);
>+		TALLOC_FREE(dn1);
>+		TALLOC_FREE(dn2);
> 		return -1;
> 	}
> 
>-- 
>2.34.1
>
>
>From 75a0183ee72559d8017d72757644be1b8f42accf Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 11 Apr 2024 18:08:54 +1200
>Subject: [PATCH 58/65] ldb-samba:ldif_handlers: dn_link_comparison: sort
> invalid DNs
>
>If both DNs are invalid, we can say they are equal.
>
>This means invalid or NULL DNs will sort to the end of the array,
>before deleted DNs:
>
>[ valid DNs, sorted | invalid/NULL DNs | deleted DNs, sorted ]
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 7280c8e53f463108fe3de443ce63572dde689a30)
>---
> lib/ldb-samba/ldif_handlers.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/lib/ldb-samba/ldif_handlers.c b/lib/ldb-samba/ldif_handlers.c
>index 8daf2e576f7..75dbf2170a0 100644
>--- a/lib/ldb-samba/ldif_handlers.c
>+++ b/lib/ldb-samba/ldif_handlers.c
>@@ -1172,12 +1172,18 @@ static int samba_ldb_dn_link_comparison(struct ldb_context *ldb, void *mem_ctx,
> 	}
> 
> 	dn1 = ldb_dn_from_ldb_val(mem_ctx, ldb, v1);
>+	dn2 = ldb_dn_from_ldb_val(mem_ctx, ldb, v2);
>+
> 	if ( ! ldb_dn_validate(dn1)) {
> 		TALLOC_FREE(dn1);
>+		if ( ! ldb_dn_validate(dn2)) {
>+			TALLOC_FREE(dn2);
>+			return 0;
>+		}
>+		TALLOC_FREE(dn2);
> 		return 1;
> 	}
> 
>-	dn2 = ldb_dn_from_ldb_val(mem_ctx, ldb, v2);
> 	if ( ! ldb_dn_validate(dn2)) {
> 		TALLOC_FREE(dn1);
> 		TALLOC_FREE(dn2);
>-- 
>2.34.1
>
>
>From 51d9430999e68ae98f8a2568afd4b6d084e62159 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Wed, 10 Apr 2024 10:54:41 +1200
>Subject: [PATCH 59/65] ldb:attrib_handlers: make ldb_comparison_Boolean more
> consistent
>
>This isn't supposed to be used for sorting, but it is hard to say it
>won't be, so we might as well make it sort properly.
>
>Following long-standing behaviour, we try to sort "FALSE" > "TRUE", by
>length, then switch to using strncasecmp().
>
>strncasecmp would sort the other way, so we swap the operands. This is
>to make e.g. "TRUE\0" sort the same as "TRUE".
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit a75c98ad688415aec8afc617a759ba90cfd9f23b)
>---
> lib/ldb/common/attrib_handlers.c | 29 +++++++++++++++++++++++++----
> 1 file changed, 25 insertions(+), 4 deletions(-)
>
>diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
>index 7e174366c53..2df2941c978 100644
>--- a/lib/ldb/common/attrib_handlers.c
>+++ b/lib/ldb/common/attrib_handlers.c
>@@ -281,15 +281,36 @@ static int ldb_canonicalise_Boolean(struct ldb_context *ldb, void *mem_ctx,
> }
> 
> /*
>-  compare two Booleans
>-*/
>+ * compare two Booleans.
>+ *
>+ * According to RFC4517 4.2.2, "the booleanMatch rule is an equality matching
>+ * rule", meaning it isn't used for ordering.
>+ *
>+ * However, it seems conceivable that Samba could be coerced into sorting on a
>+ * field with Boolean syntax, so we might as well have consistent behaviour in
>+ * that case.
>+ *
>+ * The most probably values are {"FALSE", 5} and {"TRUE", 4}. To save time we
>+ * compare first by length, which makes FALSE > TRUE. This is somewhat
>+ * contrary to convention, but is how Samba has worked forever.
>+ *
>+ * If somehow we are comparing incompletely normalised values where the length
>+ * is the same (for example {"false", 5} and {"TRUE\0", 5}), the length is the
>+ * same, and we fall back to a strncasecmp. In this case, since "FALSE" is
>+ * alphabetically lower, we swap the order, so that "TRUE\0" again comes
>+ * before "FALSE".
>+ *
>+ * ldb_canonicalise_Boolean (just above) gives us a clue as to what we might
>+ * expect to cope with by way of invalid values.
>+ */
> static int ldb_comparison_Boolean(struct ldb_context *ldb, void *mem_ctx,
> 			   const struct ldb_val *v1, const struct ldb_val *v2)
> {
> 	if (v1->length != v2->length) {
>-		return NUMERIC_CMP(v1->length, v2->length);
>+		return NUMERIC_CMP(v2->length, v1->length);
> 	}
>-	return strncasecmp((char *)v1->data, (char *)v2->data, v1->length);
>+	/* reversed, see long comment above */
>+	return strncasecmp((char *)v2->data, (char *)v1->data, v1->length);
> }
> 
> 
>-- 
>2.34.1
>
>
>From 4b300de4b32a5195b94b05b4710bab40aba5842f Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 26 Apr 2024 15:24:47 +1200
>Subject: [PATCH 60/65] ldb: avoid NULL deref in ldb_db_compare
>
>This also sorts NULLs after invalid DNs, which matches the comment
>above.
>
>CID 1596622.
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit af7654331fb6a2d9cc41cf5bdffa74c81ff4ffee)
>---
> lib/ldb/common/ldb_dn.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
>diff --git a/lib/ldb/common/ldb_dn.c b/lib/ldb/common/ldb_dn.c
>index 2674094d7f6..00015d114e4 100644
>--- a/lib/ldb/common/ldb_dn.c
>+++ b/lib/ldb/common/ldb_dn.c
>@@ -1145,13 +1145,23 @@ int ldb_dn_compare(struct ldb_dn *dn0, struct ldb_dn *dn1)
> 	 * | normal DNs, sorted | casefold failed DNs | invalid DNs | NULLs |
> 	 */
> 
>-	if (dn0 == dn1 || (dn0->invalid && dn1->invalid)) {
>+	if (dn0 == dn1) {
>+		/* this includes the both-NULL case */
> 		return 0;
> 	}
>-	if (dn0 == NULL || dn0->invalid) {
>+	if (dn0 == NULL) {
> 		return 1;
> 	}
>-	if (dn1 == NULL || dn1->invalid) {
>+	if (dn1 == NULL) {
>+		return -1;
>+	}
>+	if (dn0->invalid && dn1->invalid) {
>+		return 0;
>+	}
>+	if (dn0->invalid) {
>+		return 1;
>+	}
>+	if (dn1->invalid) {
> 		return -1;
> 	}
> 
>-- 
>2.34.1
>
>
>From 009130e6e523476931b1547696dabf21be511358 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 12 Apr 2024 18:11:47 +1200
>Subject: [PATCH 61/65] s4:dsdb:mod: repl_md: make message_sort transitive
>
>Before we had (with a TODO of regret):
>
>       if (!a1 || !a2) {
>               return strcasecmp(e1->name, e2->name);
>       }
>
>so, given {name:"A", id 2}, {name:"B", NO id}, {name:"C", id 1},
>
> A < B by name
> B < C by name
> A > C by id
>
>Now the sort order is always A > C > B.
>
>This sort could have caused mysterious crashes in repl_meta_data if
>the schema is out of sync.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5335f122fb551231a02a58f88f6a0aa23b5e02cb)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 14 +++++++++++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 175a02d3ba7..6892aa477b4 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -965,12 +965,20 @@ static int replmd_ldb_message_element_attid_sort(const struct ldb_message_elemen
> 	a2 = dsdb_attribute_by_lDAPDisplayName(schema, e2->name);
> 
> 	/*
>-	 * TODO: remove this check, we should rely on e1 and e2 having valid attribute names
>-	 *       in the schema
>+	 * If the elements do not have valid attribute names in the schema
>+	 * (which we would prefer to think can't happen), we need to sort them
>+	 * somehow. The current strategy is to put them at the end, sorted by
>+	 * attribute name.
> 	 */
>-	if (!a1 || !a2) {
>+	if (a1 == NULL && a2 == NULL) {
> 		return strcasecmp(e1->name, e2->name);
> 	}
>+	if (a1 == NULL) {
>+		return 1;
>+	}
>+	if (a2 == NULL) {
>+		return -1;
>+	}
> 	if (a1->attributeID_id == a2->attributeID_id) {
> 		return 0;
> 	}
>-- 
>2.34.1
>
>
>From 0d2f2a8e940cdcb5e8cf13bbe369275a5d0e9fd8 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 12 Apr 2024 20:28:04 +1200
>Subject: [PATCH 62/65] s4:dsdb:mod: repl_md: message sort uses NUMERIC_CMP()
>
>No change at all in the result, just saving lines and branches.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 827b0c39ed0497407bfcfc5683735a165b1b0f0a)
>---
> source4/dsdb/samdb/ldb_modules/repl_meta_data.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
>diff --git a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>index 6892aa477b4..d574ba060a3 100644
>--- a/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>+++ b/source4/dsdb/samdb/ldb_modules/repl_meta_data.c
>@@ -979,10 +979,7 @@ static int replmd_ldb_message_element_attid_sort(const struct ldb_message_elemen
> 	if (a2 == NULL) {
> 		return -1;
> 	}
>-	if (a1->attributeID_id == a2->attributeID_id) {
>-		return 0;
>-	}
>-	return a1->attributeID_id > a2->attributeID_id ? 1 : -1;
>+	return NUMERIC_CMP(a1->attributeID_id, a2->attributeID_id);
> }
> 
> static void replmd_ldb_message_sort(struct ldb_message *msg,
>-- 
>2.34.1
>
>
>From 8edc3f84a1159f8465547992c92811341c14f221 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Thu, 11 Apr 2024 13:21:25 +1200
>Subject: [PATCH 63/65] ldb:attrib_handlers: use NUMERIC_CMP in
> ldb_comparison_fold
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit f81b7c7eb206a447d799a25cc2da26304dc7567a)
>---
> lib/ldb/common/attrib_handlers.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
>index 2df2941c978..86e16e4447f 100644
>--- a/lib/ldb/common/attrib_handlers.c
>+++ b/lib/ldb/common/attrib_handlers.c
>@@ -420,7 +420,7 @@ utf8str:
> 		while (*u1 == ' ') u1++;
> 		while (*u2 == ' ') u2++;
> 	}
>-	ret = (int)(*u1 - *u2);
>+	ret = NUMERIC_CMP(*u1, *u2);
> 
> 	talloc_free(b1);
> 	talloc_free(b2);
>-- 
>2.34.1
>
>
>From f66ce0140eb6993da2d0b8c22d9c503c46150239 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Fri, 26 Apr 2024 15:58:44 +1200
>Subject: [PATCH 64/65] ldb:attrib_handlers: reduce non-transitive behaviour in
> ldb_comparison_fold
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>If two strings are invalid UTF-8, the string is first compared with
>memcmp(), which compares as unsigned char.
>
>If the strings are of different lengths and one is a substring of the
>other, the memcmp() returns 0 and a second comparison is made which
>assumes the next character in the shorter string is '\0' -- but this
>comparison was done using SIGNED chars (on most systems). That leads
>to non-transitive comparisons.
>
>Consider the strings {"a\xff", "a", "ab\xff"} under that system.
>
>   "a\xff"  < "a",      because (char)0xff == -1.
>
>   "ab\xff" > "a",     because  'b' == 98.
>
>   "ab\xff" < "a\xff", because memcmp("ab\xff", "a\xff", 2) avoiding the
>                       signed char tiebreaker.
>
>(Before c49c48afe09a1a78989628bbffd49dd3efc154dd, the final character
>might br arbitrarily cast into another character -- in latin-1, for
>example, the 0xff here would have been seen as 'Ã¿', which would be
>uppercased to 'Å¸', which is U+0178, which would be truncated to
>'\x78', a positive char.
>
>On the other hand e.g. 0xfe, 'Ã¾', would have mapped to 0xde, 'Ã',
>remaining negative).
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(backported from commit e2051eebd492a419f840280336eb242d0b4a26ac)
>[dbagnall@samba.org: master has a different toupper() function]
>---
> lib/ldb/common/attrib_handlers.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
>diff --git a/lib/ldb/common/attrib_handlers.c b/lib/ldb/common/attrib_handlers.c
>index 86e16e4447f..04090a2b02f 100644
>--- a/lib/ldb/common/attrib_handlers.c
>+++ b/lib/ldb/common/attrib_handlers.c
>@@ -388,17 +388,27 @@ utf8str:
> 	b2 = ldb_casefold(ldb, mem_ctx, s2, n2);
> 
> 	if (!b1 || !b2) {
>-		/* One of the strings was not UTF8, so we have no
>-		 * options but to do a binary compare */
>+		/*
>+		 * One of the strings was not UTF8, so we have no
>+		 * options but to do a binary compare.
>+		 */
> 		talloc_free(b1);
> 		talloc_free(b2);
> 		ret = memcmp(s1, s2, MIN(n1, n2));
> 		if (ret == 0) {
>-			if (n1 == n2) return 0;
>+			if (n1 == n2) {
>+				return 0;
>+			}
> 			if (n1 > n2) {
>-				return (int)toupper(s1[n2]);
>+				if (s1[n2] == '\0') {
>+					return 0;
>+				}
>+				return 1;
> 			} else {
>-				return -(int)toupper(s2[n1]);
>+				if (s2[n1] == '\0') {
>+					return 0;
>+				}
>+				return -1;
> 			}
> 		}
> 		return ret;
>-- 
>2.34.1
>
>
>From 6b957802a4976d14b3ad050bd4a1a94475385c20 Mon Sep 17 00:00:00 2001
>From: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Date: Tue, 21 May 2024 20:20:36 +1200
>Subject: [PATCH 65/65] s3:smbcacls: fix ace_compare
>
>We got this wrong in commit 31101a9fa1503be9d8137e42466f57d85136a156.
>
>In fact, we should probably not reorder the inherited ACLs, but that
>is for another patch series.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=15625
>
>Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
>Reviewed-by: Andrew Bartlett <abartlet@samba.org>
>(cherry picked from commit 5abeb724d74af2b861f2ee6bc27762bb5bf07bca)
>---
> source3/utils/smbcacls.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/source3/utils/smbcacls.c b/source3/utils/smbcacls.c
>index 30b25d02d0b..f7b4708d8ae 100644
>--- a/source3/utils/smbcacls.c
>+++ b/source3/utils/smbcacls.c
>@@ -553,7 +553,7 @@ static int ace_compare(struct security_ace *ace1, struct security_ace *ace2)
> 		return -1;
> 	if ((ace1->flags & SEC_ACE_FLAG_INHERITED_ACE) &&
> 			(ace2->flags & SEC_ACE_FLAG_INHERITED_ACE))
>-		return NUMERIC_CMP(ace1, ace2);
>+		return NUMERIC_CMP(ace2->type, ace1->type);
> 
> 	if (ace1->type != ace2->type) {
> 		/* note the reverse order */
>-- 
>2.34.1
>

Actions: View

Attachments on bug 15625: 18297 | 18298 | 18299 | 18323 | 18324 | 18325