Hi,
> +cleanup:
> + bpf_link__destroy(link1);
> + bpf_link__destroy(link2);
> + bpf_link__destroy(link3);
> + memcg_ops__detach(skel);
> + memcg_ops__destroy(skel);
>
> Can this crash if skel is NULL?
Yes, this is a valid bug in the selftest cleanup path.
If execution jumps to cleanup before memcg_ops__open_and_load()
succeeds, skel remains NULL. In that case, memcg_ops__detach(skel)
dereferences NULL through obj->skeleton in the generated detach helper,
as you pointed out.
This is also inconsistent with nearby tests in the same file that
already do if (skel) {
memcg_ops__detach(skel);
memcg_ops__destroy(skel);
}
The C repro, modeling the same control flow:
--8<--
// SPDX-License-Identifier: GPL-2.0
// PoC for cleanup-path NULL dereference in
test_memcg_ops_hierarchies().
#include <stdio.h>
struct bpf_object_skeleton {
int dummy;
};
struct memcg_ops {
struct bpf_object_skeleton *skeleton;
};
__attribute__((noinline))
static void bpf_object__detach_skeleton(struct bpf_object_skeleton *s)
{
(void)s;
}
/* Matches generated skeleton helper shape from review mail. */
static inline void memcg_ops__detach(struct memcg_ops *obj)
{
bpf_object__detach_skeleton(obj->skeleton);
}
static int setup_cgroup_environment_fail(void)
{
return -1;
}
int main(void)
{
int ret;
struct memcg_ops *skel = NULL;
fprintf(stderr, "[*] trigger cleanup with skel == NULL\n");
/* Simulate early failure before open_and_load() assigns skel. */
ret = setup_cgroup_environment_fail();
if (ret)
goto cleanup;
cleanup:
/* Same problematic call pattern as in the test cleanup block. */
memcg_ops__detach(skel);
return 0;
}
--8<--
Signed-off-by: XIAO WU <[email protected]>
Thanks,
xiao
