system/pipewire
system/pipewire
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/pipewire/pipewire synced 2024-10-06 16:09:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

map: fix free_list corruption when re-using removed ids

Browse source 
Re-using an id after removing it is a bug in the caller but there are
two cases where we corrupt the free list without warning:

Removing an object twice:

   id = pw_map_insert_new(object);
   pw_map_remove(map, id);
   pw_map_remove(map, id);

And inserting an element at an index previously removed:

   id = pw_map_insert_new(object);
   pw_map_remove(map, id);
   pw_map_insert_at(map, id, new_object);

The latter is arguably valid code, or at least it'll look like it's
valid code.

For both cases, check if the id to remove/insert at is a free item and
handle that accordingly.
This commit is contained in:
Peter Hutterer 2021-09-30 11:58:49 +10:00
parent 5b9447c2a4
commit 626d30e4bd
2 changed files with 109 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

19
src/pipewire/map.h
View file

@ -142,8 +142,20 @@ static inline int pw_map_insert_at(struct pw_map *map, uint32_t id, void *data)
item = (union pw_map_item *) pw_array_add(&map->items, sizeof(union pw_map_item));
if (item == NULL)
return -errno;
}
else {
} else {
if (pw_map_id_is_free(map, id)) {
uint32_t *current = &map->free_list;
while (*current != SPA_ID_INVALID) {
uint32_t current_id = (*current) >> 1;
uint32_t *next = &pw_map_get_item(map, current_id)->next;
if (current_id == id) {
*current = *next;
break;
}
current = next;
}
}
item = pw_map_get_item(map, id);
}
item->data = data;
@ -156,6 +168,9 @@ static inline int pw_map_insert_at(struct pw_map *map, uint32_t id, void *data)
*/
static inline void pw_map_remove(struct pw_map *map, uint32_t id)
{
if (pw_map_id_is_free(map, id))
return;
pw_map_get_item(map, id)->next = map->free_list;
map->free_list = (id << 1) | 1;
}

92
test/test-map.c
View file

@ -155,11 +155,103 @@ PWTEST(map_size)
return PWTEST_PASS;
}
PWTEST(map_double_remove)
{
struct pw_map map = PW_MAP_INIT(2);
int a, b, c;
void *p1 = &a, *p2 = &b, *p3 = &c;
uint32_t idx1, idx2, idx3;
idx1 = pw_map_insert_new(&map, p1);
idx2 = pw_map_insert_new(&map, p2);
idx3 = pw_map_insert_new(&map, p3);
pw_map_remove(&map, idx1); /* idx1 in the free list */
pw_map_remove(&map, idx2); /* idx1 and 2 in the free list */
pw_map_remove(&map, idx2); /* should be a noop */
idx1 = pw_map_insert_new(&map, p1);
idx2 = pw_map_insert_new(&map, p2);
pwtest_ptr_eq(p1, pw_map_lookup(&map, idx1));
pwtest_ptr_eq(p2, pw_map_lookup(&map, idx2));
pwtest_ptr_eq(p3, pw_map_lookup(&map, idx3));
pw_map_clear(&map);
return PWTEST_PASS;
}
PWTEST(map_insert_at_free)
{
struct pw_map map = PW_MAP_INIT(2);
int data[3] = {1, 2, 3};
int new_data = 4;
int *ptr[3] = {&data[0], &data[1], &data[3]};
int *new_ptr = &new_data;
int idx[3];
int rc;
/* Test cases, for an item at idx:
* 1. remove at idx, then reinsert
* 2. remove at idx, remove another item, reinsert
* 3. remove another item, remove at idx, reinsert
* 4. remove another item, remove at index, remove another item, reinsert
*
* The indices are the respective 2 bits from the iteration counter,
* we use index 3 to indicate skipping that step to handle cases 1-3.
*/
int iteration = pwtest_get_iteration(current_test);
int item_idx = iteration & 0x3;
int before_idx = (iteration >> 2) & 0x3;
int after_idx = (iteration >> 4) & 0x3;
const int SKIP = 3;
if (item_idx == SKIP)
return PWTEST_PASS;
idx[0] = pw_map_insert_new(&map, ptr[0]);
idx[1] = pw_map_insert_new(&map, ptr[1]);
idx[2] = pw_map_insert_new(&map, ptr[2]);
if (before_idx != SKIP) {
before_idx = idx[before_idx];
pw_map_remove(&map, before_idx);
}
pw_map_remove(&map, item_idx);
if (after_idx != SKIP) {
after_idx = idx[after_idx];
pw_map_remove(&map, after_idx);
}
rc = pw_map_insert_at(&map, item_idx, &new_data);
pwtest_neg_errno_ok(rc);
pwtest_ptr_eq(new_ptr, pw_map_lookup(&map, item_idx));
if (before_idx != SKIP && before_idx != item_idx) {
rc = pw_map_insert_at(&map, before_idx, &ptr[before_idx]);
pwtest_neg_errno_ok(rc);
pwtest_ptr_eq(&ptr[before_idx], pw_map_lookup(&map, before_idx));
}
if (after_idx != SKIP && after_idx != item_idx) {
rc = pw_map_insert_at(&map, after_idx, &ptr[after_idx]);
pwtest_neg_errno_ok(rc);
pwtest_ptr_eq(&ptr[after_idx], pw_map_lookup(&map, after_idx));
}
pw_map_clear(&map);
return PWTEST_PASS;
}
PWTEST_SUITE(pw_map)
{
pwtest_add(map_add_remove, PWTEST_NOARG);
pwtest_add(map_insert, PWTEST_NOARG);
pwtest_add(map_size, PWTEST_NOARG);
pwtest_add(map_double_remove, PWTEST_NOARG);
pwtest_add(map_insert_at_free, PWTEST_ARG_RANGE, 0, 63);
return PWTEST_PASS;
}