[PATCH 0/2] riscv, bpf: fix reads of thread_info.cpu

[PATCH 0/2] riscv, bpf: fix reads of thread_info.cpu

[Radim Krčmář]

Hello,

These patches are related to a recently queued series [1] that fixes the
same bugs in normal code.  That series finishes with a patch that would
have exposed the BPF bugs, but luckily it won't get merged until v6.18.

I don't know enough about BPF to verify that it emits the correct code
now, so any pointers are welcome.

1: https://lore.kernel.org/linux-riscv/20250725165410.2896641-3-rkrcmar@ventanamicro.com/

Radim Krčmář (2):
  riscv, bpf: use lw when reading int cpu in BPF_MOV64_PERCPU_REG
  riscv, bpf: use lw when reading int cpu in bpf_get_smp_processor_id

 arch/riscv/net/bpf_jit_comp64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.50.0

[PATCH 1/2] riscv, bpf: use lw when reading int cpu in BPF_MOV64_PERCPU_REG

[Radim Krčmář]

emit_ld is wrong, because thread_info.cpu is 32-bit, not xlen-bit wide.
The struct currently has a hole after cpu, so little endian accesses
seemed fine.

Fixes: 19c56d4e5be1 ("riscv, bpf: add internal-only MOV instruction to resolve per-CPU addrs")
Cc: <stable@vger.kernel.org>
Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
---
 arch/riscv/net/bpf_jit_comp64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 10e01ff06312..6e1554d89681 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1356,7 +1356,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 				emit_mv(rd, rs, ctx);
 #ifdef CONFIG_SMP
 			/* Load current CPU number in T1 */
-			emit_ld(RV_REG_T1, offsetof(struct thread_info, cpu),
+			emit_lw(RV_REG_T1, offsetof(struct thread_info, cpu),
 				RV_REG_TP, ctx);
 			/* Load address of __per_cpu_offset array in T2 */
 			emit_addr(RV_REG_T2, (u64)&__per_cpu_offset, extra_pass, ctx);
-- 
2.50.0

[PATCH 2/2] riscv, bpf: use lw when reading int cpu in bpf_get_smp_processor_id

[Radim Krčmář]

emit_ld is wrong, because thread_info.cpu is 32-bit, not xlen-bit wide.
The struct currently has a hole after cpu, so little endian accesses
seemed fine.

Fixes: 2ddec2c80b44 ("riscv, bpf: inline bpf_get_smp_processor_id()")
Cc: <stable@vger.kernel.org>
Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
---
 arch/riscv/net/bpf_jit_comp64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
index 6e1554d89681..9883a55d61b5 100644
--- a/arch/riscv/net/bpf_jit_comp64.c
+++ b/arch/riscv/net/bpf_jit_comp64.c
@@ -1763,7 +1763,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
 		 */
 		if (insn->src_reg == 0 && insn->imm == BPF_FUNC_get_smp_processor_id) {
 			/* Load current CPU number in R0 */
-			emit_ld(bpf_to_rv_reg(BPF_REG_0, ctx), offsetof(struct thread_info, cpu),
+			emit_lw(bpf_to_rv_reg(BPF_REG_0, ctx), offsetof(struct thread_info, cpu),
 				RV_REG_TP, ctx);
 			break;
 		}
-- 
2.50.0

Re: [PATCH 0/2] riscv, bpf: fix reads of thread_info.cpu

[Alexandre Ghiti]

Hi Radim,

On 8/12/25 11:02, Radim Krčmář wrote:
> Hello,
>
> These patches are related to a recently queued series [1] that fixes the
> same bugs in normal code.  That series finishes with a patch that would
> have exposed the BPF bugs, but luckily it won't get merged until v6.18.
>
> I don't know enough about BPF to verify that it emits the correct code
> now, so any pointers are welcome.
>
> 1: https://lore.kernel.org/linux-riscv/20250725165410.2896641-3-rkrcmar@ventanamicro.com/
>
> Radim Krčmář (2):
>    riscv, bpf: use lw when reading int cpu in BPF_MOV64_PERCPU_REG
>    riscv, bpf: use lw when reading int cpu in bpf_get_smp_processor_id
>
>   arch/riscv/net/bpf_jit_comp64.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>

Both patches look good so:

Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Since this only touches riscv and I have a bunch of fixes pending, I 
propose to take those patches through the riscv tree, I'll just wait for 
Björn to confirm it is correct.

@Radim: This is the third similar bug, did you check all assembly code 
(and bpf) to make sure we don't have anymore left or should I?

Thanks,

Alex

Re: [PATCH 0/2] riscv, bpf: fix reads of thread_info.cpu

[Radim Krčmář]

2025-08-12T13:37:16+02:00, Alexandre Ghiti <alex@ghiti.fr>:
> @Radim: This is the third similar bug, did you check all assembly code 
> (and bpf) to make sure we don't have anymore left or should I?

I looked at load/store instructions, including bpf, and focussed on
patterns where we access non-xlen sized data through an offset.

(Nothing else popped up, but I mostly used grep and cscope as I don't
 know of any semantic tool, so my confidence levels are low.)

Re: [PATCH 0/2] riscv, bpf: fix reads of thread_info.cpu

[Alexandre Ghiti]

Hi Radim,

On 8/12/25 15:09, Radim Krčmář wrote:
> 2025-08-12T13:37:16+02:00, Alexandre Ghiti <alex@ghiti.fr>:
>> @Radim: This is the third similar bug, did you check all assembly code
>> (and bpf) to make sure we don't have anymore left or should I?
> I looked at load/store instructions, including bpf, and focussed on
> patterns where we access non-xlen sized data through an offset.
>
> (Nothing else popped up, but I mostly used grep and cscope as I don't
>   know of any semantic tool, so my confidence levels are low.)


Ok thanks, I don't have any better idea than eye scrubbing so I'll take 
another look to improve our confidence. One good thing is that now I 
don't let them pass in reviews :)

Thanks for noticing this class of bugs!

Alex


>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH 0/2] riscv, bpf: fix reads of thread_info.cpu

[Björn Töpel]

Radim!

Radim Krčmář <rkrcmar@ventanamicro.com> writes:

> Hello,
>
> These patches are related to a recently queued series [1] that fixes the
> same bugs in normal code.  That series finishes with a patch that would
> have exposed the BPF bugs, but luckily it won't get merged until v6.18.
>
> I don't know enough about BPF to verify that it emits the correct code
> now, so any pointers are welcome.
>
> 1: https://lore.kernel.org/linux-riscv/20250725165410.2896641-3-rkrcmar@ventanamicro.com/

Apologies for the slow review!

For the series:

Acked-by: Björn Töpel <bjorn@kernel.org>
Tested-by: Björn Töpel <bjorn@rivosinc.com> # QEMU

Re: [PATCH 1/2] riscv, bpf: use lw when reading int cpu in BPF_MOV64_PERCPU_REG

[Pu Lehui]

On 2025/8/12 17:02, Radim Krčmář wrote:
> emit_ld is wrong, because thread_info.cpu is 32-bit, not xlen-bit wide.
> The struct currently has a hole after cpu, so little endian accesses
> seemed fine.
> 
> Fixes: 19c56d4e5be1 ("riscv, bpf: add internal-only MOV instruction to resolve per-CPU addrs")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
> ---
>   arch/riscv/net/bpf_jit_comp64.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 10e01ff06312..6e1554d89681 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1356,7 +1356,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   				emit_mv(rd, rs, ctx);
>   #ifdef CONFIG_SMP
>   			/* Load current CPU number in T1 */
> -			emit_ld(RV_REG_T1, offsetof(struct thread_info, cpu),
> +			emit_lw(RV_REG_T1, offsetof(struct thread_info, cpu),
>   				RV_REG_TP, ctx);
>   			/* Load address of __per_cpu_offset array in T2 */
>   			emit_addr(RV_REG_T2, (u64)&__per_cpu_offset, extra_pass, ctx);

Reviewed-by: Pu Lehui <pulehui@huawei.com>

Re: [PATCH 2/2] riscv, bpf: use lw when reading int cpu in bpf_get_smp_processor_id

[Pu Lehui]

On 2025/8/12 17:02, Radim Krčmář wrote:
> emit_ld is wrong, because thread_info.cpu is 32-bit, not xlen-bit wide.
> The struct currently has a hole after cpu, so little endian accesses
> seemed fine.
> 
> Fixes: 2ddec2c80b44 ("riscv, bpf: inline bpf_get_smp_processor_id()")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
> ---
>   arch/riscv/net/bpf_jit_comp64.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/net/bpf_jit_comp64.c b/arch/riscv/net/bpf_jit_comp64.c
> index 6e1554d89681..9883a55d61b5 100644
> --- a/arch/riscv/net/bpf_jit_comp64.c
> +++ b/arch/riscv/net/bpf_jit_comp64.c
> @@ -1763,7 +1763,7 @@ int bpf_jit_emit_insn(const struct bpf_insn *insn, struct rv_jit_context *ctx,
>   		 */
>   		if (insn->src_reg == 0 && insn->imm == BPF_FUNC_get_smp_processor_id) {
>   			/* Load current CPU number in R0 */
> -			emit_ld(bpf_to_rv_reg(BPF_REG_0, ctx), offsetof(struct thread_info, cpu),
> +			emit_lw(bpf_to_rv_reg(BPF_REG_0, ctx), offsetof(struct thread_info, cpu),
>   				RV_REG_TP, ctx);
>   			break;
>   		}

Reviewed-by: Pu Lehui <pulehui@huawei.com>