diff mbox series

[bpf-next,2/2] selftests/bpf: add fp-leaking precise subprog result tests

Message ID 20240402225020.2582397-2-andrii@kernel.org (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf-next,1/2] bpf: prevent r10 register from being marked as precise | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-13 fail Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-4 fail Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-10 success Logs for s390x-gcc / test
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-9 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-8 fail Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-14 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-11 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-18 fail Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18 and -O2 optimization
bpf/vmtest-bpf-next-VM_Test-12 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-15 success Logs for x86_64-gcc / test
bpf/vmtest-bpf-next-VM_Test-17 fail Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-llvm-17 / test
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 8 this patch: 8
netdev/build_tools success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 12 maintainers not CCed: john.fastabend@gmail.com sdf@google.com kpsingh@kernel.org martin.lau@linux.dev yonghong.song@linux.dev shuah@kernel.org linux-kselftest@vger.kernel.org mykolal@fb.com haoluo@google.com jolsa@kernel.org song@kernel.org eddyz87@gmail.com
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 8 this patch: 8
netdev/checkpatch fail CHECK: Lines should not end with a '(' ERROR: Bad function definition - unsigned long fp_leaking_subprog() should probably be unsigned long fp_leaking_subprog(void) ERROR: Bad function definition - unsigned long sneaky_fp_leaking_subprog() should probably be unsigned long sneaky_fp_leaking_subprog(void)
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18

Commit Message

Andrii Nakryiko April 2, 2024, 10:50 p.m. UTC 
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(+)

Comments

Andrii Nakryiko April 2, 2024, 11:26 p.m. UTC | #1 
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;"
> +       );
> +}
> +

[...]
Yonghong Song April 4, 2024, 6:48 p.m. UTC | #2 
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;"
>> +       );
>> +}
>> +
> [...]
>
Andrii Nakryiko April 4, 2024, 8:09 p.m. UTC | #3 
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;"
> >> +       );
> >> +}
> >> +
> > [...]
> >
Yonghong Song April 4, 2024, 8:13 p.m. UTC | #4 
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;"
>>>> +       );
>>>> +}
>>>> +
>>> [...]
>>>
diff mbox series

Patch

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")