uid-range: fix out-of-bounds write in uid_range_partition()

uid_range_partition() filled the grown entries[] buffer backwards in
place. The backward-fill invariant (the write cursor stays above the
read index) only holds when every source entry contributes at least
one partition; an entry with nr < size contributes zero, so the cursor
stalls while the read index keeps descending. A later multi-part
entry's writes then overwrite the still-live zero-part slot, the
corrupted slot is re-read as a one-part entry, and the next
range->entries[--t] underflows.

Add a forward compaction first pass that drops the zero-part entries
before the backward fill.

Follow-up for 025439faaa8c053fab9fd01fb5f45fb819408bc5

Co-Authored-by: Paul Meyer <katexochen0@gmail.com>

diff --git a/src/basic/uid-range.c b/src/basic/uid-range.c
index 62c7d7d928eb72ea54b7fc5ed8c77e97050a2dca..e9e8932f389c981868578b1c63db96770583fb6f 100644 (file)
--- a/src/basic/uid-range.c
+++ b/src/basic/uid-range.c
@@ -395,9 +395,18 @@ int uid_range_partition(UIDRange *range, uid_t size) {
         if (n_new_entries > range->n_entries && !GREEDY_REALLOC(range->entries, n_new_entries))
                 return -ENOMEM;
 
-        /* Work backwards to avoid overwriting entries we still need to read */
+        /* Compact in place: drop entries that contribute zero partitions (nr < size). This forward pass
+         * reads each entry once and only writes to lower-or-equal indices, so it cannot alias an unread
+         * source entry. */
+        size_t n_src = 0;
+        for (size_t i = 0; i < range->n_entries; i++)
+                if (range->entries[i].nr >= size)
+                        range->entries[n_src++] = range->entries[i];
+
+        /* Pre-compaction guarantees every surviving entry contributes at least one partition slot, so the
+         * write cursor t stays ahead of the read index. */
         size_t t = n_new_entries;
-        for (size_t i = range->n_entries; i > 0; i--) {
+        for (size_t i = n_src; i > 0; i--) {
                 UIDRangeEntry *e = range->entries + i - 1;
                 unsigned n_parts = e->nr / size;
 

diff --git a/src/test/test-uid-range.c b/src/test/test-uid-range.c
index 6eef98153ef5579e27d7c3d2d105d7ef5d0a75fd..fdcbd8896e80a9e1f38d1ba07135940444530527 100644 (file)
--- a/src/test/test-uid-range.c
+++ b/src/test/test-uid-range.c
@@ -318,6 +318,68 @@ TEST(uid_range_partition) {
 
         p = uid_range_free(p);
 
+        /* Small entry preceding a large entry: the small entry must be dropped and the large entry
+         * partitioned without the in-place backward-fill write cursor aliasing the still-live small entry
+         * slot. */
+        ASSERT_OK(uid_range_add_str(&p, "0-4"));
+        ASSERT_OK(uid_range_add_str(&p, "100-129"));
+        ASSERT_EQ(uid_range_entries(p), 2U);
+        ASSERT_OK(uid_range_partition(p, 10));
+        ASSERT_EQ(uid_range_entries(p), 3U);
+        ASSERT_EQ(p->entries[0].start, 100U);
+        ASSERT_EQ(p->entries[0].nr, 10U);
+        ASSERT_EQ(p->entries[1].start, 110U);
+        ASSERT_EQ(p->entries[1].nr, 10U);
+        ASSERT_EQ(p->entries[2].start, 120U);
+        ASSERT_EQ(p->entries[2].nr, 10U);
+
+        p = uid_range_free(p);
+
+        /* A too-small entry between two partitionable entries is dropped; the others still partition. */
+        ASSERT_OK(uid_range_add_str(&p, "0-4"));        /* nr=5  < size -> dropped */
+        ASSERT_OK(uid_range_add_str(&p, "50-69"));      /* nr=20       -> 2 parts */
+        ASSERT_OK(uid_range_add_str(&p, "200-204"));    /* nr=5  < size -> dropped */
+        ASSERT_OK(uid_range_add_str(&p, "1000-1029"));  /* nr=30       -> 3 parts */
+        ASSERT_EQ(uid_range_entries(p), 4U);
+        ASSERT_OK(uid_range_partition(p, 10));
+        ASSERT_EQ(uid_range_entries(p), 5U);
+        ASSERT_EQ(p->entries[0].start, 50U);
+        ASSERT_EQ(p->entries[0].nr, 10U);
+        ASSERT_EQ(p->entries[1].start, 60U);
+        ASSERT_EQ(p->entries[1].nr, 10U);
+        ASSERT_EQ(p->entries[2].start, 1000U);
+        ASSERT_EQ(p->entries[2].nr, 10U);
+        ASSERT_EQ(p->entries[3].start, 1010U);
+        ASSERT_EQ(p->entries[3].nr, 10U);
+        ASSERT_EQ(p->entries[4].start, 1020U);
+        ASSERT_EQ(p->entries[4].nr, 10U);
+
+        p = uid_range_free(p);
+
+        /* A too-small entry before a partitionable entry is dropped. */
+        ASSERT_OK(uid_range_add_str(&p, "0-4"));        /* nr=5  < size -> dropped */
+        ASSERT_OK(uid_range_add_str(&p, "100-119"));    /* nr=20       -> 2 parts */
+        ASSERT_OK(uid_range_partition(p, 10));
+        ASSERT_EQ(uid_range_entries(p), 2U);
+        ASSERT_EQ(p->entries[0].start, 100U);
+        ASSERT_EQ(p->entries[0].nr, 10U);
+        ASSERT_EQ(p->entries[1].start, 110U);
+        ASSERT_EQ(p->entries[1].nr, 10U);
+
+        p = uid_range_free(p);
+
+        /* A too-small entry after a partitionable entry is dropped. */
+        ASSERT_OK(uid_range_add_str(&p, "0-199"));      /* nr=200 -> 2 parts */
+        ASSERT_OK(uid_range_add_str(&p, "1000-1004"));  /* nr=5  < size -> dropped */
+        ASSERT_OK(uid_range_partition(p, 100));
+        ASSERT_EQ(uid_range_entries(p), 2U);
+        ASSERT_EQ(p->entries[0].start, 0U);
+        ASSERT_EQ(p->entries[0].nr, 100U);
+        ASSERT_EQ(p->entries[1].start, 100U);
+        ASSERT_EQ(p->entries[1].nr, 100U);
+
+        p = uid_range_free(p);
+
         /* Partition size of 1 */
         ASSERT_OK(uid_range_add_str(&p, "100-102"));
         ASSERT_OK(uid_range_partition(p, 1));