[bpf-next,V1] selftests/bpf: fix unmap bug in prog_tests/xdp_metadata.c

Commit Message

Jesper Dangaard Brouer Feb. 1, 2023, 6:12 p.m. UTC

The function close_xsk() unmap via munmap() the wrong memory pointer.

The call xsk_umem__delete(xsk->umem) have already freed xsk->umem.
Thus the call to munmap(xsk->umem, UMEM_SIZE) will have unpredictable
behavior that can lead to Segmentation fault elsewhere, as man page
explain subsequent references to these pages will generate SIGSEGV.

Fixes: e2a46d54d7a1 ("selftests/bpf: Verify xdp_metadata xdp->af_xdp path")
Reported-by: Martin KaFai Lau <[e-mail]>
Signed-off-by: Jesper Dangaard Brouer <[e-mail]>
---
 .../selftests/bpf/prog_tests/xdp_metadata.c        |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stanislav Fomichev Feb. 1, 2023, 7:10 p.m. UTC | #1

On Wed, Feb 1, 2023 at 10:13 AM Jesper Dangaard Brouer
<[e-mail]> wrote:
>
> The function close_xsk() unmap via munmap() the wrong memory pointer.
>
> The call xsk_umem__delete(xsk->umem) have already freed xsk->umem.
> Thus the call to munmap(xsk->umem, UMEM_SIZE) will have unpredictable
> behavior that can lead to Segmentation fault elsewhere, as man page
> explain subsequent references to these pages will generate SIGSEGV.
>
> Fixes: e2a46d54d7a1 ("selftests/bpf: Verify xdp_metadata xdp->af_xdp path")
> Reported-by: Martin KaFai Lau <[e-mail]>
> Signed-off-by: Jesper Dangaard Brouer <[e-mail]>

Good catch, thank you!

Acked-by: Stanislav Fomichev <[e-mail]>

> ---
>  .../selftests/bpf/prog_tests/xdp_metadata.c        |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> index e033d48288c0..241909d71c7e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> +++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
> @@ -121,7 +121,7 @@ static void close_xsk(struct xsk *xsk)
>                 xsk_umem__delete(xsk->umem);
>         if (xsk->socket)
>                 xsk_socket__delete(xsk->socket);
> -       munmap(xsk->umem, UMEM_SIZE);
> +       munmap(xsk->umem_area, UMEM_SIZE);
>  }
>
>  static void ip_csum(struct iphdr *iph)
>
>

[e-mail] Feb. 1, 2023, 11:50 p.m. UTC | #2

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <[e-mail]>:

On Wed, 01 Feb 2023 19:12:54 +0100 you wrote:
> The function close_xsk() unmap via munmap() the wrong memory pointer.
> 
> The call xsk_umem__delete(xsk->umem) have already freed xsk->umem.
> Thus the call to munmap(xsk->umem, UMEM_SIZE) will have unpredictable
> behavior that can lead to Segmentation fault elsewhere, as man page
> explain subsequent references to these pages will generate SIGSEGV.
> 
> [...]

Here is the summary with links:
  - [bpf-next,V1] selftests/bpf: fix unmap bug in prog_tests/xdp_metadata.c
    https://git.kernel.org/bpf/bpf-next/c/f2922f77a6a6

You are awesome, thank you!

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
index e033d48288c0..241909d71c7e 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_metadata.c
@@ -121,7 +121,7 @@  static void close_xsk(struct xsk *xsk)
 		xsk_umem__delete(xsk->umem);
 	if (xsk->socket)
 		xsk_socket__delete(xsk->socket);
-	munmap(xsk->umem, UMEM_SIZE);
+	munmap(xsk->umem_area, UMEM_SIZE);
 }
 
 static void ip_csum(struct iphdr *iph)