mirror of
https://github.com/RsyncProject/rsync.git
synced 2026-06-08 14:15:46 -04:00
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)`.
This commit is contained in:
24
xattrs.c
24
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;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user