shadow-maint / shadow

Upstream shadow tree
Other
287 stars 227 forks source link

libsubid: Memory Management in Different Heap States #1018

Closed daskol closed 3 weeks ago

daskol commented 1 month ago

As in release shadow 4.15.1, libsubid assumes that the same allocator (presumably glibc) is used across caller and callee code. The issue is getting worse in a situation when libsubid forward subordinate management to a plugin according to nsswitch.conf. In this case, memory management gets more complicated: one must be sure that a plugin and a caller use the same allocator which shares the same heap state. The issue is that a caller can use glibc malloc while a plugin uses jemalloc/tcmalloc/etc or none of them at all if a plugin is not written in C/C++.

I propose two ways to tackle the issue. In the first solution, libsubid public API should be extended with subid_free routine to release memory allocated in a plugin. The advantage of this solution is its simplicity and it requires only small changes (writing new code, mostly).

struct subid_range *ranges;
int count = subid_get_uid_ranges("username", &ranges);
// do something with ranges.
subid_free(ranges);

The second approach is wide-spread as well. It requires API call to estimate buffer size first and allocating a buffer on a caller side.

int count = subid_get_uid_ranges_count("username");
if (count < 0) {
    throw std::runtime_exception(...);
}
auto ranges = std::make_unique<subid_range[]>(count);
subid_get_uid_ranges("username", ranges.get());
// do something with ranges.

What do you think?

daskol commented 1 month ago

@hallyn @alejandro-colomar Any comments?

alejandro-colomar commented 1 month ago

I propose two ways to tackle the issue. In the first solution, libsubid public API should be extended with subid_free routine to release memory allocated in a plugin. The advantage of this solution is its simplicity and it requires only small changes (writing new code, mostly).

struct subid_range *ranges;
int count = subid_get_uid_ranges("username", &ranges);
// do something with ranges.
subid_free(ranges);

The second approach is wide-spread as well. It requires API call to estimate buffer size first and allocating a buffer on a caller side.

int count = subid_get_uid_ranges_count("username");
if (count < 0) {
    throw std::runtime_exception(...);
}
auto ranges = std::make_unique<subid_range[]>(count);
subid_get_uid_ranges("username", ranges.get());
// do something with ranges.

What do you think?

Both sound good to me.

daskol commented 1 month ago

Great. I will prepare pull request then.

hallyn commented 1 month ago

I prefer the first.

alejandro-colomar commented 3 weeks ago

I guess we can close this. If you need anything else, feel free to reopen.