On 6/4/25 1:42 PM, Yonghong Song wrote:
On 6/4/25 9:44 AM, Ihor Solodrai wrote:
On 6/3/25 5:37 PM, Ihor Solodrai wrote:
Add a test for CONST_PTR_TO_MAP comparison with a non-0 constant. A
BPF program with this code must not pass verification in unpriv.
Signed-off-by: Ihor Solodrai <isolodrai@xxxxxxxx>
---
.../selftests/bpf/progs/verifier_unpriv.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/verifier_unpriv.c b/
tools/testing/selftests/bpf/progs/verifier_unpriv.c
index 28200f068ce5..85b41f927272 100644
--- a/tools/testing/selftests/bpf/progs/verifier_unpriv.c
+++ b/tools/testing/selftests/bpf/progs/verifier_unpriv.c
@@ -634,6 +634,23 @@ l0_%=: r0 = 0; \
: __clobber_all);
}
+SEC("socket")
+__description("unpriv: cmp map pointer with const")
+__success __failure_unpriv __msg_unpriv("R1 pointer comparison
prohibited")
+__retval(0)
+__naked void cmp_map_pointer_with_const(void)
+{
+ asm volatile (" \
+ r1 = 0; \
+ r1 = %[map_hash_8b] ll; \
+ if r1 == 0xcafefeeddeadbeef goto l0_%=; \
GCC BPF caught (correctly) that this is not a valid instruction
because imm is supposed to be 32bit [1]:
progs/verifier_unpriv.c: Assembler messages:
progs/verifier_unpriv.c:643: Error: immediate out of range, shall
fit in 32 bits
make: *** [Makefile:751: /tmp/work/bpf/bpf/src/tools/testing/
selftests/bpf/bpf_gcc/verifier_unpriv.bpf.o] Error 1
But LLVM 20 let it compile and the test passes. I wonder whether it's
a bug in LLVM worth reporting?
[1] https://github.com/kernel-patches/bpf/actions/runs/15430930573/
job/43428666342
This is a missed case for llvm. See:
https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/BPF/
MCTargetDesc/BPFMCCodeEmitter.cpp#L82-L85
Basically for the following code,
unsigned BPFMCCodeEmitter::getMachineOpValue(const MCInst &MI,
const MCOperand &MO,
SmallVectorImpl<MCFixup>
&Fixups,
const MCSubtargetInfo
&STI) const {
if (MO.isReg())
return MRI.getEncodingValue(MO.getReg());
if (MO.isImm())
return static_cast<unsigned>(MO.getImm());
For 'static_cast<unsigned>(MO.getImm())', MO.getImm() value is a s64, so
casting to u32 should check
the value range and we didn't check them, hence didn't report an error.
I see. Out of curiosity I looked at llvm-objdump and indeed only lower
32 bits are in the binary:
0000000000000320 <cmp_map_pointer_with_const>:
100: b7 01 00 00 00 00 00 00 r1 = 0x0
101: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll
103: 15 01 00 00 ef be ad de if r1 == -0x21524111 goto +0x0
<l0_11>
Thanks for checking, Yonghong.
The following is the fix:
if (MO.isReg())
return MRI.getEncodingValue(MO.getReg());
- if (MO.isImm())
+ if (MO.isImm()) {
+ assert(MO.getImm() >= INT_MIN && MO.getImm() <= INT_MAX);
return static_cast<unsigned>(MO.getImm());
+ }
With the above, if the clang build enables assertion, the following dump
will show up:
clang: /home/yhs/work/llvm-project/llvm/lib/Target/BPF/MCTargetDesc/
BPFMCCodeEmitter.cpp:86: unsigned int (anonymous
namespace)::BPFMCCodeEmitter::getMachin
eOpValue(const MCInst &, const MCOperand &, SmallVectorImpl<MCFixup> &,
const MCSubtargetInfo &) const: Assertion `MO.getImm() >= INT_MIN &&
MO.getImm() <=
INT_MAX' failed.
Although llvm tends to use 'assert' a lot (and 'assert' thing will
become noop on production
build), e.g.,
[~/work/llvm-project/llvm/lib/Target/BPF/MCTargetDesc (release/19.x)]$
grep assert *.cpp
BPFAsmBackend.cpp:#include <cassert>
BPFAsmBackend.cpp: assert(unsigned(Kind - FirstTargetFixupKind) <
getNumFixupKinds() &&
BPFAsmBackend.cpp: assert(Value <= UINT32_MAX);
BPFAsmBackend.cpp: assert(Fixup.getKind() == FK_PCRel_2);
BPFELFObjectWriter.cpp: assert(SectionELF && "Null section for
reloc symbol");
BPFInstPrinter.cpp: assert(Kind == MCSymbolRefExpr::VK_None);
BPFInstPrinter.cpp: assert((Modifier == nullptr || Modifier[0] == 0) &&
"No modifiers supported");
BPFInstPrinter.cpp: assert(Op.isExpr() && "Expected an expression");
BPFInstPrinter.cpp: assert(RegOp.isReg() && "Register operand not a
register");
BPFInstPrinter.cpp: assert(0 && "Expected an immediate");
BPFMCCodeEmitter.cpp:#include <cassert>
BPFMCCodeEmitter.cpp: assert(MO.isExpr());
BPFMCCodeEmitter.cpp: assert(Expr->getKind() == MCExpr::SymbolRef);
BPFMCCodeEmitter.cpp: assert(Op1.isReg() && "First operand is not
register.");
BPFMCCodeEmitter.cpp: assert(Op2.isImm() && "Second operand is not
immediate.");
Production build tends not to enable assertion for performance reason, so
I guess we could emit an error to user for such cases. Will fix in llvm21.
+l0_%=: r0 = 0; \
+ exit; \
+" :
+ : __imm_addr(map_hash_8b)
+ : __clobber_all);
+}
+
SEC("socket")
__description("unpriv: write into frame pointer")
__failure __msg("frame pointer is read only")
