]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
bpf, sockmap: Several fixes to bpf_msg_push_data
authorZijian Zhang <zijianzhang@bytedance.com>
Wed, 6 Nov 2024 22:25:18 +0000 (22:25 +0000)
committerMartin KaFai Lau <martin.lau@kernel.org>
Thu, 7 Nov 2024 00:01:53 +0000 (16:01 -0800)
Several fixes to bpf_msg_push_data,
1. test_sockmap has tests where bpf_msg_push_data is invoked to push some
data at the end of a message, but -EINVAL is returned. In this case, in
bpf_msg_push_data, after the first loop, i will be set to msg->sg.end, add
the logic to handle it.
2. In the code block of "if (start - offset)", it's possible that "i"
points to the last of sk_msg_elem. In this case, "sk_msg_iter_next(msg,
end)" might still be called twice, another invoking is in "if (!copy)"
code block, but actually only one is needed. Add the logic to handle it,
and reconstruct the code to make the logic more clear.

Fixes: 6fff607e2f14 ("bpf: sk_msg program helper bpf_msg_push_data")
Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
Link: https://lore.kernel.org/r/20241106222520.527076-7-zijianzhang@bytedance.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
net/core/filter.c

index 82f92ed0dc7234efc090c8eb4e4973ed10de66f6..255d58bae2a900b948e653e567208e790675c83d 100644 (file)
@@ -2778,7 +2778,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);
@@ -2803,6 +2803,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;
@@ -2819,7 +2821,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);
 
@@ -2830,39 +2838,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;