* [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode
@ 2022-03-10 22:56 Toke Høiland-Jørgensen
2022-03-10 22:56 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect Toke Høiland-Jørgensen
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-03-10 22:56 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Martin KaFai Lau, Song Liu, Yonghong Song, John Fastabend,
KP Singh, David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
Toke Høiland-Jørgensen
Cc: syzbot+0e91362d99386dc5de99, netdev, bpf
The live packet mode uses some extra space at the start of each page to
cache data structures so they don't have to be rebuilt at every repetition.
This space wasn't correctly accounted for in the size checking of the
arguments supplied to userspace. In addition, the definition of the frame
size should include the size of the skb_shared_info (as there is other
logic that subtracts the size of this).
Together, these mistakes resulted in userspace being able to trip the
XDP_WARN() in xdp_update_frame_from_buff(), which syzbot discovered in
short order. Fix this by changing the frame size define and adding the
extra headroom to the bpf_prog_test_run_xdp() function. Also drop the
max_len parameter to the page_pool init, since this is related to DMA which
is not used for the page pool instance in PROG_TEST_RUN.
Reported-by: syzbot+0e91362d99386dc5de99@syzkaller.appspotmail.com
Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in BPF_PROG_RUN")
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
net/bpf/test_run.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 24405a280a9b..e7b9c2636d10 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -112,8 +112,7 @@ struct xdp_test_data {
u32 frame_cnt;
};
-#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head) \
- - sizeof(struct skb_shared_info))
+#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head))
#define TEST_XDP_MAX_BATCH 256
static void xdp_test_run_init_page(struct page *page, void *arg)
@@ -156,7 +155,6 @@ static int xdp_test_run_setup(struct xdp_test_data *xdp, struct xdp_buff *orig_c
.flags = 0,
.pool_size = xdp->batch_size,
.nid = NUMA_NO_NODE,
- .max_len = TEST_XDP_FRAME_SIZE,
.init_callback = xdp_test_run_init_page,
.init_arg = xdp,
};
@@ -1230,6 +1228,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
batch_size = NAPI_POLL_WEIGHT;
else if (batch_size > TEST_XDP_MAX_BATCH)
return -E2BIG;
+
+ headroom += sizeof(struct xdp_page_head);
} else if (batch_size) {
return -EINVAL;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect
2022-03-10 22:56 [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Toke Høiland-Jørgensen
@ 2022-03-10 22:56 ` Toke Høiland-Jørgensen
2022-03-11 0:28 ` Martin KaFai Lau
2022-03-11 0:05 ` [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Martin KaFai Lau
2022-03-11 21:10 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-03-10 22:56 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
Andrii Nakryiko, Martin KaFai Lau, Song Liu, Yonghong Song,
KP Singh
Cc: Toke Høiland-Jørgensen, Shuah Khan, netdev, bpf
This adds an extra test to the xdp_do_redirect selftest for XDP live packet
mode, which verifies that the maximum permissible packet size is accepted
without any errors, and that a too big packet is correctly rejected.
Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
---
.../bpf/prog_tests/xdp_do_redirect.c | 24 +++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
index 9926b07e38c8..a50971c6cf4a 100644
--- a/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
+++ b/tools/testing/selftests/bpf/prog_tests/xdp_do_redirect.c
@@ -62,6 +62,28 @@ static int attach_tc_prog(struct bpf_tc_hook *hook, int fd)
return 0;
}
+/* The maximum permissible size is: PAGE_SIZE - sizeof(struct xdp_page_head) -
+ * sizeof(struct skb_shared_info) - XDP_PACKET_HEADROOM = 3368 bytes
+ */
+#define MAX_PKT_SIZE 3368
+static void test_max_pkt_size(int fd)
+{
+ char data[MAX_PKT_SIZE + 1] = {};
+ int err;
+ DECLARE_LIBBPF_OPTS(bpf_test_run_opts, opts,
+ .data_in = &data,
+ .data_size_in = MAX_PKT_SIZE,
+ .flags = BPF_F_TEST_XDP_LIVE_FRAMES,
+ .repeat = 1,
+ );
+ err = bpf_prog_test_run_opts(fd, &opts);
+ ASSERT_OK(err, "prog_run_max_size");
+
+ opts.data_size_in += 1;
+ err = bpf_prog_test_run_opts(fd, &opts);
+ ASSERT_EQ(err, -EINVAL, "prog_run_too_big");
+}
+
#define NUM_PKTS 10000
void test_xdp_do_redirect(void)
{
@@ -167,6 +189,8 @@ void test_xdp_do_redirect(void)
ASSERT_EQ(skel->bss->pkts_seen_zero, 2, "pkt_count_zero");
ASSERT_EQ(skel->bss->pkts_seen_tc, NUM_PKTS - 2, "pkt_count_tc");
+ test_max_pkt_size(bpf_program__fd(skel->progs.xdp_count_pkts));
+
out_tc:
bpf_tc_hook_destroy(&tc_hook);
out:
--
2.35.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode
2022-03-10 22:56 [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Toke Høiland-Jørgensen
2022-03-10 22:56 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect Toke Høiland-Jørgensen
@ 2022-03-11 0:05 ` Martin KaFai Lau
2022-03-11 15:02 ` Toke Høiland-Jørgensen
2022-03-11 21:10 ` patchwork-bot+netdevbpf
2 siblings, 1 reply; 6+ messages in thread
From: Martin KaFai Lau @ 2022-03-11 0:05 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu,
Yonghong Song, John Fastabend, KP Singh, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer,
syzbot+0e91362d99386dc5de99, netdev, bpf
On Thu, Mar 10, 2022 at 11:56:20PM +0100, Toke Høiland-Jørgensen wrote:
> The live packet mode uses some extra space at the start of each page to
> cache data structures so they don't have to be rebuilt at every repetition.
> This space wasn't correctly accounted for in the size checking of the
> arguments supplied to userspace. In addition, the definition of the frame
> size should include the size of the skb_shared_info (as there is other
> logic that subtracts the size of this).
>
> Together, these mistakes resulted in userspace being able to trip the
> XDP_WARN() in xdp_update_frame_from_buff(), which syzbot discovered in
> short order. Fix this by changing the frame size define and adding the
> extra headroom to the bpf_prog_test_run_xdp() function. Also drop the
> max_len parameter to the page_pool init, since this is related to DMA which
> is not used for the page pool instance in PROG_TEST_RUN.
>
> Reported-by: syzbot+0e91362d99386dc5de99@syzkaller.appspotmail.com
> Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in BPF_PROG_RUN")
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
> ---
> net/bpf/test_run.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 24405a280a9b..e7b9c2636d10 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -112,8 +112,7 @@ struct xdp_test_data {
> u32 frame_cnt;
> };
>
> -#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head) \
> - - sizeof(struct skb_shared_info))
> +#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head))
> #define TEST_XDP_MAX_BATCH 256
>
> static void xdp_test_run_init_page(struct page *page, void *arg)
> @@ -156,7 +155,6 @@ static int xdp_test_run_setup(struct xdp_test_data *xdp, struct xdp_buff *orig_c
> .flags = 0,
> .pool_size = xdp->batch_size,
> .nid = NUMA_NO_NODE,
> - .max_len = TEST_XDP_FRAME_SIZE,
> .init_callback = xdp_test_run_init_page,
> .init_arg = xdp,
> };
> @@ -1230,6 +1228,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
> batch_size = NAPI_POLL_WEIGHT;
> else if (batch_size > TEST_XDP_MAX_BATCH)
> return -E2BIG;
> +
> + headroom += sizeof(struct xdp_page_head);
The orig_ctx->data_end will ensure there is a sizeof(struct skb_shared_info)
tailroom ? It is quite tricky to read but I don't have a better idea
either.
Acked-by: Martin KaFai Lau <kafai@fb.com>
> } else if (batch_size) {
> return -EINVAL;
> }
> --
> 2.35.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect
2022-03-10 22:56 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect Toke Høiland-Jørgensen
@ 2022-03-11 0:28 ` Martin KaFai Lau
0 siblings, 0 replies; 6+ messages in thread
From: Martin KaFai Lau @ 2022-03-11 0:28 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Alexei Starovoitov, Daniel Borkmann, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer, John Fastabend,
Andrii Nakryiko, Song Liu, Yonghong Song, KP Singh, Shuah Khan,
netdev, bpf
On Thu, Mar 10, 2022 at 11:56:21PM +0100, Toke Høiland-Jørgensen wrote:
> This adds an extra test to the xdp_do_redirect selftest for XDP live packet
> mode, which verifies that the maximum permissible packet size is accepted
> without any errors, and that a too big packet is correctly rejected.
Acked-by: Martin KaFai Lau <kafai@fb.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode
2022-03-11 0:05 ` [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Martin KaFai Lau
@ 2022-03-11 15:02 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2022-03-11 15:02 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Song Liu,
Yonghong Song, John Fastabend, KP Singh, David S. Miller,
Jakub Kicinski, Jesper Dangaard Brouer,
syzbot+0e91362d99386dc5de99, netdev, bpf
Martin KaFai Lau <kafai@fb.com> writes:
> On Thu, Mar 10, 2022 at 11:56:20PM +0100, Toke Høiland-Jørgensen wrote:
>> The live packet mode uses some extra space at the start of each page to
>> cache data structures so they don't have to be rebuilt at every repetition.
>> This space wasn't correctly accounted for in the size checking of the
>> arguments supplied to userspace. In addition, the definition of the frame
>> size should include the size of the skb_shared_info (as there is other
>> logic that subtracts the size of this).
>>
>> Together, these mistakes resulted in userspace being able to trip the
>> XDP_WARN() in xdp_update_frame_from_buff(), which syzbot discovered in
>> short order. Fix this by changing the frame size define and adding the
>> extra headroom to the bpf_prog_test_run_xdp() function. Also drop the
>> max_len parameter to the page_pool init, since this is related to DMA which
>> is not used for the page pool instance in PROG_TEST_RUN.
>>
>> Reported-by: syzbot+0e91362d99386dc5de99@syzkaller.appspotmail.com
>> Fixes: b530e9e1063e ("bpf: Add "live packet" mode for XDP in BPF_PROG_RUN")
>> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
>> ---
>> net/bpf/test_run.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
>> index 24405a280a9b..e7b9c2636d10 100644
>> --- a/net/bpf/test_run.c
>> +++ b/net/bpf/test_run.c
>> @@ -112,8 +112,7 @@ struct xdp_test_data {
>> u32 frame_cnt;
>> };
>>
>> -#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head) \
>> - - sizeof(struct skb_shared_info))
>> +#define TEST_XDP_FRAME_SIZE (PAGE_SIZE - sizeof(struct xdp_page_head))
>> #define TEST_XDP_MAX_BATCH 256
>>
>> static void xdp_test_run_init_page(struct page *page, void *arg)
>> @@ -156,7 +155,6 @@ static int xdp_test_run_setup(struct xdp_test_data *xdp, struct xdp_buff *orig_c
>> .flags = 0,
>> .pool_size = xdp->batch_size,
>> .nid = NUMA_NO_NODE,
>> - .max_len = TEST_XDP_FRAME_SIZE,
>> .init_callback = xdp_test_run_init_page,
>> .init_arg = xdp,
>> };
>> @@ -1230,6 +1228,8 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr,
>> batch_size = NAPI_POLL_WEIGHT;
>> else if (batch_size > TEST_XDP_MAX_BATCH)
>> return -E2BIG;
>> +
>> + headroom += sizeof(struct xdp_page_head);
> The orig_ctx->data_end will ensure there is a sizeof(struct skb_shared_info)
> tailroom ? It is quite tricky to read but I don't have a better idea
> either.
Yeah, the length checks are all done for the non-live data case (in
bpf_test_init()), so seemed simpler to just account the extra headroom
to those checks instead of adding an extra check to the live-packet
code.
> Acked-by: Martin KaFai Lau <kafai@fb.com>
Thanks!
-Toke
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode
2022-03-10 22:56 [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Toke Høiland-Jørgensen
2022-03-10 22:56 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect Toke Høiland-Jørgensen
2022-03-11 0:05 ` [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Martin KaFai Lau
@ 2022-03-11 21:10 ` patchwork-bot+netdevbpf
2 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-03-11 21:10 UTC (permalink / raw)
To: =?utf-8?b?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2VuIDx0b2tlQHJlZGhhdC5jb20+?=
Cc: ast, daniel, andrii, kafai, songliubraving, yhs, john.fastabend,
kpsingh, davem, kuba, hawk, syzbot+0e91362d99386dc5de99, netdev,
bpf
Hello:
This series was applied to bpf/bpf-next.git (master)
by Daniel Borkmann <daniel@iogearbox.net>:
On Thu, 10 Mar 2022 23:56:20 +0100 you wrote:
> The live packet mode uses some extra space at the start of each page to
> cache data structures so they don't have to be rebuilt at every repetition.
> This space wasn't correctly accounted for in the size checking of the
> arguments supplied to userspace. In addition, the definition of the frame
> size should include the size of the skb_shared_info (as there is other
> logic that subtracts the size of this).
>
> [...]
Here is the summary with links:
- [bpf-next,1/2] bpf, test_run: Fix packet size check for live packet mode
https://git.kernel.org/bpf/bpf-next/c/b6f1f780b393
- [bpf-next,2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect
https://git.kernel.org/bpf/bpf-next/c/c09df4bd3a91
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-03-11 22:52 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-10 22:56 [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Toke Høiland-Jørgensen
2022-03-10 22:56 ` [PATCH bpf-next 2/2] selftests/bpf: Add a test for maximum packet size in xdp_do_redirect Toke Høiland-Jørgensen
2022-03-11 0:28 ` Martin KaFai Lau
2022-03-11 0:05 ` [PATCH bpf-next 1/2] bpf, test_run: Fix packet size check for live packet mode Martin KaFai Lau
2022-03-11 15:02 ` Toke Høiland-Jørgensen
2022-03-11 21:10 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).