Skip to content

Commit

Permalink
Merge branch 'Fixes to bpf_msg_push/pop_data and test_sockmap'
Browse files Browse the repository at this point in the history 
Zijian Zhang says:

====================
Several fixes to test_sockmap and added push/pop logic for msg_verify_data
Before the fixes, some of the tests in test_sockmap are problematic,
resulting in pseudo-correct result.

1. txmsg_pass is not set in some tests, as a result, no eBPF program is
attached to the sockmap.
2. In SENDPAGE, a wrong iov_length in test_send_large may result in some
test skippings and failures.
3. The calculation of total_bytes in msg_loop_rx is wrong, which may cause
msg_loop_rx end early and skip some data tests.

Besides, for msg_verify_data, I added push/pop checking logic to function
msg_verify_data and added more tests for different cases.

After that, I found that there are some bugs in bpf_msg_push_data,
bpf_msg_pop_data and sk_msg_reset_curr, and fix them. I guess the reason
why they have not been exposed is that because of the above problems, they
will not be triggered.

With the fixes, we can pass the sockmap test with data integrity test now.
However, the fixes to test_sockmap expose more problems in sockhash test
with SENDPAGE and ktls with SENDPAGE.

v1 -> v2:
  - Rebased to the latest bpf-next net branch.

The problem I observed,
1. In sockhash test, a NULL pointer kernel BUG will be reported for nearly
every cork test. More inspections are needed for splice_to_socket.

BUG: kernel NULL pointer dereference, address: 0000000000000008
PGD 0 P4D 0
Oops: Oops: 0000 [#3] PREEMPT SMP PTI
CPU: 3 UID: 0 PID: 2122 Comm: test_sockmap 6.12.0-rc2.bm.1-amd64+ torvalds#98
Tainted: [D]=DIE
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014
RIP: 0010:splice_to_socket+0x34a/0x480
Call Trace:
 <TASK>
 ? __die_body+0x1e/0x60
 ? page_fault_oops+0x159/0x4d0
 ? exc_page_fault+0x7e/0x180
 ? asm_exc_page_fault+0x26/0x30
 ? splice_to_socket+0x34a/0x480
? __memcg_slab_post_alloc_hook+0x205/0x3c0
? alloc_pipe_info+0xd6/0x1f0
? __kmalloc_noprof+0x37f/0x3b0
direct_splice_actor+0x40/0x100
splice_direct_to_actor+0xfd/0x290
? __pfx_direct_splice_actor+0x10/0x10
do_splice_direct_actor+0x82/0xb0
? __pfx_direct_file_splice_eof+0x10/0x10
do_splice_direct+0x13/0x20
? __pfx_direct_splice_actor+0x10/0x10
do_sendfile+0x33c/0x3f0
__x64_sys_sendfile64+0xa7/0xc0
do_syscall_64+0x62/0x170
entry_SYSCALL_64_after_hwframe+0x76/0x7e
 </TASK>
Modules linked in:
CR2: 0000000000000008
---[ end trace 0000000000000000 ]---

2. txmsg_pass are not set before, and some tests are skipped. Now after
the fixes, we have some failure cases now. More fixes are needed either
for the selftest or the ktls kernel code.

1/ 6 sockhash:ktls:txmsg test passthrough:OK
2/ 6 sockhash:ktls:txmsg test redirect:OK
3/ 1 sockhash:ktls:txmsg test redirect wait send mem:OK
4/ 6 sockhash:ktls:txmsg test drop:OK
5/ 6 sockhash:ktls:txmsg test ingress redirect:OK
6/ 7 sockhash:ktls:txmsg test skb:OK
7/12 sockhash:ktls:txmsg test apply:OK
8/12 sockhash:ktls:txmsg test cork:OK
9/ 3 sockhash:ktls:txmsg test hanging corks:OK
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
10/11 sockhash:ktls:txmsg test push_data:FAIL
detected data corruption @Iov[0]:0 17 != 00, 00 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 00 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
11/17 sockhash:ktls:txmsg test pull-data:FAIL
recv failed(): Invalid argument
rx thread exited with err 1.
recv failed(): Invalid argument
rx thread exited with err 1.
recv failed(): Bad message
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
detected data corruption @Iov[0]:0 17 != 00, 03 ?= 01
data verify msg failed: Unknown error -2001
rx thread exited with err 1.
12/ 9 sockhash:ktls:txmsg test pop-data:FAIL
recv failed(): Bad message
rx thread exited with err 1.
recv failed(): Bad message
rx thread exited with err 1.
13/ 6 sockhash:ktls:txmsg test push/pop data:FAIL
14/ 1 sockhash:ktls:txmsg test ingress parser:OK
15/ 0 sockhash:ktls:txmsg test ingress parser2:OK
Pass: 11 Fail: 17
====================

Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
  • Loading branch information
Martin KaFai Lau committed Nov 7, 2024
2 parents ac1bd50 + 955afd5 commit 141b4d6
Show file tree
Hide file tree
Showing 2 changed files with 214 additions and 54 deletions.
88 changes: 51 additions & 37 deletions net/core/filter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2604,18 +2604,16 @@ BPF_CALL_2(bpf_msg_cork_bytes, struct sk_msg *, msg, u32, bytes)

static void sk_msg_reset_curr(struct sk_msg *msg)
{
u32 i = msg->sg.start;
u32 len = 0;

do {
len += sk_msg_elem(msg, i)->length;
sk_msg_iter_var_next(i);
if (len >= msg->sg.size)
break;
} while (i != msg->sg.end);
if (!msg->sg.size) {
msg->sg.curr = msg->sg.start;
msg->sg.copybreak = 0;
} else {
u32 i = msg->sg.end;

msg->sg.curr = i;
msg->sg.copybreak = 0;
sk_msg_iter_var_prev(i);
msg->sg.curr = i;
msg->sg.copybreak = msg->sg.data[i].length;
}
}

static const struct bpf_func_proto bpf_msg_cork_bytes_proto = {
Expand Down Expand Up @@ -2778,7 +2776,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
sk_msg_iter_var_next(i);
} while (i != msg->sg.end);

if (start >= offset + l)
if (start > offset + l)
return -EINVAL;

space = MAX_MSG_FRAGS - sk_msg_elem_used(msg);
Expand All @@ -2803,6 +2801,8 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,

raw = page_address(page);

if (i == msg->sg.end)
sk_msg_iter_var_prev(i);
psge = sk_msg_elem(msg, i);
front = start - offset;
back = psge->length - front;
Expand All @@ -2819,7 +2819,13 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
}

put_page(sg_page(psge));
} else if (start - offset) {
new = i;
goto place_new;
}

if (start - offset) {
if (i == msg->sg.end)
sk_msg_iter_var_prev(i);
psge = sk_msg_elem(msg, i);
rsge = sk_msg_elem_cpy(msg, i);

Expand All @@ -2830,39 +2836,44 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
sk_msg_iter_var_next(i);
sg_unmark_end(psge);
sg_unmark_end(&rsge);
sk_msg_iter_next(msg, end);
}

/* Slot(s) to place newly allocated data */
sk_msg_iter_next(msg, end);
new = i;
sk_msg_iter_var_next(i);

if (i == msg->sg.end) {
if (!rsge.length)
goto place_new;
sk_msg_iter_next(msg, end);
goto place_new;
}

/* Shift one or two slots as needed */
if (!copy) {
sge = sk_msg_elem_cpy(msg, i);
sge = sk_msg_elem_cpy(msg, new);
sg_unmark_end(&sge);

nsge = sk_msg_elem_cpy(msg, i);
if (rsge.length) {
sk_msg_iter_var_next(i);
sg_unmark_end(&sge);
nnsge = sk_msg_elem_cpy(msg, i);
sk_msg_iter_next(msg, end);
}

nsge = sk_msg_elem_cpy(msg, i);
while (i != msg->sg.end) {
msg->sg.data[i] = sge;
sge = nsge;
sk_msg_iter_var_next(i);
if (rsge.length) {
sk_msg_iter_var_next(i);
nsge = nnsge;
nnsge = sk_msg_elem_cpy(msg, i);
}

while (i != msg->sg.end) {
msg->sg.data[i] = sge;
sge = nsge;
sk_msg_iter_var_next(i);
if (rsge.length) {
nsge = nnsge;
nnsge = sk_msg_elem_cpy(msg, i);
} else {
nsge = sk_msg_elem_cpy(msg, i);
}
} else {
nsge = sk_msg_elem_cpy(msg, i);
}
}

place_new:
/* Place newly allocated data buffer */
sk_mem_charge(msg->sk, len);
msg->sg.size += len;
Expand Down Expand Up @@ -2891,8 +2902,10 @@ static const struct bpf_func_proto bpf_msg_push_data_proto = {

static void sk_msg_shift_left(struct sk_msg *msg, int i)
{
struct scatterlist *sge = sk_msg_elem(msg, i);
int prev;

put_page(sg_page(sge));
do {
prev = i;
sk_msg_iter_var_next(i);
Expand Down Expand Up @@ -2929,6 +2942,9 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
if (unlikely(flags))
return -EINVAL;

if (unlikely(len == 0))
return 0;

/* First find the starting scatterlist element */
i = msg->sg.start;
do {
Expand All @@ -2941,7 +2957,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
} while (i != msg->sg.end);

/* Bounds checks: start and pop must be inside message */
if (start >= offset + l || last >= msg->sg.size)
if (start >= offset + l || last > msg->sg.size)
return -EINVAL;

space = MAX_MSG_FRAGS - sk_msg_elem_used(msg);
Expand Down Expand Up @@ -2970,12 +2986,12 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
*/
if (start != offset) {
struct scatterlist *nsge, *sge = sk_msg_elem(msg, i);
int a = start;
int a = start - offset;
int b = sge->length - pop - a;

sk_msg_iter_var_next(i);

if (pop < sge->length - a) {
if (b > 0) {
if (space) {
sge->length = a;
sk_msg_shift_right(msg, i);
Expand All @@ -2994,7 +3010,6 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
if (unlikely(!page))
return -ENOMEM;

sge->length = a;
orig = sg_page(sge);
from = sg_virt(sge);
to = page_address(page);
Expand All @@ -3004,7 +3019,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
put_page(orig);
}
pop = 0;
} else if (pop >= sge->length - a) {
} else {
pop -= (sge->length - a);
sge->length = a;
}
Expand Down Expand Up @@ -3038,7 +3053,6 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
pop -= sge->length;
sk_msg_shift_left(msg, i);
}
sk_msg_iter_var_next(i);
}

sk_mem_uncharge(msg->sk, len - pop);
Expand Down
Loading

0 comments on commit 141b4d6

Please sign in to comment.