Skip to contentSkip to conversation

arm64: bpf: Fix UBSAN misaligned access in BPF JIT

PATCH In review Fuad Tabba 2 participants 3 messages Feb 24 – Feb 25

Subsystems (3)
BPF arm64 BPF JIT for arm64
lore
Share & tools
lore

Fuad Tabba

author
#0 lore raw

PATCH 1/1 arm64: bpf: Fix UBSAN misaligned access in BPF JIT

struct bpf_plt contains a u64 'target' field. The BPF JIT allocator was using an alignment of 4 bytes (sizeof(u32)), which could lead to the 'target' field being misaligned in the JIT buffer.

Increase the alignment requirement to 8 bytes (sizeof(u64)) in bpf_jit_binary_pack_alloc() to guarantee proper alignment for struct bpf_plt.

Fixes b2ad54e1533e9 ("bpf, arm64: Implement bpf_arch_text_poke() for arm64")
Signed-off-by Fuad Tabba <[email protected]>
---
 arch/arm64/net/bpf_jit_comp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/net/bpf_jit_comp.c +1 -1
index 356d33c7a4ae..adf84962d579 100644
@@ -2119,7 +2119,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	extable_offset = round_up(prog_size + PLT_TARGET_SIZE, extable_align);
 	image_size = extable_offset + extable_size;
 	ro_header = bpf_jit_binary_pack_alloc(image_size, &ro_image_ptr,
-					      sizeof(u32), &header, &image_ptr,
+					      sizeof(u64), &header, &image_ptr,
 					      jit_fill_hole);
 	if (!ro_header) {
 		prog = orig_prog;
Signature · 1 line
2.53.0.371.g1d285c8824-goog
maintainer
#1 lore raw
Quoted 19 lines · Fuad Tabba

struct bpf_plt contains a u64 'target' field. The BPF JIT allocator was using an alignment of 4 bytes (sizeof(u32)), which could lead to the 'target' field being misaligned in the JIT buffer.

Increase the alignment requirement to 8 bytes (sizeof(u64)) in bpf_jit_binary_pack_alloc() to guarantee proper alignment for struct bpf_plt.

Fixes b2ad54e1533e9 ("bpf, arm64: Implement bpf_arch_text_poke() for arm64")
Signed-off-by Fuad Tabba <[email protected]>
---
 arch/arm64/net/bpf_jit_comp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/arm64/net/bpf_jit_comp.c +0 -0
index 356d33c7a4ae..adf84962d579 100644

arch/arm64/net/bpf_jit_comp.c struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) · line 2123

Quoted 6 lines
arch/arm64/net/bpf_jit_comp.c +1 -1
@@ -2119,7 +2119,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
 	extable_offset = round_up(prog_size + PLT_TARGET_SIZE, extable_align);
 	image_size = extable_offset + extable_size;
 	ro_header = bpf_jit_binary_pack_alloc(image_size, &ro_image_ptr,
-					      sizeof(u32), &header, &image_ptr,
+					      sizeof(u64), &header, &image_ptr,
 					      jit_fill_hole);

Did you see a functional issue with this or are you just trying to squash a UBSAN splat? I can't see an issue with the code and it seems a bit over-the-top to over-align the whole JIT buffer just because of structure alignment rules that don't really make sense for the actual PLT.

Does marking 'struct bpf_plt' as __packed help?

Will

author
#2 lore raw

Hi Will,

Quoted 16 lines · Will Deacon
Quoted 14 lines · Fuad Tabba

struct bpf_plt contains a u64 'target' field. The BPF JIT allocator was using an alignment of 4 bytes (sizeof(u32)), which could lead to the 'target' field being misaligned in the JIT buffer.

Increase the alignment requirement to 8 bytes (sizeof(u64)) in bpf_jit_binary_pack_alloc() to guarantee proper alignment for struct bpf_plt.

Fixes b2ad54e1533e9 ("bpf, arm64: Implement bpf_arch_text_poke() for arm64")
Signed-off-by Fuad Tabba <[email protected]>
---
 arch/arm64/net/bpf_jit_comp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 16 lines

arch/arm64/net/bpf_jit_comp.c struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog) · line 2123

Quoted 11 lines
arch/arm64/net/bpf_jit_comp.c +1 -1
index 356d33c7a4ae..adf84962d579 100644
@@ -2119,7 +2119,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *prog)
      extable_offset = round_up(prog_size + PLT_TARGET_SIZE, extable_align);
      image_size = extable_offset + extable_size;
      ro_header = bpf_jit_binary_pack_alloc(image_size, &ro_image_ptr,
-                                           sizeof(u32), &header, &image_ptr,
+                                           sizeof(u64), &header, &image_ptr,
                                            jit_fill_hole);

Did you see a functional issue with this or are you just trying to squash a UBSAN splat? I can't see an issue with the code and it seems a bit over-the-top to over-align the whole JIT buffer just because of structure alignment rules that don't really make sense for the actual PLT.

I didn't see a functional issue. I was debugging something else, and ran into the UBSAN splat.

Quoted 1 line

Does marking 'struct bpf_plt' as __packed help?

It does. It drops the compiler's alignment assumption silencing the UBSAN warning without touching the allocator, and the generated assembly is identical. I'll respin a v2 with that fix instead.

Thanks,
/fuad

Quoted 2 lines

Will