* [PATCH bpf-next 1/2] bpf: prevent r10 register from being marked as precise
@ 2024-04-02 22:50 Andrii Nakryiko
2024-04-02 22:50 ` [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2024-04-02 22:50 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau
Cc: andrii, kernel-team, syzbot+148110ee7cf72f39f33e
r10 is a special register that is not under BPF program's control and is
always effectively precise. The rest of precision logic assumes that
only r0-r9 SCALAR registers are marked as precise, so prevent r10 from
being marked precise.
This can happen due to signed cast instruction allowing to do something
like `r0 = (s8)r10;`, which later, if r0 needs to be precise, would lead
to an attempt to mark r10 as precise.
Prevent this with an extra check during instruction backtracking.
Fixes: 8100928c8814 ("bpf: Support new sign-extension mov insns")
Reported-by: syzbot+148110ee7cf72f39f33e@syzkaller.appspotmail.com
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
kernel/bpf/verifier.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index fcb62300f407..02361e2a2f4e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -3615,7 +3615,8 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* sreg needs precision before this insn
*/
bt_clear_reg(bt, dreg);
- bt_set_reg(bt, sreg);
+ if (sreg != BPF_REG_FP)
+ bt_set_reg(bt, sreg);
} else {
/* dreg = K
* dreg needs precision after this insn.
@@ -3631,7 +3632,8 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
* both dreg and sreg need precision
* before this insn
*/
- bt_set_reg(bt, sreg);
+ if (sreg != BPF_REG_FP)
+ bt_set_reg(bt, sreg);
} /* else dreg += K
* dreg still needs precision before this insn
*/
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-02 22:50 [PATCH bpf-next 1/2] bpf: prevent r10 register from being marked as precise Andrii Nakryiko
@ 2024-04-02 22:50 ` Andrii Nakryiko
2024-04-02 23:26 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2024-04-02 22:50 UTC (permalink / raw)
To: bpf, ast, daniel, martin.lau
Cc: andrii, kernel-team, syzbot+148110ee7cf72f39f33e
Add selftests validating that BPF verifier handles precision marking
for SCALAR registers derived from r10 (fp) register correctly.
Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
.../bpf/progs/verifier_subprog_precision.c | 86 +++++++++++++++++++
1 file changed, 86 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
index 6f5d19665cf6..e1a8f107f0a7 100644
--- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
+++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
@@ -76,6 +76,92 @@ __naked int subprog_result_precise(void)
);
}
+__naked __noinline __used
+static unsigned long fp_leaking_subprog()
+{
+ asm volatile (
+ "r0 = (s8)r10;"
+ "exit;"
+ );
+}
+
+__naked __noinline __used
+static unsigned long sneaky_fp_leaking_subprog()
+{
+ asm volatile (
+ "r1 = r10;"
+ "r0 = (s8)r1;"
+ "exit;"
+ );
+}
+
+SEC("?raw_tp")
+__success __log_level(2)
+__msg("6: (0f) r1 += r0")
+__msg("mark_precise: frame0: last_idx 6 first_idx 0 subseq_idx -1")
+__msg("mark_precise: frame0: regs=r0 stack= before 5: (bf) r1 = r6")
+__msg("mark_precise: frame0: regs=r0 stack= before 4: (27) r0 *= 4")
+__msg("mark_precise: frame0: regs=r0 stack= before 3: (57) r0 &= 3")
+__msg("mark_precise: frame0: regs=r0 stack= before 10: (95) exit")
+__msg("mark_precise: frame1: regs=r0 stack= before 9: (bf) r0 = (s8)r10")
+__msg("7: R0_w=scalar")
+__naked int fp_precise_subprog_result(void)
+{
+ asm volatile (
+ "call fp_leaking_subprog;"
+ /* use subprog's returned value (which is derived from r10=fp
+ * register), as index into vals array, forcing all of that to
+ * be known precisely
+ */
+ "r0 &= 3;"
+ "r0 *= 4;"
+ "r1 = %[vals];"
+ /* force precision marking */
+ "r1 += r0;"
+ "r0 = *(u32 *)(r1 + 0);"
+ "exit;"
+ :
+ : __imm_ptr(vals)
+ : __clobber_common
+ );
+}
+
+SEC("?raw_tp")
+__success __log_level(2)
+__msg("6: (0f) r1 += r0")
+__msg("mark_precise: frame0: last_idx 6 first_idx 0 subseq_idx -1")
+__msg("mark_precise: frame0: regs=r0 stack= before 5: (bf) r1 = r6")
+__msg("mark_precise: frame0: regs=r0 stack= before 4: (27) r0 *= 4")
+__msg("mark_precise: frame0: regs=r0 stack= before 3: (57) r0 &= 3")
+__msg("mark_precise: frame0: regs=r0 stack= before 11: (95) exit")
+__msg("mark_precise: frame1: regs=r0 stack= before 10: (bf) r0 = (s8)r1")
+/* here r1 is marked precise, even though it's fp register, but that's fine
+ * because by the time we get out of subprogram it has to be derived from r10
+ * anyways, at which point we'll break precision chain
+ */
+__msg("mark_precise: frame1: regs=r1 stack= before 9: (bf) r1 = r10")
+__msg("7: R0_w=scalar")
+__naked int sneaky_fp_precise_subprog_result(void)
+{
+ asm volatile (
+ "call sneaky_fp_leaking_subprog;"
+ /* use subprog's returned value (which is derived from r10=fp
+ * register), as index into vals array, forcing all of that to
+ * be known precisely
+ */
+ "r0 &= 3;"
+ "r0 *= 4;"
+ "r1 = %[vals];"
+ /* force precision marking */
+ "r1 += r0;"
+ "r0 = *(u32 *)(r1 + 0);"
+ "exit;"
+ :
+ : __imm_ptr(vals)
+ : __clobber_common
+ );
+}
+
SEC("?raw_tp")
__success __log_level(2)
__msg("9: (0f) r1 += r0")
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-02 22:50 ` [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
@ 2024-04-02 23:26 ` Andrii Nakryiko
2024-04-04 18:48 ` Yonghong Song
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2024-04-02 23:26 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, ast, daniel, martin.lau, kernel-team, syzbot+148110ee7cf72f39f33e
On Tue, Apr 2, 2024 at 3:50 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>
> Add selftests validating that BPF verifier handles precision marking
> for SCALAR registers derived from r10 (fp) register correctly.
>
> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> ---
> .../bpf/progs/verifier_subprog_precision.c | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
> index 6f5d19665cf6..e1a8f107f0a7 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
> @@ -76,6 +76,92 @@ __naked int subprog_result_precise(void)
> );
> }
>
> +__naked __noinline __used
> +static unsigned long fp_leaking_subprog()
> +{
> + asm volatile (
> + "r0 = (s8)r10;"
Our CI's clang doesn't like this instruction. I guess I'll have to
encode it in binary form :(
> + "exit;"
> + );
> +}
> +
[...]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-02 23:26 ` Andrii Nakryiko
@ 2024-04-04 18:48 ` Yonghong Song
2024-04-04 20:09 ` Andrii Nakryiko
0 siblings, 1 reply; 6+ messages in thread
From: Yonghong Song @ 2024-04-04 18:48 UTC (permalink / raw)
To: Andrii Nakryiko, Andrii Nakryiko
Cc: bpf, ast, daniel, martin.lau, kernel-team, syzbot+148110ee7cf72f39f33e
On 4/2/24 4:26 PM, Andrii Nakryiko wrote:
> On Tue, Apr 2, 2024 at 3:50 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>> Add selftests validating that BPF verifier handles precision marking
>> for SCALAR registers derived from r10 (fp) register correctly.
>>
>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>> ---
>> .../bpf/progs/verifier_subprog_precision.c | 86 +++++++++++++++++++
>> 1 file changed, 86 insertions(+)
>>
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>> index 6f5d19665cf6..e1a8f107f0a7 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>> @@ -76,6 +76,92 @@ __naked int subprog_result_precise(void)
>> );
>> }
>>
>> +__naked __noinline __used
>> +static unsigned long fp_leaking_subprog()
>> +{
>> + asm volatile (
>> + "r0 = (s8)r10;"
> Our CI's clang doesn't like this instruction. I guess I'll have to
> encode it in binary form :(
This patch disappeared from CI so I am not able to check the result.
But I tried with the following small example.
$ cat t.c
__attribute__((naked)) unsigned long t(void)
{
asm volatile("r0 = (s8)r10;"
"exit;"
);
}
$ clang --target=bpf -O2 -mcpu=v2 -g -c t.c && llvm-objdump -d t.o
t.o: file format elf64-bpf
Disassembly of section .text:
0000000000000000 <t>:
0: bf a0 08 00 00 00 00 00 r0 = (s8)r10
1: 95 00 00 00 00 00 00 00 exit
-mcpu=v3/v4 has the same result.
Not sure what clang complains.
>
>> + "exit;"
>> + );
>> +}
>> +
> [...]
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-04 18:48 ` Yonghong Song
@ 2024-04-04 20:09 ` Andrii Nakryiko
2024-04-04 20:13 ` Yonghong Song
0 siblings, 1 reply; 6+ messages in thread
From: Andrii Nakryiko @ 2024-04-04 20:09 UTC (permalink / raw)
To: Yonghong Song
Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team,
syzbot+148110ee7cf72f39f33e
On Thu, Apr 4, 2024 at 11:48 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>
>
> On 4/2/24 4:26 PM, Andrii Nakryiko wrote:
> > On Tue, Apr 2, 2024 at 3:50 PM Andrii Nakryiko <andrii@kernel.org> wrote:
> >> Add selftests validating that BPF verifier handles precision marking
> >> for SCALAR registers derived from r10 (fp) register correctly.
> >>
> >> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
> >> ---
> >> .../bpf/progs/verifier_subprog_precision.c | 86 +++++++++++++++++++
> >> 1 file changed, 86 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
> >> index 6f5d19665cf6..e1a8f107f0a7 100644
> >> --- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
> >> +++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
> >> @@ -76,6 +76,92 @@ __naked int subprog_result_precise(void)
> >> );
> >> }
> >>
> >> +__naked __noinline __used
> >> +static unsigned long fp_leaking_subprog()
> >> +{
> >> + asm volatile (
> >> + "r0 = (s8)r10;"
> > Our CI's clang doesn't like this instruction. I guess I'll have to
> > encode it in binary form :(
>
> This patch disappeared from CI so I am not able to check the result.
>
> But I tried with the following small example.
>
> $ cat t.c
> __attribute__((naked)) unsigned long t(void)
> {
> asm volatile("r0 = (s8)r10;"
> "exit;"
> );
> }
>
> $ clang --target=bpf -O2 -mcpu=v2 -g -c t.c && llvm-objdump -d t.o
>
You are using local clang built from source code, right? I think our
BPF CI still is on Clang 17 or something, so it doesn't yet understand
"(s8)r10" syntax, unfortunately.
> t.o: file format elf64-bpf
>
> Disassembly of section .text:
>
> 0000000000000000 <t>:
> 0: bf a0 08 00 00 00 00 00 r0 = (s8)r10
> 1: 95 00 00 00 00 00 00 00 exit
>
>
> -mcpu=v3/v4 has the same result.
> Not sure what clang complains.
>
> >
> >> + "exit;"
> >> + );
> >> +}
> >> +
> > [...]
> >
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests
2024-04-04 20:09 ` Andrii Nakryiko
@ 2024-04-04 20:13 ` Yonghong Song
0 siblings, 0 replies; 6+ messages in thread
From: Yonghong Song @ 2024-04-04 20:13 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Andrii Nakryiko, bpf, ast, daniel, martin.lau, kernel-team,
syzbot+148110ee7cf72f39f33e
On 4/4/24 1:09 PM, Andrii Nakryiko wrote:
> On Thu, Apr 4, 2024 at 11:48 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>> On 4/2/24 4:26 PM, Andrii Nakryiko wrote:
>>> On Tue, Apr 2, 2024 at 3:50 PM Andrii Nakryiko <andrii@kernel.org> wrote:
>>>> Add selftests validating that BPF verifier handles precision marking
>>>> for SCALAR registers derived from r10 (fp) register correctly.
>>>>
>>>> Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
>>>> ---
>>>> .../bpf/progs/verifier_subprog_precision.c | 86 +++++++++++++++++++
>>>> 1 file changed, 86 insertions(+)
>>>>
>>>> diff --git a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>>>> index 6f5d19665cf6..e1a8f107f0a7 100644
>>>> --- a/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>>>> +++ b/tools/testing/selftests/bpf/progs/verifier_subprog_precision.c
>>>> @@ -76,6 +76,92 @@ __naked int subprog_result_precise(void)
>>>> );
>>>> }
>>>>
>>>> +__naked __noinline __used
>>>> +static unsigned long fp_leaking_subprog()
>>>> +{
>>>> + asm volatile (
>>>> + "r0 = (s8)r10;"
>>> Our CI's clang doesn't like this instruction. I guess I'll have to
>>> encode it in binary form :(
>> This patch disappeared from CI so I am not able to check the result.
>>
>> But I tried with the following small example.
>>
>> $ cat t.c
>> __attribute__((naked)) unsigned long t(void)
>> {
>> asm volatile("r0 = (s8)r10;"
>> "exit;"
>> );
>> }
>>
>> $ clang --target=bpf -O2 -mcpu=v2 -g -c t.c && llvm-objdump -d t.o
>>
> You are using local clang built from source code, right? I think our
> BPF CI still is on Clang 17 or something, so it doesn't yet understand
> "(s8)r10" syntax, unfortunately.
Yes, it makes sense. Indeed in that case, either using bytes or guarding
with >= llvm18 is needed.
>
>
>> t.o: file format elf64-bpf
>>
>> Disassembly of section .text:
>>
>> 0000000000000000 <t>:
>> 0: bf a0 08 00 00 00 00 00 r0 = (s8)r10
>> 1: 95 00 00 00 00 00 00 00 exit
>>
>>
>> -mcpu=v3/v4 has the same result.
>> Not sure what clang complains.
>>
>>>> + "exit;"
>>>> + );
>>>> +}
>>>> +
>>> [...]
>>>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-04-04 20:13 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-02 22:50 [PATCH bpf-next 1/2] bpf: prevent r10 register from being marked as precise Andrii Nakryiko
2024-04-02 22:50 ` [PATCH bpf-next 2/2] selftests/bpf: add fp-leaking precise subprog result tests Andrii Nakryiko
2024-04-02 23:26 ` Andrii Nakryiko
2024-04-04 18:48 ` Yonghong Song
2024-04-04 20:09 ` Andrii Nakryiko
2024-04-04 20:13 ` Yonghong Song
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.