From: Kunwu Chan <[email protected]>
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/[email protected]/
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(+)
--
2.40.1
From: Kunwu Chan <[email protected]>
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 <[email protected]>
---
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;
+ 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];
--
2.40.1
From: Kunwu Chan <[email protected]>
There is a 'malloc' call, which can be unsuccessful.
Add the malloc failure checking to avoid possible null
dereference and give more information
about test fail reasons.
Signed-off-by: Kunwu Chan <[email protected]>
---
tools/testing/selftests/bpf/prog_tests/sockopt.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt.c b/tools/testing/selftests/bpf/prog_tests/sockopt.c
index 5a4491d4edfe..bde63e1f74dc 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockopt.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockopt.c
@@ -1100,6 +1100,12 @@ static int run_test(int cgroup_fd, struct sockopt_test *test, bool use_io_uring)
}
optval = malloc(test->get_optlen);
+ if (!optval) {
+ log_err("Failed to allocate memory");
+ ret = -1;
+ goto close_sock_fd;
+ }
+
memset(optval, 0, test->get_optlen);
socklen_t optlen = test->get_optlen;
socklen_t expected_get_optlen = test->get_optlen_ret ?:
--
2.40.1
From: Kunwu Chan <[email protected]>
There is a 'malloc' call, which can be unsuccessful.
Add the malloc failure checking to avoid possible null
dereference.
Signed-off-by: Kunwu Chan <[email protected]>
---
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));
--
2.40.1
From: Kunwu Chan <[email protected]>
There is a 'malloc' call, which can be unsuccessful.
Add the malloc failure checking to avoid possible null
dereference.
Signed-off-by: Kunwu Chan <[email protected]>
Suggested-by: Daniel Borkmann <[email protected]>
---
Changes in v2:
- Use ASSERT* instead of CHECK
- Add suggested-by tag
---
tools/testing/selftests/bpf/prog_tests/tp_attach_query.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
index 655d69f0ff0b..a5ebfc172ad8 100644
--- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
+++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
@@ -39,6 +39,9 @@ void serial_test_tp_attach_query(void)
attr.wakeup_events = 1;
query = malloc(sizeof(*query) + sizeof(__u32) * num_progs);
+ if (!ASSERT_OK_PTR(query, "malloc"))
+ return;
+
for (i = 0; i < num_progs; i++) {
err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj[i],
&prog_fd[i]);
--
2.40.1
On 5/10/24 2:58 PM, [email protected] wrote:
> From: Kunwu Chan <[email protected]>
>
> 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 <[email protected]>
> ---
> 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];
--
BR,
Muhammad Usama Anjum
On 5/10/24 2:58 PM, [email protected] wrote:
> From: Kunwu Chan <[email protected]>
>
> There is a 'malloc' call, which can be unsuccessful.
> Add the malloc failure checking to avoid possible null
> dereference and give more information
> about test fail reasons.
>
> Signed-off-by: Kunwu Chan <[email protected]>
LGTM
Reviewed-by: Muhammad Usama Anjum <[email protected]>
> ---
> tools/testing/selftests/bpf/prog_tests/sockopt.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt.c b/tools/testing/selftests/bpf/prog_tests/sockopt.c
> index 5a4491d4edfe..bde63e1f74dc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt.c
> @@ -1100,6 +1100,12 @@ static int run_test(int cgroup_fd, struct sockopt_test *test, bool use_io_uring)
> }
>
> optval = malloc(test->get_optlen);
> + if (!optval) {
> + log_err("Failed to allocate memory");
> + ret = -1;
> + goto close_sock_fd;
> + }
> +
> memset(optval, 0, test->get_optlen);
> socklen_t optlen = test->get_optlen;
> socklen_t expected_get_optlen = test->get_optlen_ret ?:
--
BR,
Muhammad Usama Anjum
On 5/10/24 2:58 PM, [email protected] wrote:
> From: Kunwu Chan <[email protected]>
>
> There is a 'malloc' call, which can be unsuccessful.
> Add the malloc failure checking to avoid possible null
> dereference.
>
> Signed-off-by: Kunwu Chan <[email protected]>
LGTM
Reviewed-by: Muhammad Usama Anjum <[email protected]>
> ---
> 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));
--
BR,
Muhammad Usama Anjum
On 5/10/24 2:58 PM, [email protected] wrote:
> From: Kunwu Chan <[email protected]>
>
> There is a 'malloc' call, which can be unsuccessful.
> Add the malloc failure checking to avoid possible null
> dereference.
>
> Signed-off-by: Kunwu Chan <[email protected]>
> Suggested-by: Daniel Borkmann <[email protected]>
> ---
> Changes in v2:
> - Use ASSERT* instead of CHECK
> - Add suggested-by tag
> ---
> tools/testing/selftests/bpf/prog_tests/tp_attach_query.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> index 655d69f0ff0b..a5ebfc172ad8 100644
> --- a/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> +++ b/tools/testing/selftests/bpf/prog_tests/tp_attach_query.c
> @@ -39,6 +39,9 @@ void serial_test_tp_attach_query(void)
> attr.wakeup_events = 1;
>
> query = malloc(sizeof(*query) + sizeof(__u32) * num_progs);
> + if (!ASSERT_OK_PTR(query, "malloc"))
> + return;
> +
LGTM
Reviewed-by: Muhammad Usama Anjum <[email protected]>
> for (i = 0; i < num_progs; i++) {
> err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj[i],
> &prog_fd[i]);
--
BR,
Muhammad Usama Anjum
On 2024/5/10 19:20, Muhammad Usama Anjum wrote:
> On 5/10/24 2:58 PM, [email protected] wrote:
>> From: Kunwu Chan <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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];
--
Thanks,
Kunwu.Chan
> 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