The "malloc" call may not be successful.Add the malloc
failure checking to avoid possible null dereference.
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
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
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
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
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/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..302b25408a53 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 (CHECK(!query, "malloc()", "error:%s\n", strerror(errno)))
+ 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
…
> This patch will add the malloc failure checking
…
* Please use a corresponding imperative wording for the change description.
* Would you like to add the tag “Fixes” accordingly?
…
> +++ 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;
> + }
…
How do you think about to reuse “errno” in such error cases?
Regards,
Markus
…
> This patch will add the malloc failure checking
…
* Please use a corresponding imperative wording for the change description.
* Would you like to add the tag “Fixes” accordingly?
Regards,
Markus
…
> Add the malloc failure checking to avoid possible null
> dereference.
…
How do you think about the following wording variant?
Add a return value check so that a null pointer dereference will be avoided
after a memory allocation failure.
Would you like to add the tag “Fixes” accordingly?
…
> +++ 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;
…
How do you think about to reuse the variable “errno” in such an error case?
Regards,
Markus
…
> Add the malloc failure checking to avoid possible null
> dereference.
…
How do you think about to use the following wording variant?
Add a return value check so that a null pointer dereference will be avoided
after a memory allocation failure.
Would you like to add the tag “Fixes” accordingly?
Regards,
Markus
On 4/24/24 4:04 AM, Kunwu Chan wrote:
> 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/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..302b25408a53 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 (CHECK(!query, "malloc()", "error:%s\n", strerror(errno)))
Series looks reasonable, small nit on CHECK() : Lets use ASSERT*() macros given they are
preferred over the latter :
if (!ASSERT_OK_PTR(buf, "malloc"))
> + return;
> +
> for (i = 0; i < num_progs; i++) {
> err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj[i],
> &prog_fd[i]);
>
On 5/3/24 5:47 PM, Daniel Borkmann wrote:
> On 4/24/24 4:04 AM, Kunwu Chan wrote:
>> 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/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..302b25408a53 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 (CHECK(!query, "malloc()", "error:%s\n", strerror(errno)))
>
> Series looks reasonable, small nit on CHECK() : Lets use ASSERT*() macros given they are
> preferred over the latter :
>
> if (!ASSERT_OK_PTR(buf, "malloc"))
( Also as a side-note: Fixes tag on all these patches is not needed given this will just
end up spamming stable tree. If you indeed end up with NULL then the tests will just
segfault & fail. )
>> + return;
>> +
>> for (i = 0; i < num_progs; i++) {
>> err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj[i],
>> &prog_fd[i]);
>>
>