Skip to content

Commit 7ded842

Browse files
iii-iAlexei Starovoitov
authored andcommitted
s390/bpf: Fix bpf_plt pointer arithmetic
Kui-Feng Lee reported a crash on s390x triggered by the dummy_st_ops/dummy_init_ptr_arg test [1]: [<0000000000000002>] 0x2 [<00000000009d5cde>] bpf_struct_ops_test_run+0x156/0x250 [<000000000033145a>] __sys_bpf+0xa1a/0xd00 [<00000000003319dc>] __s390x_sys_bpf+0x44/0x50 [<0000000000c4382c>] __do_syscall+0x244/0x300 [<0000000000c59a40>] system_call+0x70/0x98 This is caused by GCC moving memcpy() after assignments in bpf_jit_plt(), resulting in NULL pointers being written instead of the return and the target addresses. Looking at the GCC internals, the reordering is allowed because the alias analysis thinks that the memcpy() destination and the assignments' left-hand-sides are based on different objects: new_plt and bpf_plt_ret/bpf_plt_target respectively, and therefore they cannot alias. This is in turn due to a violation of the C standard: When two pointers are subtracted, both shall point to elements of the same array object, or one past the last element of the array object ... From the C's perspective, bpf_plt_ret and bpf_plt are distinct objects and cannot be subtracted. In the practical terms, doing so confuses the GCC's alias analysis. The code was written this way in order to let the C side know a few offsets defined in the assembly. While nice, this is by no means necessary. Fix the noncompliance by hardcoding these offsets. [1] https://lore.kernel.org/bpf/c9923c1d-971d-4022-8dc8-1364e929d34c@gmail.com/ Fixes: f1d5df8 ("s390/bpf: Implement bpf_arch_text_poke()") Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> Message-ID: <20240320015515.11883-1-iii@linux.ibm.com> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
1 parent f6e9223 commit 7ded842

1 file changed

Lines changed: 20 additions & 26 deletions

File tree

arch/s390/net/bpf_jit_comp.c

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -516,11 +516,12 @@ static void bpf_skip(struct bpf_jit *jit, int size)
516516
* PLT for hotpatchable calls. The calling convention is the same as for the
517517
* ftrace hotpatch trampolines: %r0 is return address, %r1 is clobbered.
518518
*/
519-
extern const char bpf_plt[];
520-
extern const char bpf_plt_ret[];
521-
extern const char bpf_plt_target[];
522-
extern const char bpf_plt_end[];
523-
#define BPF_PLT_SIZE 32
519+
struct bpf_plt {
520+
char code[16];
521+
void *ret;
522+
void *target;
523+
} __packed;
524+
extern const struct bpf_plt bpf_plt;
524525
asm(
525526
".pushsection .rodata\n"
526527
" .balign 8\n"
@@ -531,15 +532,14 @@ asm(
531532
" .balign 8\n"
532533
"bpf_plt_ret: .quad 0\n"
533534
"bpf_plt_target: .quad 0\n"
534-
"bpf_plt_end:\n"
535535
" .popsection\n"
536536
);
537537

538-
static void bpf_jit_plt(void *plt, void *ret, void *target)
538+
static void bpf_jit_plt(struct bpf_plt *plt, void *ret, void *target)
539539
{
540-
memcpy(plt, bpf_plt, BPF_PLT_SIZE);
541-
*(void **)((char *)plt + (bpf_plt_ret - bpf_plt)) = ret;
542-
*(void **)((char *)plt + (bpf_plt_target - bpf_plt)) = target ?: ret;
540+
memcpy(plt, &bpf_plt, sizeof(*plt));
541+
plt->ret = ret;
542+
plt->target = target;
543543
}
544544

545545
/*
@@ -662,9 +662,9 @@ static void bpf_jit_epilogue(struct bpf_jit *jit, u32 stack_depth)
662662
jit->prg = ALIGN(jit->prg, 8);
663663
jit->prologue_plt = jit->prg;
664664
if (jit->prg_buf)
665-
bpf_jit_plt(jit->prg_buf + jit->prg,
665+
bpf_jit_plt((struct bpf_plt *)(jit->prg_buf + jit->prg),
666666
jit->prg_buf + jit->prologue_plt_ret, NULL);
667-
jit->prg += BPF_PLT_SIZE;
667+
jit->prg += sizeof(struct bpf_plt);
668668
}
669669

670670
static int get_probe_mem_regno(const u8 *insn)
@@ -2040,9 +2040,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
20402040
struct bpf_jit jit;
20412041
int pass;
20422042

2043-
if (WARN_ON_ONCE(bpf_plt_end - bpf_plt != BPF_PLT_SIZE))
2044-
return orig_fp;
2045-
20462043
if (!fp->jit_requested)
20472044
return orig_fp;
20482045

@@ -2148,14 +2145,11 @@ bool bpf_jit_supports_far_kfunc_call(void)
21482145
int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
21492146
void *old_addr, void *new_addr)
21502147
{
2148+
struct bpf_plt expected_plt, current_plt, new_plt, *plt;
21512149
struct {
21522150
u16 opc;
21532151
s32 disp;
21542152
} __packed insn;
2155-
char expected_plt[BPF_PLT_SIZE];
2156-
char current_plt[BPF_PLT_SIZE];
2157-
char new_plt[BPF_PLT_SIZE];
2158-
char *plt;
21592153
char *ret;
21602154
int err;
21612155

@@ -2174,18 +2168,18 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type t,
21742168
*/
21752169
} else {
21762170
/* Verify the PLT. */
2177-
plt = (char *)ip + (insn.disp << 1);
2178-
err = copy_from_kernel_nofault(current_plt, plt, BPF_PLT_SIZE);
2171+
plt = ip + (insn.disp << 1);
2172+
err = copy_from_kernel_nofault(&current_plt, plt,
2173+
sizeof(current_plt));
21792174
if (err < 0)
21802175
return err;
21812176
ret = (char *)ip + 6;
2182-
bpf_jit_plt(expected_plt, ret, old_addr);
2183-
if (memcmp(current_plt, expected_plt, BPF_PLT_SIZE))
2177+
bpf_jit_plt(&expected_plt, ret, old_addr);
2178+
if (memcmp(&current_plt, &expected_plt, sizeof(current_plt)))
21842179
return -EINVAL;
21852180
/* Adjust the call address. */
2186-
bpf_jit_plt(new_plt, ret, new_addr);
2187-
s390_kernel_write(plt + (bpf_plt_target - bpf_plt),
2188-
new_plt + (bpf_plt_target - bpf_plt),
2181+
bpf_jit_plt(&new_plt, ret, new_addr);
2182+
s390_kernel_write(&plt->target, &new_plt.target,
21892183
sizeof(void *));
21902184
}
21912185

0 commit comments

Comments
 (0)