selftests/bpf: Fix null pointer check in skb_pkt_end.c

[PATCH] selftests/bpf: Fix null pointer check in skb_pkt_end.c

Posted by Prabhav Kumar Vaish 9 months, 3 weeks ago

Ensure that 'tcp' is checked for NULL before dereferencing. This resolves
a potential null pointer dereference warning reported by static analysis.

Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com>
---
 tools/testing/selftests/bpf/progs/skb_pkt_end.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/skb_pkt_end.c b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
index 3bb4451524a1..db33ff2839f7 100644
--- a/tools/testing/selftests/bpf/progs/skb_pkt_end.c
+++ b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
@@ -45,10 +45,10 @@ int main_prog(struct __sk_buff *skb)
 		goto out;
 
 	tcp = (void*)(ip + 1);
-	if (tcp->dest != 0)
-		goto out;
 	if (!tcp)
 		goto out;
+	if (tcp->dest != 0)
+		goto out;
 
 	urg_ptr = tcp->urg_ptr;
 
-- 
2.34.1

Re: [PATCH] selftests/bpf: Fix null pointer check in skb_pkt_end.c

Posted by Martin KaFai Lau 9 months, 2 weeks ago

On 4/22/25 11:23 AM, Prabhav Kumar Vaish wrote:
> Ensure that 'tcp' is checked for NULL before dereferencing. This resolves
> a potential null pointer dereference warning reported by static analysis.
> 
> Signed-off-by: Prabhav Kumar Vaish <pvkumar5749404@gmail.com>
> ---
>   tools/testing/selftests/bpf/progs/skb_pkt_end.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/progs/skb_pkt_end.c b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
> index 3bb4451524a1..db33ff2839f7 100644
> --- a/tools/testing/selftests/bpf/progs/skb_pkt_end.c
> +++ b/tools/testing/selftests/bpf/progs/skb_pkt_end.c
> @@ -45,10 +45,10 @@ int main_prog(struct __sk_buff *skb)
>   		goto out;
>   
>   	tcp = (void*)(ip + 1);
> -	if (tcp->dest != 0)
> -		goto out;
>   	if (!tcp)

This case will never be hit, so this change is not doing anything other than 
silencing the static checker. Take a look at commit 9cc873e85800 
("selftests/bpf: Add skb_pkt_end test"). The test was written to have a specific 
llvm generated code. You will need to check the generated code is still testing 
what it is supposed to test.

pw-bot: cr

>   		goto out;
> +	if (tcp->dest != 0)
> +		goto out;
>   
>   	urg_ptr = tcp->urg_ptr;
>

Re: [PATCH] selftests/bpf: Fix potential null pointer dereference in skb_pkt_end.c

Posted by Prabhav Kumar Vaish 9 months, 2 weeks ago

From: Prabhav Kumar <pvkumar5749404@gmail.com>

Hi Martin,
Thank you for the feedback.

You're right — I reviewed commit 9cc873e85800 and now understand that the test is designed to validate specific LLVM code generation, not just runtime behavior. I see how my change, although addressing the warning, could unintentionally alter the generated instructions and defeat the purpose of the test.

I'll drop this patch to preserve the original test intent. Thanks again for the context!

Best Regards,
Prabhav