| Message ID | 20240510095803.472840-2-kunwu.chan@linux.dev (mailing list archive) |
|---|---|
| State | New |
| 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 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];
> There is a 'malloc' call, which can be unsuccessful. two calls? > This patch will add the malloc failure checking … Please use imperative wordings for improved change descriptions also in your patches. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.9#n94 Regards, Markus
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; + 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];