* [PATCH bpf-next] bpf: Clear subreg_def for global function return values
@ 2021-02-12 4:04 Ilya Leoshkevich
2021-02-12 9:31 ` Yauheni Kaliuta
2021-02-15 22:50 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Ilya Leoshkevich @ 2021-02-12 4:04 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann
Cc: bpf, Heiko Carstens, Vasily Gorbik, Yauheni Kaliuta, Ilya Leoshkevich
test_global_func4 fails on s390 as reported by Yauheni in [1].
The immediate problem is that the zext code includes the instruction,
whose result needs to be zero-extended, into the zero-extension
patchlet, and if this instruction happens to be a branch, then its
delta is not adjusted. As a result, the verifier rejects the program
later.
However, according to [2], as far as the verifier's algorithm is
concerned and as specified by the insn_no_def() function, branching
insns do not define anything. This includes call insns, even though
one might argue that they define %r0.
This means that the real problem is that zero extension kicks in at
all. This happens because clear_caller_saved_regs() sets BPF_REG_0's
subreg_def after global function calls. This can be fixed in many
ways; this patch mimics what helper function call handling already
does.
[1] https://lore.kernel.org/bpf/20200903140542.156624-1-yauheni.kaliuta@redhat.com/
[2] https://lore.kernel.org/bpf/CAADnVQ+2RPKcftZw8d+B1UwB35cpBhpF5u3OocNh90D9pETPwg@mail.gmail.com/
Fixes: 51c39bb1d5d1 ("bpf: Introduce function-by-function verification")
Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
kernel/bpf/verifier.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index beae700bb56e..183fae996ad0 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -5211,8 +5211,9 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
subprog);
clear_caller_saved_regs(env, caller->regs);
- /* All global functions return SCALAR_VALUE */
+ /* All global functions return a 64-bit SCALAR_VALUE */
mark_reg_unknown(env, caller->regs, BPF_REG_0);
+ caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
/* continue with next insn after call */
return 0;
--
2.29.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Clear subreg_def for global function return values
2021-02-12 4:04 [PATCH bpf-next] bpf: Clear subreg_def for global function return values Ilya Leoshkevich
@ 2021-02-12 9:31 ` Yauheni Kaliuta
2021-02-15 22:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Yauheni Kaliuta @ 2021-02-12 9:31 UTC (permalink / raw)
To: Ilya Leoshkevich
Cc: Alexei Starovoitov, Daniel Borkmann, bpf, Heiko Carstens, Vasily Gorbik
Hi, Ilya!
>>>>> On Fri, 12 Feb 2021 05:04:08 +0100, Ilya Leoshkevich wrote:
> test_global_func4 fails on s390 as reported by Yauheni in [1].
> The immediate problem is that the zext code includes the instruction,
> whose result needs to be zero-extended, into the zero-extension
> patchlet, and if this instruction happens to be a branch, then its
> delta is not adjusted. As a result, the verifier rejects the program
> later.
Thank you for addressing that!
> However, according to [2], as far as the verifier's algorithm is
> concerned and as specified by the insn_no_def() function, branching
> insns do not define anything. This includes call insns, even though
> one might argue that they define %r0.
I still think that the patching code should be fixed as well,
even if it's a separate issue.
But I got the attitude.
> This means that the real problem is that zero extension kicks in at
> all. This happens because clear_caller_saved_regs() sets BPF_REG_0's
> subreg_def after global function calls. This can be fixed in many
> ways; this patch mimics what helper function call handling already
> does.
> [1] https://lore.kernel.org/bpf/20200903140542.156624-1-yauheni.kaliuta@redhat.com/
> [2]
> https://lore.kernel.org/bpf/CAADnVQ+2RPKcftZw8d+B1UwB35cpBhpF5u3OocNh90D9pETPwg@mail.gmail.com/
> Fixes: 51c39bb1d5d1 ("bpf: Introduce function-by-function verification")
> Reported-by: Yauheni Kaliuta <yauheni.kaliuta@redhat.com>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
> kernel/bpf/verifier.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index beae700bb56e..183fae996ad0 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5211,8 +5211,9 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
> subprog);
> clear_caller_saved_regs(env, caller->regs);
> - /* All global functions return SCALAR_VALUE */
> + /* All global functions return a 64-bit SCALAR_VALUE */
> mark_reg_unknown(env, caller->regs, BPF_REG_0);
> + caller->regs[BPF_REG_0].subreg_def = DEF_NOT_SUBREG;
> /* continue with next insn after call */
> return 0;
> --
> 2.29.2
--
WBR,
Yauheni Kaliuta
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next] bpf: Clear subreg_def for global function return values
2021-02-12 4:04 [PATCH bpf-next] bpf: Clear subreg_def for global function return values Ilya Leoshkevich
2021-02-12 9:31 ` Yauheni Kaliuta
@ 2021-02-15 22:50 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-15 22:50 UTC (permalink / raw)
To: Ilya Leoshkevich; +Cc: ast, daniel, bpf, heiko.carstens, gor, yauheni.kaliuta
Hello:
This patch was applied to bpf/bpf-next.git (refs/heads/master):
On Fri, 12 Feb 2021 05:04:08 +0100 you wrote:
> test_global_func4 fails on s390 as reported by Yauheni in [1].
>
> The immediate problem is that the zext code includes the instruction,
> whose result needs to be zero-extended, into the zero-extension
> patchlet, and if this instruction happens to be a branch, then its
> delta is not adjusted. As a result, the verifier rejects the program
> later.
>
> [...]
Here is the summary with links:
- [bpf-next] bpf: Clear subreg_def for global function return values
https://git.kernel.org/bpf/bpf-next/c/45159b27637b
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-02-15 22:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-12 4:04 [PATCH bpf-next] bpf: Clear subreg_def for global function return values Ilya Leoshkevich
2021-02-12 9:31 ` Yauheni Kaliuta
2021-02-15 22:50 ` patchwork-bot+netdevbpf
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.