mirror of
https://github.com/torvalds/linux.git
synced 2026-06-05 13:06:59 +02:00
de58ef414d
llvm patch [1] enabled cross-function optimization for func arguments
(ArgumentPromotion) at -O2 level. And this caused s390 sock_fields
test failure ([2]). The failure is gone right now as patch [1] was
reverted in [3]. But it is possible that patch [3] will be reverted
again and then the test failure in [2] will show up again. So it is
desirable to fix the failure regardless.
The following is an analysis why sock_field test fails with
llvm patch [1].
The main problem is in
static __noinline bool sk_dst_port__load_word(struct bpf_sock *sk)
{
__u32 *word = (__u32 *)&sk->dst_port;
return word[0] == bpf_htons(0xcafe);
}
static __noinline bool sk_dst_port__load_half(struct bpf_sock *sk)
{
__u16 *half = (__u16 *)&sk->dst_port;
return half[0] == bpf_htons(0xcafe);
}
...
int read_sk_dst_port(struct __sk_buff *skb)
{
...
sk = skb->sk;
...
if (!sk_dst_port__load_word(sk))
RET_LOG();
if (!sk_dst_port__load_half(sk))
RET_LOG();
...
}
Through some cross-function optimization by ArgumentPromotion
optimization, the compiler does:
static __noinline bool sk_dst_port__load_word(__u32 word_val)
{
return word_val == bpf_htons(0xcafe);
}
static __noinline bool sk_dst_port__load_half(__u16 half_val)
{
return half_val == bpf_htons(0xcafe);
}
...
int read_sk_dst_port(struct __sk_buff *skb)
{
...
sk = skb->sk;
...
__u32 *word = (__u32 *)&sk->dst_port;
__u32 word_val = word[0];
...
if (!sk_dst_port__load_word(word_val))
RET_LOG();
__u16 half_val = word_val >> 16;
if (!sk_dst_port__load_half(half_val))
RET_LOG();
...
}
In current uapi bpf.h, we have
struct bpf_sock {
...
__be16 dst_port; /* network byte order */
__u16 :16; /* zero padding */
...
};
But the old kernel (e.g., 5.6) we have
struct bpf_sock {
...
__u32 dst_port; /* network byte order */
...
};
So for backward compatability reason, 4-byte load of
dst_port is converted to 2-byte load internally.
Specifically, 'word_val = word[0]' is replaced by 2-byte load
by the verifier and this caused the trouble for later
sk_dst_port__load_half() where half_val becomes 0.
Typical usr program won't have such a code pattern tiggering
the above bug, so let us fix the test failure with source
code change. Adding an empty asm volatile statement seems
enough to prevent undesired transformation.
[1] https://reviews.llvm.org/D148269
[2] https://lore.kernel.org/bpf/e7f2c5e8-a50c-198d-8f95-388165f1e4fd@meta.com/
[3] https://reviews.llvm.org/rG141be5c062ecf22bd287afffd310e8ac4711444a
Tested-by: Ilya Leoshkevich <iii@linux.ibm.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
Link: https://lore.kernel.org/r/20230516214945.1013578-1-yhs@fb.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
|
||
|---|---|---|
| .. | ||
| alsa | ||
| amd-pstate | ||
| arm64 | ||
| bpf | ||
| breakpoints | ||
| capabilities | ||
| cgroup | ||
| clone3 | ||
| core | ||
| cpu-hotplug | ||
| cpufreq | ||
| damon | ||
| dma | ||
| dmabuf-heaps | ||
| drivers | ||
| efivarfs | ||
| exec | ||
| filesystems | ||
| firmware | ||
| fpu | ||
| ftrace | ||
| futex | ||
| gpio | ||
| hid | ||
| ia64 | ||
| intel_pstate | ||
| iommu | ||
| ipc | ||
| ir | ||
| kcmp | ||
| kexec | ||
| kmod | ||
| kselftest | ||
| kvm | ||
| landlock | ||
| lib | ||
| livepatch | ||
| lkdtm | ||
| locking | ||
| media_tests | ||
| membarrier | ||
| memfd | ||
| memory-hotplug | ||
| mincore | ||
| mm | ||
| mount | ||
| mount_setattr | ||
| move_mount_set_group | ||
| mqueue | ||
| nci | ||
| net | ||
| netfilter | ||
| nolibc | ||
| nsfs | ||
| ntb | ||
| openat2 | ||
| perf_events | ||
| pid_namespace | ||
| pidfd | ||
| powerpc | ||
| prctl | ||
| proc | ||
| pstore | ||
| ptp | ||
| ptrace | ||
| rcutorture | ||
| resctrl | ||
| riscv | ||
| rlimits | ||
| rseq | ||
| rtc | ||
| safesetid | ||
| sched | ||
| seccomp | ||
| sgx | ||
| sigaltstack | ||
| size | ||
| sparc64 | ||
| splice | ||
| static_keys | ||
| sync | ||
| syscall_user_dispatch | ||
| sysctl | ||
| tc-testing | ||
| tdx | ||
| timens | ||
| timers | ||
| tmpfs | ||
| tpm2 | ||
| uevent | ||
| user | ||
| user_events | ||
| vDSO | ||
| watchdog | ||
| wireguard | ||
| x86 | ||
| zram | ||
| .gitignore | ||
| gen_kselftest_tar.sh | ||
| kselftest_deps.sh | ||
| kselftest_harness.h | ||
| kselftest_install.sh | ||
| kselftest_module.h | ||
| kselftest.h | ||
| lib.mk | ||
| Makefile | ||
| run_kselftest.sh | ||