* [RFC bpf-next 0/2] bpf, mips: Add BPF_MOVSX support to the JITs
@ 2026-08-19 1:05 Nicholas Dudar
2026-08-19 1:05 ` [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers Nicholas Dudar
2026-08-19 1:05 ` [RFC bpf-next 2/2] bpf, mips: Add support for BPF_MOVSX in the JITs Nicholas Dudar
0 siblings, 2 replies; 4+ messages in thread
From: Nicholas Dudar @ 2026-08-19 1:05 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai, johan.almbladh,
paulburton, tsbogend, linux-mips, linux-kernel
bpf-next is closed for the merge window, so I am sending this for
review. I plan to post this series after it reopens.
The MIPS32 and MIPS64 JITs lower register BPF_MOVSX as an ordinary
move because their register-move paths do not interpret insn->off.
Negative low-width values therefore retain incorrect upper bits.
First factor ordinary register moves into backend-local 32- and 64-bit
helpers. Then handle the BPF-defined MOVSX widths from insn->off in
those helpers. This keeps the verifier-inserted ALU32 zero extension on
its existing path and preserves the MIPS32 register-pair handling. The
implementation uses shift sequences rather than the R2+ seb/seh
instructions so it also works on base MIPS32.
MOVSX can be applied before or after the in-flight SDIV/SMOD series.
Both must precede MEMSX (yet to be sent). The ordering discussion is
here:
Link: https://lore.kernel.org/bpf/CAJZwKkis=3NGw9At0WfiZaRYEoMPbQfqJKN0e+vtRDvp4VY5Ng@mail.gmail.com/
Patch 1 did not change any test_bpf result. On the four little-endian
profiles where the complete suite was run at each boundary, patch 2
fixed the ALU32 byte and halfword MOVSX cases and the ALU64 byte,
halfword, and word cases. The 32-bit totals moved from 1031 passes and
31 failures to 1036 passes and 26 failures; the 64-bit totals moved
from 1030 passes and 31 failures to 1035 passes and 26 failures. Patch
2 did not change any other test result. Ordinary MOV and zero-extension
controls passed at each boundary.
Selector-focused QEMU testing covered MIPS32 base, R2, and R6 plus
pre-R6 MIPS64, each in big- and little-endian configurations. No
physical MIPS hardware was tested.
Nicholas Dudar (2):
bpf, mips: Factor register moves into helpers
bpf, mips: Add support for BPF_MOVSX in the JITs
arch/mips/net/bpf_jit_comp32.c | 54 +++++++++++++++++++++++++++++++---
arch/mips/net/bpf_jit_comp64.c | 48 ++++++++++++++++++++++++++++--
2 files changed, 95 insertions(+), 7 deletions(-)
base-commit: 6655c409707ec8ce9ce0850ffe4fe02331fd4d9c
^ permalink raw reply [flat|nested] 4+ messages in thread
* [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers
2026-08-19 1:05 [RFC bpf-next 0/2] bpf, mips: Add BPF_MOVSX support to the JITs Nicholas Dudar
@ 2026-08-19 1:05 ` Nicholas Dudar
2026-08-19 1:16 ` sashiko-bot
2026-08-19 1:05 ` [RFC bpf-next 2/2] bpf, mips: Add support for BPF_MOVSX in the JITs Nicholas Dudar
1 sibling, 1 reply; 4+ messages in thread
From: Nicholas Dudar @ 2026-08-19 1:05 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai, johan.almbladh,
paulburton, tsbogend, linux-mips, linux-kernel
Both MIPS JITs currently lower register MOV instructions directly in
build_insn(), duplicating backend-specific handling for ALU32 and ALU64
destinations.
Introduce 32-bit and 64-bit register-move helpers in each backend and
route the existing MOV paths through them. Keep the verifier-inserted
zero-extension marker on its dedicated path, so this is a
behavior-preserving refactor.
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
arch/mips/net/bpf_jit_comp32.c | 22 ++++++++++++++++++----
arch/mips/net/bpf_jit_comp64.c | 18 +++++++++++++++---
2 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
index 40a878b672f5d..bfe73b023983e 100644
--- a/arch/mips/net/bpf_jit_comp32.c
+++ b/arch/mips/net/bpf_jit_comp32.c
@@ -190,6 +190,22 @@ static void emit_zext_ver(struct jit_context *ctx, const u8 dst[])
}
}
+/* Register move operation (32-bit) */
+static void emit_mov_r32(struct jit_context *ctx, const u8 dst[],
+ const u8 src[])
+{
+ emit_mov_r(ctx, lo(dst), lo(src));
+ emit_zext_ver(ctx, dst);
+}
+
+/* Register move operation (64-bit) */
+static void emit_mov_r64(struct jit_context *ctx, const u8 dst[],
+ const u8 src[])
+{
+ emit_mov_r(ctx, lo(dst), lo(src));
+ emit_mov_r(ctx, hi(dst), hi(src));
+}
+
/* Load delay slot, if ISA mandates it */
static void emit_load_delay(struct jit_context *ctx)
{
@@ -1485,8 +1501,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
/* Special mov32 for zext */
emit_mov_i(ctx, hi(dst), 0);
} else {
- emit_mov_r(ctx, lo(dst), lo(src));
- emit_zext_ver(ctx, dst);
+ emit_mov_r32(ctx, dst, src);
}
break;
/* dst = -dst */
@@ -1555,8 +1570,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
break;
/* dst = src (64-bit) */
case BPF_ALU64 | BPF_MOV | BPF_X:
- emit_mov_r(ctx, lo(dst), lo(src));
- emit_mov_r(ctx, hi(dst), hi(src));
+ emit_mov_r64(ctx, dst, src);
break;
/* dst = -dst (64-bit) */
case BPF_ALU64 | BPF_NEG:
diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c
index fa7e9aa37f498..45fee6f6b87e9 100644
--- a/arch/mips/net/bpf_jit_comp64.c
+++ b/arch/mips/net/bpf_jit_comp64.c
@@ -120,6 +120,19 @@ static void emit_zext_ver(struct jit_context *ctx, u8 dst)
emit_zext(ctx, dst);
}
+/* Register move operation (32-bit) */
+static void emit_mov_r32(struct jit_context *ctx, u8 dst, u8 src)
+{
+ emit_mov_r(ctx, dst, src);
+ emit_zext_ver(ctx, dst);
+}
+
+/* Register move operation (64-bit) */
+static void emit_mov_r64(struct jit_context *ctx, u8 dst, u8 src)
+{
+ emit_mov_r(ctx, dst, src);
+}
+
/* dst = imm (64-bit) */
static void emit_mov_i64(struct jit_context *ctx, u8 dst, u64 imm64)
{
@@ -656,8 +669,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
/* Special mov32 for zext */
emit_zext(ctx, dst);
} else {
- emit_mov_r(ctx, dst, src);
- emit_zext_ver(ctx, dst);
+ emit_mov_r32(ctx, dst, src);
}
break;
/* dst = -dst */
@@ -742,7 +754,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
break;
/* dst = src (64-bit) */
case BPF_ALU64 | BPF_MOV | BPF_X:
- emit_mov_r(ctx, dst, src);
+ emit_mov_r64(ctx, dst, src);
break;
/* dst = -dst (64-bit) */
case BPF_ALU64 | BPF_NEG:
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [RFC bpf-next 2/2] bpf, mips: Add support for BPF_MOVSX in the JITs
2026-08-19 1:05 [RFC bpf-next 0/2] bpf, mips: Add BPF_MOVSX support to the JITs Nicholas Dudar
2026-08-19 1:05 ` [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers Nicholas Dudar
@ 2026-08-19 1:05 ` Nicholas Dudar
1 sibling, 0 replies; 4+ messages in thread
From: Nicholas Dudar @ 2026-08-19 1:05 UTC (permalink / raw)
To: bpf
Cc: ast, daniel, andrii, eddyz87, memxor, martin.lau, song,
yonghong.song, jolsa, emil, ihor.solodrai, johan.almbladh,
paulburton, tsbogend, linux-mips, linux-kernel
The MIPS32 and MIPS64 JITs lower register BPF_MOVSX instructions as
ordinary moves because their register-move paths do not interpret
insn->off. Negative low-width values therefore retain incorrect upper
bits.
Teach the register-move helpers to sign-extend the BPF-defined MOVSX
widths: 8 and 16 for ALU32, and 8, 16, and 32 for ALU64. Propagate the
sign into the MIPS32 high word for ALU64, while retaining the existing
verifier-managed zero extension for ALU32.
Keep the verifier-inserted zero-extension move on its dedicated path
and interpret the raw offset within the MOV helpers.
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nicholas Dudar <main.kalliope@gmail.com>
---
arch/mips/net/bpf_jit_comp32.c | 50 ++++++++++++++++++++++++++++------
arch/mips/net/bpf_jit_comp64.c | 46 +++++++++++++++++++++++++------
2 files changed, 79 insertions(+), 17 deletions(-)
diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
index bfe73b023983e..66ade9d77638d 100644
--- a/arch/mips/net/bpf_jit_comp32.c
+++ b/arch/mips/net/bpf_jit_comp32.c
@@ -190,20 +190,52 @@ static void emit_zext_ver(struct jit_context *ctx, const u8 dst[])
}
}
-/* Register move operation (32-bit) */
+/* Register move operation (32-bit), optionally with sign extension */
static void emit_mov_r32(struct jit_context *ctx, const u8 dst[],
- const u8 src[])
+ const u8 src[], s16 off)
{
- emit_mov_r(ctx, lo(dst), lo(src));
+ switch (off) {
+ case 8:
+ emit(ctx, sll, lo(dst), lo(src), 24);
+ emit(ctx, sra, lo(dst), lo(dst), 24);
+ clobber_reg(ctx, lo(dst));
+ break;
+ case 16:
+ emit(ctx, sll, lo(dst), lo(src), 16);
+ emit(ctx, sra, lo(dst), lo(dst), 16);
+ clobber_reg(ctx, lo(dst));
+ break;
+ default:
+ emit_mov_r(ctx, lo(dst), lo(src));
+ break;
+ }
emit_zext_ver(ctx, dst);
}
-/* Register move operation (64-bit) */
+/* Register move operation (64-bit), optionally with sign extension */
static void emit_mov_r64(struct jit_context *ctx, const u8 dst[],
- const u8 src[])
+ const u8 src[], s16 off)
{
- emit_mov_r(ctx, lo(dst), lo(src));
- emit_mov_r(ctx, hi(dst), hi(src));
+ switch (off) {
+ case 8:
+ emit(ctx, sll, lo(dst), lo(src), 24);
+ emit(ctx, sra, lo(dst), lo(dst), 24);
+ break;
+ case 16:
+ emit(ctx, sll, lo(dst), lo(src), 16);
+ emit(ctx, sra, lo(dst), lo(dst), 16);
+ break;
+ case 32:
+ emit(ctx, move, lo(dst), lo(src));
+ break;
+ default:
+ emit_mov_r(ctx, lo(dst), lo(src));
+ emit_mov_r(ctx, hi(dst), hi(src));
+ return;
+ }
+ clobber_reg(ctx, lo(dst));
+ emit(ctx, sra, hi(dst), lo(dst), 31);
+ clobber_reg(ctx, hi(dst));
}
/* Load delay slot, if ISA mandates it */
@@ -1501,7 +1533,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
/* Special mov32 for zext */
emit_mov_i(ctx, hi(dst), 0);
} else {
- emit_mov_r32(ctx, dst, src);
+ emit_mov_r32(ctx, dst, src, off);
}
break;
/* dst = -dst */
@@ -1570,7 +1602,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
break;
/* dst = src (64-bit) */
case BPF_ALU64 | BPF_MOV | BPF_X:
- emit_mov_r64(ctx, dst, src);
+ emit_mov_r64(ctx, dst, src, off);
break;
/* dst = -dst (64-bit) */
case BPF_ALU64 | BPF_NEG:
diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c
index 45fee6f6b87e9..31f73f50caf5d 100644
--- a/arch/mips/net/bpf_jit_comp64.c
+++ b/arch/mips/net/bpf_jit_comp64.c
@@ -120,17 +120,47 @@ static void emit_zext_ver(struct jit_context *ctx, u8 dst)
emit_zext(ctx, dst);
}
-/* Register move operation (32-bit) */
-static void emit_mov_r32(struct jit_context *ctx, u8 dst, u8 src)
+/* Register move operation (32-bit), optionally with sign extension */
+static void emit_mov_r32(struct jit_context *ctx, u8 dst, u8 src, s16 off)
{
- emit_mov_r(ctx, dst, src);
+ switch (off) {
+ case 8:
+ emit(ctx, dsll32, dst, src, 24);
+ emit(ctx, dsra32, dst, dst, 24);
+ clobber_reg(ctx, dst);
+ break;
+ case 16:
+ emit(ctx, dsll32, dst, src, 16);
+ emit(ctx, dsra32, dst, dst, 16);
+ clobber_reg(ctx, dst);
+ break;
+ default:
+ emit_mov_r(ctx, dst, src);
+ break;
+ }
emit_zext_ver(ctx, dst);
}
-/* Register move operation (64-bit) */
-static void emit_mov_r64(struct jit_context *ctx, u8 dst, u8 src)
+/* Register move operation (64-bit), optionally with sign extension */
+static void emit_mov_r64(struct jit_context *ctx, u8 dst, u8 src, s16 off)
{
- emit_mov_r(ctx, dst, src);
+ switch (off) {
+ case 8:
+ emit(ctx, dsll32, dst, src, 24);
+ emit(ctx, dsra32, dst, dst, 24);
+ break;
+ case 16:
+ emit(ctx, dsll32, dst, src, 16);
+ emit(ctx, dsra32, dst, dst, 16);
+ break;
+ case 32:
+ emit(ctx, sll, dst, src, 0);
+ break;
+ default:
+ emit_mov_r(ctx, dst, src);
+ return;
+ }
+ clobber_reg(ctx, dst);
}
/* dst = imm (64-bit) */
@@ -669,7 +699,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
/* Special mov32 for zext */
emit_zext(ctx, dst);
} else {
- emit_mov_r32(ctx, dst, src);
+ emit_mov_r32(ctx, dst, src, off);
}
break;
/* dst = -dst */
@@ -754,7 +784,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
break;
/* dst = src (64-bit) */
case BPF_ALU64 | BPF_MOV | BPF_X:
- emit_mov_r64(ctx, dst, src);
+ emit_mov_r64(ctx, dst, src, off);
break;
/* dst = -dst (64-bit) */
case BPF_ALU64 | BPF_NEG:
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers
2026-08-19 1:05 ` [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers Nicholas Dudar
@ 2026-08-19 1:16 ` sashiko-bot
0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-19 1:16 UTC (permalink / raw)
To: Nicholas Dudar; +Cc: bpf
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The MIPS BPF JIT silently miscompiles `BPF_MOVSX` (move with sign extension) instructions as standard zero-extended or 64-bit copies.
--
--- Patch [1]: [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers ---
commit a341e9c2445aa10a781c4264cc9b57117f7c9711
Author: Nicholas Dudar <main.kalliope@gmail.com>
bpf, mips: Factor register moves into helpers
This patch refactors the MIPS BPF JIT by introducing 32-bit and 64-bit
register-move helpers to remove duplicated backend-specific handling for
MOV instructions. It preserves the verifier-inserted zero-extension marker
behavior.
> diff --git a/arch/mips/net/bpf_jit_comp32.c b/arch/mips/net/bpf_jit_comp32.c
> index 40a878b672f5d..bfe73b023983e 100644
> --- a/arch/mips/net/bpf_jit_comp32.c
> +++ b/arch/mips/net/bpf_jit_comp32.c
[ ... ]
> @@ -1485,8 +1501,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
> /* Special mov32 for zext */
> emit_mov_i(ctx, hi(dst), 0);
> } else {
> - emit_mov_r(ctx, lo(dst), lo(src));
> - emit_zext_ver(ctx, dst);
> + emit_mov_r32(ctx, dst, src);
[Severity: Critical]
This is a pre-existing issue, but does this switch case in build_insn()
silently miscompile BPF_MOVSX (move with sign extension) instructions?
When the BPF program uses a move with sign extension (insn->off != 0), this
code generation appears to perform a standard zero-extended copy instead.
Since the verifier tracks bounds assuming the value was sign-extended,
could this discrepancy allow a program to bypass verifier bounds checks
and perform out-of-bounds memory accesses?
Since you are refactoring the MOV handlers here, should emit_mov_r32() be
updated to take insn->off and implement the proper sign extension logic?
> }
> break;
> /* dst = -dst */
[ ... ]
> diff --git a/arch/mips/net/bpf_jit_comp64.c b/arch/mips/net/bpf_jit_comp64.c
> index fa7e9aa37f498..45fee6f6b87e9 100644
> --- a/arch/mips/net/bpf_jit_comp64.c
> +++ b/arch/mips/net/bpf_jit_comp64.c
[ ... ]
> @@ -742,7 +754,7 @@ int build_insn(const struct bpf_insn *insn, struct jit_context *ctx)
> break;
> /* dst = src (64-bit) */
> case BPF_ALU64 | BPF_MOV | BPF_X:
> - emit_mov_r(ctx, dst, src);
> + emit_mov_r64(ctx, dst, src);
[Severity: Critical]
This is also a pre-existing issue, but does the 64-bit MOV handler similarly
ignore insn->off in build_insn()?
Like the 32-bit case, if a program uses a BPF_MOVSX instruction, does this
result in a standard 64-bit copy instead of the required sign extension?
> break;
> /* dst = -dst (64-bit) */
> case BPF_ALU64 | BPF_NEG:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260819010523.1057789-1-main.kalliope@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-19 1:16 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-19 1:05 [RFC bpf-next 0/2] bpf, mips: Add BPF_MOVSX support to the JITs Nicholas Dudar
2026-08-19 1:05 ` [RFC bpf-next 1/2] bpf, mips: Factor register moves into helpers Nicholas Dudar
2026-08-19 1:16 ` sashiko-bot
2026-08-19 1:05 ` [RFC bpf-next 2/2] bpf, mips: Add support for BPF_MOVSX in the JITs Nicholas Dudar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox