On 2022/10/18 0:36, Stanislav Fomichev wrote:
On Sat, Oct 15, 2022 at 2:16 AM Zhengchao Shao <shaozhengchao@xxxxxxxxxx> wrote:
As [0] see, bpf_prog_test_run_skb() should allow user space to forward
14-bytes packet via BPF_PROG_RUN instead of dropping packet directly.
So fix it.
0: https://github.com/cilium/ebpf/commit/a38fb6b5a46ab3b5639ea4d421232a10013596c0
Fixes: fd1894224407 ("bpf: Don't redirect packets with invalid pkt_len")
Signed-off-by: Zhengchao Shao <shaozhengchao@xxxxxxxxxx>
---
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 13d578ce2a09..aa1b49f19ca3 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -979,9 +979,6 @@ static int convert___skb_to_skb(struct sk_buff *skb, struct __sk_buff *__skb)
{
struct qdisc_skb_cb *cb = (struct qdisc_skb_cb *)skb->cb;
- if (!skb->len)
- return -EINVAL;
-
if (!__skb)
return 0;
@@ -1102,6 +1099,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
if (IS_ERR(data))
return PTR_ERR(data);
+ if (size == ETH_HLEN)
+ is_l2 = true;
+
Don't think this will work? That is_l2 is there to expose proper l2/l3
skb for specific hooks; we can't suddenly start exposing l2 headers to
the hooks that don't expect it.
Does it make sense to start with a small reproducer that triggers the
issue first? We can have a couple of cases for
len=0/ETH_HLEN-1/ETH_HLEN+1 and trigger them from the bpf program that
redirects to different devices (to trigger dev_is_mac_header_xmit).
Hi Stanislav:
Thank you for your review. Is_l2 is the flag of a specific
hook. Therefore, do you mean that if skb->len is equal to 0, just
add the length back?
ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
if (IS_ERR(ctx)) {
kfree(data);
--
2.17.1
