From fa084c4ae32b5a56046f5d000ade79ed0e71cc98 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 8 Jun 2026 07:18:19 +1000 Subject: [PATCH] xattrs: fix UBSan-detected undefined behavior Three pre-existing issues UBSan flags during the xattr tests: * xattr_lookup_hash(): the summed hashlittle2() values overflow the signed int64 accumulator (UB). Accumulate in uint64_t and convert back at return -- the key is only used for hash-table equality, so the value is unchanged. * rsync_xal_get(): for an empty list (count == 0) the loop init `rxa += count-1` forms `items - 1` on a NULL `items` (UB). Guard with `if (count)`. * rsync_xal_store(): `memcpy(dst, xalp->items, 0)` passes a NULL source for an empty list (UB). Guard with `if (xalp->count)`. --- xattrs.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/xattrs.c b/xattrs.c index 99795f24..0b971d54 100644 --- a/xattrs.c +++ b/xattrs.c @@ -295,8 +295,12 @@ static int rsync_xal_get(const char *fname, item_list *xalp) rxa = xalp->items; if (count > 1) qsort(rxa, count, sizeof (rsync_xa), rsync_xal_compare_names); - for (rxa += count-1; count; count--, rxa--) - rxa->num = count; + /* Guard count==0: rxa is then xalp->items (possibly NULL) and the + * "rxa += count-1" init would form NULL-1 (undefined). */ + if (count) { + for (rxa += count-1; count; count--, rxa--) + rxa->num = count; + } return 0; } @@ -381,17 +385,19 @@ static int64 xattr_lookup_hash(const item_list *xalp) { const rsync_xa *rxas = xalp->items; size_t i; - int64 key = hashlittle2(&xalp->count, sizeof xalp->count); + /* Accumulate unsigned: the summed hash values routinely overflow a + * signed int64 (UB), and we only care about the resulting bit pattern. */ + uint64_t key = (uint64_t)hashlittle2(&xalp->count, sizeof xalp->count); for (i = 0; i < xalp->count; i++) { - key += hashlittle2(rxas[i].name, rxas[i].name_len); + key += (uint64_t)hashlittle2(rxas[i].name, rxas[i].name_len); if (rxas[i].datum_len > MAX_FULL_DATUM) - key += hashlittle2(rxas[i].datum, xattr_sum_len); + key += (uint64_t)hashlittle2(rxas[i].datum, xattr_sum_len); else - key += hashlittle2(rxas[i].datum, rxas[i].datum_len); + key += (uint64_t)hashlittle2(rxas[i].datum, rxas[i].datum_len); } - return key; + return (int64)key; } static int find_matching_xattr(const item_list *xalp) @@ -460,7 +466,9 @@ static int rsync_xal_store(item_list *xalp) * entire initial-count, not just enough space for one new item. */ *new_list = empty_xa_list; (void)EXPAND_ITEM_LIST(&new_list->xa_items, rsync_xa, xalp->count); - memcpy(new_list->xa_items.items, xalp->items, xalp->count * sizeof (rsync_xa)); + /* xalp->items is NULL for an empty list; memcpy(dst, NULL, 0) is UB. */ + if (xalp->count) + memcpy(new_list->xa_items.items, xalp->items, xalp->count * sizeof (rsync_xa)); new_list->xa_items.count = xalp->count; xalp->count = 0;