bpf, sockmap: Several fixes to bpf_msg_pop_data

Several fixes to bpf_msg_pop_data,
1. In sk_msg_shift_left, we should put_page
2. if (len == 0), return early is better
3. pop the entire sk_msg (last == msg->sg.size) should be supported
4. Fix for the value of variable "a"
5. In sk_msg_shift_left, after shifting, i has already pointed to the next
element. Addtional sk_msg_iter_var_next may result in BUG.

Fixes: 7246d8ed4dcc ("bpf: helper to pop data from messages")
Signed-off-by: Zijian Zhang <zijianzhang@bytedance.com>
Reviewed-by: John Fastabend <john.fastabend@gmail.com>
Link: https://lore.kernel.org/r/20241106222520.527076-8-zijianzhang@bytedance.com
Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>

diff --git a/net/core/filter.c b/net/core/filter.c
index 255d58bae2a900b948e653e567208e790675c83d..2fdba950b5751586b264ba08f088b7ca1e3e605a 100644 (file)
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2904,8 +2904,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);
@@ -2942,6 +2944,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 {
@@ -2954,7 +2959,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);
@@ -2983,12 +2988,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);
@@ -3007,7 +3012,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);
@@ -3017,7 +3021,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;
                }
@@ -3051,7 +3055,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);