On 5/12/20 9:25 AM, Alexei Starovoitov wrote:
On Tue, May 12, 2020 at 08:41:19AM -0700, Yonghong Song wrote:
On 5/9/20 5:41 PM, Alexei Starovoitov wrote:
On Sat, May 09, 2020 at 10:59:00AM -0700, Yonghong Song wrote:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 70ad009577f8..d725ff7d11db 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -7101,6 +7101,10 @@ static int check_return_code(struct bpf_verifier_env *env)
return 0;
range = tnum_const(0);
break;
+ case BPF_PROG_TYPE_TRACING:
+ if (env->prog->expected_attach_type != BPF_TRACE_ITER)
+ return 0;
+ break;
Not related to this set, but I just noticed that I managed to forget to
add this check for fentry/fexit/freplace.
While it's not too late let's enforce return 0 for them ?
Could you follow up with a patch for bpf tree?
Just want to double check. In selftests, we have
SEC("fentry/__set_task_comm")
int BPF_PROG(prog4, struct task_struct *tsk, const char *buf, bool exec)
{
return !tsk;
}
SEC("fexit/__set_task_comm")
int BPF_PROG(prog5, struct task_struct *tsk, const char *buf, bool exec)
{
return !tsk;
}
fentry/fexit may returrn 1. What is the intention here? Does this mean
we should allow [0, 1] instead of [0, 0]?
Argh. I missed that bit when commit ac065870d9282 tweaked the return
value. For fentry/exit the return value is ignored by trampoline.
imo it's misleading to users and should be rejected by the verifier.
so [0,0] for fentry/fexit
Sounds good. Will craft patch to enforce fentry/fexit with [0,0] then.
Thanks!
For freplace, we have
__u64 test_get_skb_len = 0;
SEC("freplace/get_skb_len")
int new_get_skb_len(struct __sk_buff *skb)
{
int len = skb->len;
if (len != 74)
return 0;
test_get_skb_len = 1;
return 74; /* original get_skb_len() returns skb->len */
}
That means freplace may return arbitrary values depending on what
to replace?
yes. freplace and fmod_ret can return anything.
