| Message ID | 20240510095803.472840-1-kunwu.chan@linux.dev |
|---|---|
| Headers | show |
| Series | Add some 'malloc' failure checks | expand |
On 5/10/24 2:58 PM, kunwu.chan@linux.dev wrote: > From: Kunwu Chan <chentao@kylinos.cn> > > There is a 'malloc' call, which can be unsuccessful. > This patch will add the malloc failure checking > to avoid possible null dereference. > > Signed-off-by: Kunwu Chan <chentao@kylinos.cn> > --- > tools/testing/selftests/bpf/test_progs.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c > index 89ff704e9dad..ecc3ddeceeeb 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -582,6 +582,11 @@ int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len) > > val_buf1 = malloc(stack_trace_len); > val_buf2 = malloc(stack_trace_len); > + if (!val_buf1 || !val_buf2) { > + err = -ENOMEM; Return from here instead of going to out where free(val_buf*) is being called. > + goto out; > + } > + > cur_key_p = NULL; > next_key_p = &key; > while (bpf_map_get_next_key(smap_fd, cur_key_p, next_key_p) == 0) { > @@ -1197,6 +1202,8 @@ static int dispatch_thread_send_subtests(int sock_fd, struct test_state *state) > int subtest_num = state->subtest_num; > > state->subtest_states = malloc(subtest_num * sizeof(*subtest_state)); > + if (!state->subtest_states) > + return -ENOMEM; > > for (int i = 0; i < subtest_num; i++) { > subtest_state = &state->subtest_states[i];
On 5/10/24 2:58 PM, kunwu.chan@linux.dev wrote: > From: Kunwu Chan <chentao@kylinos.cn> > > There is a 'malloc' call, which can be unsuccessful. > Add the malloc failure checking to avoid possible null > dereference. > > Signed-off-by: Kunwu Chan <chentao@kylinos.cn> LGTM Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com> > --- > tools/testing/selftests/bpf/test_verifier.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c > index df04bda1c927..9c80b2943418 100644 > --- a/tools/testing/selftests/bpf/test_verifier.c > +++ b/tools/testing/selftests/bpf/test_verifier.c > @@ -762,6 +762,8 @@ static int load_btf_spec(__u32 *types, int types_len, > ); > > raw_btf = malloc(sizeof(hdr) + types_len + strings_len); > + if (!raw_btf) > + return -ENOMEM; > > ptr = raw_btf; > memcpy(ptr, &hdr, sizeof(hdr));
On 2024/5/10 19:20, Muhammad Usama Anjum wrote: > On 5/10/24 2:58 PM, kunwu.chan@linux.dev wrote: >> From: Kunwu Chan <chentao@kylinos.cn> >> >> There is a 'malloc' call, which can be unsuccessful. >> This patch will add the malloc failure checking >> to avoid possible null dereference. >> >> Signed-off-by: Kunwu Chan <chentao@kylinos.cn> >> --- >> tools/testing/selftests/bpf/test_progs.c | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c >> index 89ff704e9dad..ecc3ddeceeeb 100644 >> --- a/tools/testing/selftests/bpf/test_progs.c >> +++ b/tools/testing/selftests/bpf/test_progs.c >> @@ -582,6 +582,11 @@ int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len) >> >> val_buf1 = malloc(stack_trace_len); >> val_buf2 = malloc(stack_trace_len); >> + if (!val_buf1 || !val_buf2) { >> + err = -ENOMEM; > Return from here instead of going to out where free(val_buf*) is being called. I think it's no harm. And Unify the processing at the end to achieve uniform format. >> + goto out; >> + } >> + >> cur_key_p = NULL; >> next_key_p = &key; >> while (bpf_map_get_next_key(smap_fd, cur_key_p, next_key_p) == 0) { >> @@ -1197,6 +1202,8 @@ static int dispatch_thread_send_subtests(int sock_fd, struct test_state *state) >> int subtest_num = state->subtest_num; >> >> state->subtest_states = malloc(subtest_num * sizeof(*subtest_state)); >> + if (!state->subtest_states) >> + return -ENOMEM; >> >> for (int i = 0; i < subtest_num; i++) { >> subtest_state = &state->subtest_states[i];
From: Kunwu Chan <chentao@kylinos.cn> The "malloc" call may not be successful.Add the malloc failure checking to avoid possible null dereference. Changes since v1 [1]: - As Daniel Borkmann suggested, change patch 4/4 only - Other 3 patches no changes. [1] https://lore.kernel.org/all/20240424020444.2375773-1-chentao@kylinos.cn/ Kunwu Chan (4): selftests/bpf: Add some null pointer checks selftests/bpf/sockopt: Add a null pointer check for the run_test selftests/bpf: Add a null pointer check for the load_btf_spec selftests/bpf: Add a null pointer check for the serial_test_tp_attach_query tools/testing/selftests/bpf/prog_tests/sockopt.c | 6 ++++++ tools/testing/selftests/bpf/prog_tests/tp_attach_query.c | 3 +++ tools/testing/selftests/bpf/test_progs.c | 7 +++++++ tools/testing/selftests/bpf/test_verifier.c | 2 ++ 4 files changed, 18 insertions(+)