[bpf-next,0/3] bpf: Perform necessary sign/zero extension for kfunc return values

Message

Yonghong Song Aug. 7, 2022, 5:51 p.m. UTC

Tejun reported a bpf program kfunc return value mis-handling which
may cause incorrect result. If the kfunc return value is boolean
or u8, the bpf program produce incorrect results.

The main reason is due to mismatch of return value expectation between
native architecture and bpf. For example, for x86_64, if a kfunc
returns a u8, the kfunc returns 64-bit %rax, the top 56 bits might
be garbage. This is okay if the caller is x86_64 as the caller can
use special instruction to access lower 8-bit register %al. But this
will cause a problem for bpf program since bpf program assumes
the whole r0 register should contain correct value.
This patch set fixed the issue by doing necessary zero/sign extension
for the kfunc return value to meet bpf requirement.

For the rest of patches, Patch 1 is a preparation patch. Patch 2
implemented kernel support to perform necessary zero/sign extension
for kfunc return value. Patch 3 added two tests, one with return type
u8 and another with s16.

Yonghong Song (3):
  bpf: Always return corresponding btf_type in __get_type_size()
  bpf: Perform necessary sign/zero extension for kfunc return values
  selftests/bpf: Add tests with u8/s16 kfunc return types

 include/linux/bpf.h                           |  2 ++
 kernel/bpf/btf.c                              | 18 +++++++---
 kernel/bpf/verifier.c                         | 35 +++++++++++++++++--
 net/bpf/test_run.c                            | 12 +++++++
 .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
 .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++
 6 files changed, 102 insertions(+), 7 deletions(-)

Comments

Andrii Nakryiko Aug. 8, 2022, 11:22 p.m. UTC | #1

On Sun, Aug 7, 2022 at 10:51 AM Yonghong Song <[e-mail]> wrote:
>
> Tejun reported a bpf program kfunc return value mis-handling which
> may cause incorrect result. If the kfunc return value is boolean
> or u8, the bpf program produce incorrect results.
>
> The main reason is due to mismatch of return value expectation between
> native architecture and bpf. For example, for x86_64, if a kfunc
> returns a u8, the kfunc returns 64-bit %rax, the top 56 bits might
> be garbage. This is okay if the caller is x86_64 as the caller can
> use special instruction to access lower 8-bit register %al. But this
> will cause a problem for bpf program since bpf program assumes
> the whole r0 register should contain correct value.
> This patch set fixed the issue by doing necessary zero/sign extension
> for the kfunc return value to meet bpf requirement.
>
> For the rest of patches, Patch 1 is a preparation patch. Patch 2
> implemented kernel support to perform necessary zero/sign extension
> for kfunc return value. Patch 3 added two tests, one with return type
> u8 and another with s16.
>
> Yonghong Song (3):
>   bpf: Always return corresponding btf_type in __get_type_size()
>   bpf: Perform necessary sign/zero extension for kfunc return values
>   selftests/bpf: Add tests with u8/s16 kfunc return types
>
>  include/linux/bpf.h                           |  2 ++
>  kernel/bpf/btf.c                              | 18 +++++++---
>  kernel/bpf/verifier.c                         | 35 +++++++++++++++++--
>  net/bpf/test_run.c                            | 12 +++++++
>  .../selftests/bpf/prog_tests/kfunc_call.c     | 10 ++++++
>  .../selftests/bpf/progs/kfunc_call_test.c     | 32 +++++++++++++++++
>  6 files changed, 102 insertions(+), 7 deletions(-)
>
> --
> 2.30.2
>

LGTM.

Acked-by: Andrii Nakryiko <[e-mail]>

[e-mail] Aug. 9, 2022, 5:40 p.m. UTC | #2

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <[e-mail]>:

On Sun, 7 Aug 2022 10:51:11 -0700 you wrote:
> Tejun reported a bpf program kfunc return value mis-handling which
> may cause incorrect result. If the kfunc return value is boolean
> or u8, the bpf program produce incorrect results.
> 
> The main reason is due to mismatch of return value expectation between
> native architecture and bpf. For example, for x86_64, if a kfunc
> returns a u8, the kfunc returns 64-bit %rax, the top 56 bits might
> be garbage. This is okay if the caller is x86_64 as the caller can
> use special instruction to access lower 8-bit register %al. But this
> will cause a problem for bpf program since bpf program assumes
> the whole r0 register should contain correct value.
> This patch set fixed the issue by doing necessary zero/sign extension
> for the kfunc return value to meet bpf requirement.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/3] bpf: Always return corresponding btf_type in __get_type_size()
    https://git.kernel.org/bpf/bpf-next/c/a00ed8430199
  - [bpf-next,2/3] bpf: Perform necessary sign/zero extension for kfunc return values
    (no matching commit)
  - [bpf-next,3/3] selftests/bpf: Add tests with u8/s16 kfunc return types
    (no matching commit)

You are awesome, thank you!