[v10] Fix the null pointer dereference issue in bpf_lwt_xmit_push_encap

[PATCH v10 bpf-next 2/2] selftests/bpf: Add selftests for the invocation of bpf_lwt_xmit_push_encap

Posted by Feng Yang 4 weeks, 1 day ago

From: Feng Yang <yangfeng@kylinos.cn>

Calling bpf_lwt_xmit_push_encap will not cause a crash when dst is missing.

Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
---
 .../selftests/bpf/prog_tests/lwt_misc.c       |  9 ++++++++
 tools/testing/selftests/bpf/progs/lwt_misc.c  | 22 +++++++++++++++++++
 2 files changed, 31 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_misc.c
 create mode 100644 tools/testing/selftests/bpf/progs/lwt_misc.c

diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_misc.c b/tools/testing/selftests/bpf/prog_tests/lwt_misc.c
new file mode 100644
index 000000000000..6940fca38512
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/lwt_misc.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <test_progs.h>
+#include "lwt_misc.skel.h"
+
+void test_lwt_misc(void)
+{
+	RUN_TESTS(lwt_misc);
+}
diff --git a/tools/testing/selftests/bpf/progs/lwt_misc.c b/tools/testing/selftests/bpf/progs/lwt_misc.c
new file mode 100644
index 000000000000..aa638a4f2922
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/lwt_misc.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include "bpf_misc.h"
+
+SEC("lwt_xmit")
+__success __retval(0)
+int test_missing_dst_call_bpf_lwt_xmit_push_encap(struct __sk_buff *skb)
+{
+	struct iphdr iph;
+
+	__builtin_memset(&iph, 0, sizeof(struct iphdr));
+	iph.ihl = 5;
+	iph.version = 4;
+
+	bpf_lwt_push_encap(skb, BPF_LWT_ENCAP_IP, &iph, sizeof(struct iphdr));
+
+	return 0;
+}
+
+char _license[] SEC("license") = "GPL";
-- 
2.43.0

Re: [PATCH v10 bpf-next 2/2] selftests/bpf: Add selftests for the invocation of bpf_lwt_xmit_push_encap

Posted by Leon Hwang 4 weeks, 1 day ago

On 4/3/26 17:44, Feng Yang wrote:
> From: Feng Yang <yangfeng@kylinos.cn>
> 
> Calling bpf_lwt_xmit_push_encap will not cause a crash when dst is missing.
> 
> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
> ---
>  .../selftests/bpf/prog_tests/lwt_misc.c       |  9 ++++++++
>  tools/testing/selftests/bpf/progs/lwt_misc.c  | 22 +++++++++++++++++++
>  2 files changed, 31 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_misc.c
>  create mode 100644 tools/testing/selftests/bpf/progs/lwt_misc.c
> 

Unnecessary to create new files.

Move the adding test to verifier_lwt.c. prog_tests/verifier.c will run
the test.

Thanks,
Leon

> diff --git a/tools/testing/selftests/bpf/prog_tests/lwt_misc.c b/tools/testing/selftests/bpf/prog_tests/lwt_misc.c
> new file mode 100644
> index 000000000000..6940fca38512
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/lwt_misc.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <test_progs.h>
> +#include "lwt_misc.skel.h"
> +
> +void test_lwt_misc(void)
> +{
> +	RUN_TESTS(lwt_misc);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/lwt_misc.c b/tools/testing/selftests/bpf/progs/lwt_misc.c
> new file mode 100644
> index 000000000000..aa638a4f2922
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/lwt_misc.c
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include "bpf_misc.h"
> +
> +SEC("lwt_xmit")
> +__success __retval(0)
> +int test_missing_dst_call_bpf_lwt_xmit_push_encap(struct __sk_buff *skb)
> +{
> +	struct iphdr iph;
> +
> +	__builtin_memset(&iph, 0, sizeof(struct iphdr));
> +	iph.ihl = 5;
> +	iph.version = 4;
> +
> +	bpf_lwt_push_encap(skb, BPF_LWT_ENCAP_IP, &iph, sizeof(struct iphdr));
> +
> +	return 0;
> +}
> +
> +char _license[] SEC("license") = "GPL";

Re: [PATCH v10 bpf-next 2/2] selftests/bpf: Add selftests for the invocation of bpf_lwt_xmit_push_encap

Posted by Martin KaFai Lau 4 weeks ago

On 3/4/26 2:33 AM, Leon Hwang wrote:
> On 4/3/26 17:44, Feng Yang wrote:
>> From: Feng Yang <yangfeng@kylinos.cn>
>>
>> Calling bpf_lwt_xmit_push_encap will not cause a crash when dst is missing.
>>
>> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
>> ---
>>   .../selftests/bpf/prog_tests/lwt_misc.c       |  9 ++++++++
>>   tools/testing/selftests/bpf/progs/lwt_misc.c  | 22 +++++++++++++++++++
>>   2 files changed, 31 insertions(+)
>>   create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_misc.c
>>   create mode 100644 tools/testing/selftests/bpf/progs/lwt_misc.c
>>
> 
> Unnecessary to create new files.
> 
> Move the adding test to verifier_lwt.c. prog_tests/verifier.c will run
> the test.

It is actually the change from v9 to v10. I think leaving it separate is 
better. It is not testing the verifier, so logically it does not fit in 
verifier_lwt.c. It probably won't be the last fix for test_run+lwt, so 
separating it should be useful going forward.

Re: [PATCH v10 bpf-next 2/2] selftests/bpf: Add selftests for the invocation of bpf_lwt_xmit_push_encap

Posted by Leon Hwang 4 weeks ago

On 5/3/26 09:11, Martin KaFai Lau wrote:
> On 3/4/26 2:33 AM, Leon Hwang wrote:
>> On 4/3/26 17:44, Feng Yang wrote:
>>> From: Feng Yang <yangfeng@kylinos.cn>
>>>
>>> Calling bpf_lwt_xmit_push_encap will not cause a crash when dst is
>>> missing.
>>>
>>> Signed-off-by: Feng Yang <yangfeng@kylinos.cn>
>>> ---
>>>   .../selftests/bpf/prog_tests/lwt_misc.c       |  9 ++++++++
>>>   tools/testing/selftests/bpf/progs/lwt_misc.c  | 22 +++++++++++++++++++
>>>   2 files changed, 31 insertions(+)
>>>   create mode 100644 tools/testing/selftests/bpf/prog_tests/lwt_misc.c
>>>   create mode 100644 tools/testing/selftests/bpf/progs/lwt_misc.c
>>>
>>
>> Unnecessary to create new files.
>>
>> Move the adding test to verifier_lwt.c. prog_tests/verifier.c will run
>> the test.
> 
> It is actually the change from v9 to v10. I think leaving it separate is

Got it.

> better. It is not testing the verifier, so logically it does not fit in
> verifier_lwt.c. It probably won't be the last fix for test_run+lwt, so
> separating it should be useful going forward.
> 

I realized it after sending the reply. Thanks for the explanation.

Thanks,
Leon