* [PATCH v5 2/3] powerpc/atomics: Use immediate operand when possible
From: Christophe Leroy @ 2021-09-21 15:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e6f815d9181bab09df3b350af51149437863e9f9.1632236981.git.christophe.leroy@csgroup.eu>
Today we get the following code generation for atomic operations:
c001bb2c: 39 20 00 01 li r9,1
c001bb30: 7d 40 18 28 lwarx r10,0,r3
c001bb34: 7d 09 50 50 subf r8,r9,r10
c001bb38: 7d 00 19 2d stwcx. r8,0,r3
c001c7a8: 39 40 00 01 li r10,1
c001c7ac: 7d 00 18 28 lwarx r8,0,r3
c001c7b0: 7c ea 42 14 add r7,r10,r8
c001c7b4: 7c e0 19 2d stwcx. r7,0,r3
By allowing GCC to choose between immediate or regular operation,
we get:
c001bb2c: 7d 20 18 28 lwarx r9,0,r3
c001bb30: 39 49 ff ff addi r10,r9,-1
c001bb34: 7d 40 19 2d stwcx. r10,0,r3
--
c001c7a4: 7d 40 18 28 lwarx r10,0,r3
c001c7a8: 39 0a 00 01 addi r8,r10,1
c001c7ac: 7d 00 19 2d stwcx. r8,0,r3
For "and", the dot form has to be used because "andi" doesn't exist.
For logical operations we use unsigned 16 bits immediate.
For arithmetic operations we use signed 16 bits immediate.
On pmac32_defconfig, it reduces the text by approx another 8 kbytes.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
---
v4: Rebased
v2: Use "addc/addic"
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/atomic.h | 56 +++++++++++++++----------------
1 file changed, 28 insertions(+), 28 deletions(-)
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index 6a53ef178bfd..ce0d5a013c58 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -37,62 +37,62 @@ static __inline__ void arch_atomic_set(atomic_t *v, int i)
__asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) : "r"(i));
}
-#define ATOMIC_OP(op, asm_op) \
+#define ATOMIC_OP(op, asm_op, suffix, sign, ...) \
static __inline__ void arch_atomic_##op(int a, atomic_t *v) \
{ \
int t; \
\
__asm__ __volatile__( \
"1: lwarx %0,0,%3 # atomic_" #op "\n" \
- #asm_op " %0,%2,%0\n" \
+ #asm_op "%I2" suffix " %0,%0,%2\n" \
" stwcx. %0,0,%3 \n" \
" bne- 1b\n" \
: "=&r" (t), "+m" (v->counter) \
- : "r" (a), "r" (&v->counter) \
- : "cc"); \
+ : "r"#sign (a), "r" (&v->counter) \
+ : "cc", ##__VA_ARGS__); \
} \
-#define ATOMIC_OP_RETURN_RELAXED(op, asm_op) \
+#define ATOMIC_OP_RETURN_RELAXED(op, asm_op, suffix, sign, ...) \
static inline int arch_atomic_##op##_return_relaxed(int a, atomic_t *v) \
{ \
int t; \
\
__asm__ __volatile__( \
"1: lwarx %0,0,%3 # atomic_" #op "_return_relaxed\n" \
- #asm_op " %0,%2,%0\n" \
+ #asm_op "%I2" suffix " %0,%0,%2\n" \
" stwcx. %0,0,%3\n" \
" bne- 1b\n" \
: "=&r" (t), "+m" (v->counter) \
- : "r" (a), "r" (&v->counter) \
- : "cc"); \
+ : "r"#sign (a), "r" (&v->counter) \
+ : "cc", ##__VA_ARGS__); \
\
return t; \
}
-#define ATOMIC_FETCH_OP_RELAXED(op, asm_op) \
+#define ATOMIC_FETCH_OP_RELAXED(op, asm_op, suffix, sign, ...) \
static inline int arch_atomic_fetch_##op##_relaxed(int a, atomic_t *v) \
{ \
int res, t; \
\
__asm__ __volatile__( \
"1: lwarx %0,0,%4 # atomic_fetch_" #op "_relaxed\n" \
- #asm_op " %1,%3,%0\n" \
+ #asm_op "%I3" suffix " %1,%0,%3\n" \
" stwcx. %1,0,%4\n" \
" bne- 1b\n" \
: "=&r" (res), "=&r" (t), "+m" (v->counter) \
- : "r" (a), "r" (&v->counter) \
- : "cc"); \
+ : "r"#sign (a), "r" (&v->counter) \
+ : "cc", ##__VA_ARGS__); \
\
return res; \
}
-#define ATOMIC_OPS(op, asm_op) \
- ATOMIC_OP(op, asm_op) \
- ATOMIC_OP_RETURN_RELAXED(op, asm_op) \
- ATOMIC_FETCH_OP_RELAXED(op, asm_op)
+#define ATOMIC_OPS(op, asm_op, suffix, sign, ...) \
+ ATOMIC_OP(op, asm_op, suffix, sign, ##__VA_ARGS__) \
+ ATOMIC_OP_RETURN_RELAXED(op, asm_op, suffix, sign, ##__VA_ARGS__)\
+ ATOMIC_FETCH_OP_RELAXED(op, asm_op, suffix, sign, ##__VA_ARGS__)
-ATOMIC_OPS(add, add)
-ATOMIC_OPS(sub, subf)
+ATOMIC_OPS(add, add, "c", I, "xer")
+ATOMIC_OPS(sub, sub, "c", I, "xer")
#define arch_atomic_add_return_relaxed arch_atomic_add_return_relaxed
#define arch_atomic_sub_return_relaxed arch_atomic_sub_return_relaxed
@@ -101,13 +101,13 @@ ATOMIC_OPS(sub, subf)
#define arch_atomic_fetch_sub_relaxed arch_atomic_fetch_sub_relaxed
#undef ATOMIC_OPS
-#define ATOMIC_OPS(op, asm_op) \
- ATOMIC_OP(op, asm_op) \
- ATOMIC_FETCH_OP_RELAXED(op, asm_op)
+#define ATOMIC_OPS(op, asm_op, suffix, sign) \
+ ATOMIC_OP(op, asm_op, suffix, sign) \
+ ATOMIC_FETCH_OP_RELAXED(op, asm_op, suffix, sign)
-ATOMIC_OPS(and, and)
-ATOMIC_OPS(or, or)
-ATOMIC_OPS(xor, xor)
+ATOMIC_OPS(and, and, ".", K)
+ATOMIC_OPS(or, or, "", K)
+ATOMIC_OPS(xor, xor, "", K)
#define arch_atomic_fetch_and_relaxed arch_atomic_fetch_and_relaxed
#define arch_atomic_fetch_or_relaxed arch_atomic_fetch_or_relaxed
@@ -241,15 +241,15 @@ static __inline__ int arch_atomic_fetch_add_unless(atomic_t *v, int a, int u)
"1: lwarx %0,0,%1 # atomic_fetch_add_unless\n\
cmpw 0,%0,%3 \n\
beq 2f \n\
- add %0,%2,%0 \n"
+ add%I2c %0,%0,%2 \n"
" stwcx. %0,0,%1 \n\
bne- 1b \n"
PPC_ATOMIC_EXIT_BARRIER
-" subf %0,%2,%0 \n\
+" sub%I2c %0,%0,%2 \n\
2:"
: "=&r" (t)
- : "r" (&v->counter), "r" (a), "r" (u)
- : "cc", "memory");
+ : "r" (&v->counter), "rI" (a), "r" (u)
+ : "cc", "memory", "xer");
return t;
}
--
2.31.1
^ permalink raw reply related
* [PATCH v5 1/3] powerpc/bitops: Use immediate operand when possible
From: Christophe Leroy @ 2021-09-21 15:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Today we get the following code generation for bitops like
set or clear bit:
c0009fe0: 39 40 08 00 li r10,2048
c0009fe4: 7c e0 40 28 lwarx r7,0,r8
c0009fe8: 7c e7 53 78 or r7,r7,r10
c0009fec: 7c e0 41 2d stwcx. r7,0,r8
c000d568: 39 00 18 00 li r8,6144
c000d56c: 7c c0 38 28 lwarx r6,0,r7
c000d570: 7c c6 40 78 andc r6,r6,r8
c000d574: 7c c0 39 2d stwcx. r6,0,r7
Most set bits are constant on lower 16 bits, so it can easily
be replaced by the "immediate" version of the operation. Allow
GCC to choose between the normal or immediate form.
For clear bits, on 32 bits 'rlwinm' can be used instead of 'andc' for
when all bits to be cleared are consecutive.
On 64 bits we don't have any equivalent single operation for clearing,
single bits or a few bits, we'd need two 'rldicl' so it is not
worth it, the li/andc sequence is doing the same.
With this patch we get:
c0009fe0: 7d 00 50 28 lwarx r8,0,r10
c0009fe4: 61 08 08 00 ori r8,r8,2048
c0009fe8: 7d 00 51 2d stwcx. r8,0,r10
c000d558: 7c e0 40 28 lwarx r7,0,r8
c000d55c: 54 e7 05 64 rlwinm r7,r7,0,21,18
c000d560: 7c e0 41 2d stwcx. r7,0,r8
On pmac32_defconfig, it reduces the text by approx 10 kbytes.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Reviewed-by: Segher Boessenkool <segher@kernel.crashing.org>
---
v5: Fixed the argument of is_rlwinm_mask_valid() in test_and_clear_bits()
v4: Rebased
v3:
- Using the mask validation proposed by Segher
v2:
- Use "n" instead of "i" as constraint for the rlwinm mask
- Improve mask verification to handle more than single bit masks
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/bitops.h | 89 ++++++++++++++++++++++++++++---
1 file changed, 81 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/bitops.h b/arch/powerpc/include/asm/bitops.h
index 11847b6a244e..a05d8c62cbea 100644
--- a/arch/powerpc/include/asm/bitops.h
+++ b/arch/powerpc/include/asm/bitops.h
@@ -71,19 +71,61 @@ static inline void fn(unsigned long mask, \
__asm__ __volatile__ ( \
prefix \
"1:" PPC_LLARX "%0,0,%3,0\n" \
- stringify_in_c(op) "%0,%0,%2\n" \
+ #op "%I2 %0,%0,%2\n" \
PPC_STLCX "%0,0,%3\n" \
"bne- 1b\n" \
: "=&r" (old), "+m" (*p) \
- : "r" (mask), "r" (p) \
+ : "rK" (mask), "r" (p) \
: "cc", "memory"); \
}
DEFINE_BITOP(set_bits, or, "")
-DEFINE_BITOP(clear_bits, andc, "")
-DEFINE_BITOP(clear_bits_unlock, andc, PPC_RELEASE_BARRIER)
DEFINE_BITOP(change_bits, xor, "")
+static __always_inline bool is_rlwinm_mask_valid(unsigned long x)
+{
+ if (!x)
+ return false;
+ if (x & 1)
+ x = ~x; // make the mask non-wrapping
+ x += x & -x; // adding the low set bit results in at most one bit set
+
+ return !(x & (x - 1));
+}
+
+#define DEFINE_CLROP(fn, prefix) \
+static inline void fn(unsigned long mask, volatile unsigned long *_p) \
+{ \
+ unsigned long old; \
+ unsigned long *p = (unsigned long *)_p; \
+ \
+ if (IS_ENABLED(CONFIG_PPC32) && \
+ __builtin_constant_p(mask) && is_rlwinm_mask_valid(~mask)) {\
+ asm volatile ( \
+ prefix \
+ "1:" "lwarx %0,0,%3\n" \
+ "rlwinm %0,%0,0,%2\n" \
+ "stwcx. %0,0,%3\n" \
+ "bne- 1b\n" \
+ : "=&r" (old), "+m" (*p) \
+ : "n" (~mask), "r" (p) \
+ : "cc", "memory"); \
+ } else { \
+ asm volatile ( \
+ prefix \
+ "1:" PPC_LLARX "%0,0,%3,0\n" \
+ "andc %0,%0,%2\n" \
+ PPC_STLCX "%0,0,%3\n" \
+ "bne- 1b\n" \
+ : "=&r" (old), "+m" (*p) \
+ : "r" (mask), "r" (p) \
+ : "cc", "memory"); \
+ } \
+}
+
+DEFINE_CLROP(clear_bits, "")
+DEFINE_CLROP(clear_bits_unlock, PPC_RELEASE_BARRIER)
+
static inline void arch_set_bit(int nr, volatile unsigned long *addr)
{
set_bits(BIT_MASK(nr), addr + BIT_WORD(nr));
@@ -116,12 +158,12 @@ static inline unsigned long fn( \
__asm__ __volatile__ ( \
prefix \
"1:" PPC_LLARX "%0,0,%3,%4\n" \
- stringify_in_c(op) "%1,%0,%2\n" \
+ #op "%I2 %1,%0,%2\n" \
PPC_STLCX "%1,0,%3\n" \
"bne- 1b\n" \
postfix \
: "=&r" (old), "=&r" (t) \
- : "r" (mask), "r" (p), "i" (IS_ENABLED(CONFIG_PPC64) ? eh : 0) \
+ : "rK" (mask), "r" (p), "i" (IS_ENABLED(CONFIG_PPC64) ? eh : 0) \
: "cc", "memory"); \
return (old & mask); \
}
@@ -130,11 +172,42 @@ DEFINE_TESTOP(test_and_set_bits, or, PPC_ATOMIC_ENTRY_BARRIER,
PPC_ATOMIC_EXIT_BARRIER, 0)
DEFINE_TESTOP(test_and_set_bits_lock, or, "",
PPC_ACQUIRE_BARRIER, 1)
-DEFINE_TESTOP(test_and_clear_bits, andc, PPC_ATOMIC_ENTRY_BARRIER,
- PPC_ATOMIC_EXIT_BARRIER, 0)
DEFINE_TESTOP(test_and_change_bits, xor, PPC_ATOMIC_ENTRY_BARRIER,
PPC_ATOMIC_EXIT_BARRIER, 0)
+static inline unsigned long test_and_clear_bits(unsigned long mask, volatile unsigned long *_p)
+{
+ unsigned long old, t;
+ unsigned long *p = (unsigned long *)_p;
+
+ if (IS_ENABLED(CONFIG_PPC32) &&
+ __builtin_constant_p(mask) && is_rlwinm_mask_valid(~mask)) {
+ asm volatile (
+ PPC_ATOMIC_ENTRY_BARRIER
+ "1:" "lwarx %0,0,%3\n"
+ "rlwinm %1,%0,0,%2\n"
+ "stwcx. %1,0,%3\n"
+ "bne- 1b\n"
+ PPC_ATOMIC_EXIT_BARRIER
+ : "=&r" (old), "=&r" (t)
+ : "n" (~mask), "r" (p)
+ : "cc", "memory");
+ } else {
+ asm volatile (
+ PPC_ATOMIC_ENTRY_BARRIER
+ "1:" PPC_LLARX "%0,0,%3,0\n"
+ "andc %1,%0,%2\n"
+ PPC_STLCX "%1,0,%3\n"
+ "bne- 1b\n"
+ PPC_ATOMIC_EXIT_BARRIER
+ : "=&r" (old), "=&r" (t)
+ : "r" (mask), "r" (p)
+ : "cc", "memory");
+ }
+
+ return (old & mask);
+}
+
static inline int arch_test_and_set_bit(unsigned long nr,
volatile unsigned long *addr)
{
--
2.31.1
^ permalink raw reply related
* [PATCH v5 3/3] powerpc/atomics: Remove atomic_inc()/atomic_dec() and friends
From: Christophe Leroy @ 2021-09-21 15:09 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <e6f815d9181bab09df3b350af51149437863e9f9.1632236981.git.christophe.leroy@csgroup.eu>
Now that atomic_add() and atomic_sub() handle immediate operands,
atomic_inc() and atomic_dec() have no added value compared to the
generic fallback which calls atomic_add(1) and atomic_sub(1).
Also remove atomic_inc_not_zero() which fallsback to
atomic_add_unless() which itself fallsback to
atomic_fetch_add_unless() which now handles immediate operands.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v4: rebased
v2: New
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/atomic.h | 95 -------------------------------
1 file changed, 95 deletions(-)
diff --git a/arch/powerpc/include/asm/atomic.h b/arch/powerpc/include/asm/atomic.h
index ce0d5a013c58..395d1feb5790 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -118,71 +118,6 @@ ATOMIC_OPS(xor, xor, "", K)
#undef ATOMIC_OP_RETURN_RELAXED
#undef ATOMIC_OP
-static __inline__ void arch_atomic_inc(atomic_t *v)
-{
- int t;
-
- __asm__ __volatile__(
-"1: lwarx %0,0,%2 # atomic_inc\n\
- addic %0,%0,1\n"
-" stwcx. %0,0,%2 \n\
- bne- 1b"
- : "=&r" (t), "+m" (v->counter)
- : "r" (&v->counter)
- : "cc", "xer");
-}
-#define arch_atomic_inc arch_atomic_inc
-
-static __inline__ int arch_atomic_inc_return_relaxed(atomic_t *v)
-{
- int t;
-
- __asm__ __volatile__(
-"1: lwarx %0,0,%2 # atomic_inc_return_relaxed\n"
-" addic %0,%0,1\n"
-" stwcx. %0,0,%2\n"
-" bne- 1b"
- : "=&r" (t), "+m" (v->counter)
- : "r" (&v->counter)
- : "cc", "xer");
-
- return t;
-}
-
-static __inline__ void arch_atomic_dec(atomic_t *v)
-{
- int t;
-
- __asm__ __volatile__(
-"1: lwarx %0,0,%2 # atomic_dec\n\
- addic %0,%0,-1\n"
-" stwcx. %0,0,%2\n\
- bne- 1b"
- : "=&r" (t), "+m" (v->counter)
- : "r" (&v->counter)
- : "cc", "xer");
-}
-#define arch_atomic_dec arch_atomic_dec
-
-static __inline__ int arch_atomic_dec_return_relaxed(atomic_t *v)
-{
- int t;
-
- __asm__ __volatile__(
-"1: lwarx %0,0,%2 # atomic_dec_return_relaxed\n"
-" addic %0,%0,-1\n"
-" stwcx. %0,0,%2\n"
-" bne- 1b"
- : "=&r" (t), "+m" (v->counter)
- : "r" (&v->counter)
- : "cc", "xer");
-
- return t;
-}
-
-#define arch_atomic_inc_return_relaxed arch_atomic_inc_return_relaxed
-#define arch_atomic_dec_return_relaxed arch_atomic_dec_return_relaxed
-
#define arch_atomic_cmpxchg(v, o, n) \
(arch_cmpxchg(&((v)->counter), (o), (n)))
#define arch_atomic_cmpxchg_relaxed(v, o, n) \
@@ -255,36 +190,6 @@ static __inline__ int arch_atomic_fetch_add_unless(atomic_t *v, int a, int u)
}
#define arch_atomic_fetch_add_unless arch_atomic_fetch_add_unless
-/**
- * atomic_inc_not_zero - increment unless the number is zero
- * @v: pointer of type atomic_t
- *
- * Atomically increments @v by 1, so long as @v is non-zero.
- * Returns non-zero if @v was non-zero, and zero otherwise.
- */
-static __inline__ int arch_atomic_inc_not_zero(atomic_t *v)
-{
- int t1, t2;
-
- __asm__ __volatile__ (
- PPC_ATOMIC_ENTRY_BARRIER
-"1: lwarx %0,0,%2 # atomic_inc_not_zero\n\
- cmpwi 0,%0,0\n\
- beq- 2f\n\
- addic %1,%0,1\n"
-" stwcx. %1,0,%2\n\
- bne- 1b\n"
- PPC_ATOMIC_EXIT_BARRIER
- "\n\
-2:"
- : "=&r" (t1), "=&r" (t2)
- : "r" (&v->counter)
- : "cc", "xer", "memory");
-
- return t1;
-}
-#define arch_atomic_inc_not_zero(v) arch_atomic_inc_not_zero((v))
-
/*
* Atomically test *v and decrement if it is greater than 0.
* The function returns the old value of *v minus 1, even if
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v3 8/8] bpf ppc32: Access only if addr is kernel address
From: Christophe Leroy @ 2021-09-21 14:27 UTC (permalink / raw)
To: Hari Bathini, naveen.n.rao, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-9-hbathini@linux.ibm.com>
Le 21/09/2021 à 15:29, Hari Bathini a écrit :
> With KUAP enabled, any kernel code which wants to access userspace
> needs to be surrounded by disable-enable KUAP. But that is not
> happening for BPF_PROBE_MEM load instruction. Though PPC32 does not
> support read protection, considering the fact that PTR_TO_BTF_ID
> (which uses BPF_PROBE_MEM mode) could either be a valid kernel pointer
> or NULL but should never be a pointer to userspace address, execute
> BPF_PROBE_MEM load only if addr is kernel address, otherwise set
> dst_reg=0 and move on.
>
> This will catch NULL, valid or invalid userspace pointers. Only bad
> kernel pointer will be handled by BPF exception table.
>
> [Alexei suggested for x86]
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v3:
> * Updated jump for PPC_BCC to always be the same while emitting
> a NOP instruction when needed.
>
>
> arch/powerpc/net/bpf_jit_comp32.c | 35 +++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index 1239643f532c..59849e1230d2 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -825,6 +825,41 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
> fallthrough;
> case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
> + /*
> + * As PTR_TO_BTF_ID that uses BPF_PROBE_MEM mode could either be a valid
> + * kernel pointer or NULL but not a userspace address, execute BPF_PROBE_MEM
> + * load only if addr is kernel address (see is_kernel_addr()), otherwise
> + * set dst_reg=0 and move on.
> + */
> + if (BPF_MODE(code) == BPF_PROBE_MEM) {
> + EMIT(PPC_RAW_ADDI(b2p[TMP_REG], src_reg, off));
> + PPC_LI32(_R0, TASK_SIZE);
> + EMIT(PPC_RAW_CMPLW(b2p[TMP_REG], _R0));
You may drop the ADDI and do:
PPC_LI32(_R0, TASK_SIZE - off);
EMIT(PPC_RAW_CMPLW(src_reg, _R0));
It will likely be the same number of instructions because now the
PPC_LI32 will generate two instruction, but it avoids the use of TMP_REG.
> + PPC_BCC(COND_GT, (ctx->idx + 5) * 4);
> + EMIT(PPC_RAW_LI(dst_reg, 0));
> + /*
> + * For BPF_DW case, "li reg_h,0" would be needed when
> + * !fp->aux->verifier_zext. Emit NOP otherwise.
> + *
> + * Note that "li reg_h,0" is emitted for BPF_B/H/W case,
> + * if necessary. So, jump there insted of emitting an
> + * additional "li reg_h,0" instruction.
> + */
> + if (size == BPF_DW && !fp->aux->verifier_zext)
> + EMIT(PPC_RAW_LI(dst_reg_h, 0));
> + else
> + EMIT(PPC_RAW_NOP());
> + /*
> + * Need to jump two instructions instead of one for BPF_DW case
> + * as there are two load instructions for dst_reg_h & dst_reg
> + * respectively.
> + */
> + if (size == BPF_DW)
> + PPC_JMP((ctx->idx + 3) * 4);
> + else
> + PPC_JMP((ctx->idx + 2) * 4);
> + }
> +
> switch (size) {
> case BPF_B:
> EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
>
^ permalink raw reply
* Re: [PATCH v3 7/8] bpf ppc32: Add BPF_PROBE_MEM support for JIT
From: Christophe Leroy @ 2021-09-21 14:22 UTC (permalink / raw)
To: Hari Bathini, naveen.n.rao, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-8-hbathini@linux.ibm.com>
Le 21/09/2021 à 15:29, Hari Bathini a écrit :
> BPF load instruction with BPF_PROBE_MEM mode can cause a fault
> inside kernel. Append exception table for such instructions
> within BPF program.
>
> Unlike other archs which uses extable 'fixup' field to pass dest_reg
> and nip, BPF exception table on PowerPC follows the generic PowerPC
> exception table design, where it populates both fixup and extable
> sections within BPF program. fixup section contains 3 instructions,
> first 2 instructions clear dest_reg (lower & higher 32-bit registers)
> and last instruction jumps to next instruction in the BPF code.
> extable 'insn' field contains relative offset of the instruction and
> 'fixup' field contains relative offset of the fixup entry. Example
> layout of BPF program with extable present:
>
> +------------------+
> | |
> | |
> 0x4020 -->| lwz r28,4(r4) |
> | |
> | |
> 0x40ac -->| lwz r3,0(r24) |
> | lwz r4,4(r24) |
> | |
> | |
> |------------------|
> 0x4278 -->| li r28,0 | \
> | li r27,0 | | fixup entry
> | b 0x4024 | /
> 0x4284 -->| li r4,0 |
> | li r3,0 |
> | b 0x40b4 |
> |------------------|
> 0x4290 -->| insn=0xfffffd90 | \ extable entry
> | fixup=0xffffffe4 | /
> 0x4298 -->| insn=0xfffffe14 |
> | fixup=0xffffffe8 |
> +------------------+
>
> (Addresses shown here are chosen random, not real)
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v3:
> * Changed how BPF_FIXUP_LEN is defined based on Chris' suggestion.
>
>
> arch/powerpc/net/bpf_jit.h | 4 ++++
> arch/powerpc/net/bpf_jit_comp.c | 2 ++
> arch/powerpc/net/bpf_jit_comp32.c | 34 +++++++++++++++++++++++++++++++
> 3 files changed, 40 insertions(+)
>
> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
> index 561689a2abdf..800734056200 100644
> --- a/arch/powerpc/net/bpf_jit.h
> +++ b/arch/powerpc/net/bpf_jit.h
> @@ -144,7 +144,11 @@ struct codegen_context {
> unsigned int exentry_idx;
> };
>
> +#ifdef CONFIG_PPC32
> +#define BPF_FIXUP_LEN 3 /* Three instructions => 12 bytes */
> +#else
> #define BPF_FIXUP_LEN 2 /* Two instructions => 8 bytes */
> +#endif
>
> static inline void bpf_flush_icache(void *start, void *end)
> {
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index f02457c6b54f..1a0041997050 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -297,6 +297,8 @@ int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct code
> (ctx->exentry_idx * BPF_FIXUP_LEN * 4);
>
> fixup[0] = PPC_RAW_LI(dst_reg, 0);
> + if (IS_ENABLED(CONFIG_PPC32))
> + fixup[1] = PPC_RAW_LI(dst_reg - 1, 0); /* clear higher 32-bit register too */
>
> fixup[BPF_FIXUP_LEN - 1] =
> PPC_RAW_BRANCH((long)(pc + jmp_off) - (long)&fixup[BPF_FIXUP_LEN - 1]);
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index 820c7848434e..1239643f532c 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -812,11 +812,19 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> */
> case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */
> fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_B:
> + fallthrough;
Same comment about the fallthroughs
> case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */
> fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_H:
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */
> fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_W:
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
> + fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
> switch (size) {
> case BPF_B:
> EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> @@ -841,6 +849,32 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
>
> if (size != BPF_DW && !fp->aux->verifier_zext)
> EMIT(PPC_RAW_LI(dst_reg_h, 0));
> +
> + if (BPF_MODE(code) == BPF_PROBE_MEM) {
> + int insn_idx = ctx->idx - 1;
> + int jmp_off = 4;
> +
> + /*
> + * In case of BPF_DW, two lwz instructions are emitted, one
> + * for higher 32-bit and another for lower 32-bit. So, set
> + * ex->insn to the first of the two and jump over both
> + * instructions in fixup.
> + *
> + * Similarly, with !verifier_zext, two instructions are
> + * emitted for BPF_B/H/W case. So, set ex->insn to the
> + * instruction that could fault and skip over both
> + * instructions.
> + */
> + if (size == BPF_DW || !fp->aux->verifier_zext) {
> + insn_idx -= 1;
> + jmp_off += 4;
> + }
> +
> + ret = bpf_add_extable_entry(fp, image, pass, ctx, insn_idx,
> + jmp_off, dst_reg);
> + if (ret)
> + return ret;
> + }
> break;
>
> /*
>
^ permalink raw reply
* Re: [PATCH v3 6/8] bpf ppc64: Access only if addr is kernel address
From: Christophe Leroy @ 2021-09-21 14:21 UTC (permalink / raw)
To: Hari Bathini, naveen.n.rao, mpe, ast, daniel
Cc: Ravi Bangoria, songliubraving, netdev, john.fastabend, andrii,
kpsingh, paulus, yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-7-hbathini@linux.ibm.com>
Le 21/09/2021 à 15:29, Hari Bathini a écrit :
> From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>
> On PPC64 with KUAP enabled, any kernel code which wants to
> access userspace needs to be surrounded by disable-enable KUAP.
> But that is not happening for BPF_PROBE_MEM load instruction.
> So, when BPF program tries to access invalid userspace address,
> page-fault handler considers it as bad KUAP fault:
>
> Kernel attempted to read user page (d0000000) - exploit attempt? (uid: 0)
>
> Considering the fact that PTR_TO_BTF_ID (which uses BPF_PROBE_MEM
> mode) could either be a valid kernel pointer or NULL but should
> never be a pointer to userspace address, execute BPF_PROBE_MEM load
> only if addr is kernel address, otherwise set dst_reg=0 and move on.
>
> This will catch NULL, valid or invalid userspace pointers. Only bad
> kernel pointer will be handled by BPF exception table.
>
> [Alexei suggested for x86]
> Suggested-by: Alexei Starovoitov <ast@kernel.org>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v3:
> * Used is_kernel_addr() logic instead of using TASK_SIZE_MAX check
> all the time.
> * Addressed other comments from Christophe.
>
>
> arch/powerpc/net/bpf_jit_comp64.c | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
>
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 506934c13ef7..06e1206a4266 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -734,6 +734,35 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> case BPF_LDX | BPF_MEM | BPF_DW:
> fallthrough;
> case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
> + /*
> + * As PTR_TO_BTF_ID that uses BPF_PROBE_MEM mode could either be a valid
> + * kernel pointer or NULL but not a userspace address, execute BPF_PROBE_MEM
> + * load only if addr is kernel address (see is_kernel_addr()), otherwise
> + * set dst_reg=0 and move on.
> + */
> + if (BPF_MODE(code) == BPF_PROBE_MEM) {
> + EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], src_reg, off));
> +#ifdef CONFIG_PPC_BOOK3E_64
It is better to use IS_ENABLED() whenever possible,
if (IS_ENABLED((CONFIG_PPC_BOOK3E_64))
PPC_LI64(b2p[TMP_REG_2], 0x8000000000000000ul);
else
PPC_LI64(b2p[TMP_REG_2], PAGE_OFFSET);
> + PPC_LI64(b2p[TMP_REG_2], 0x8000000000000000ul);
> +#elif defined(CONFIG_PPC_BOOK3S_64)
> + PPC_LI64(b2p[TMP_REG_2], PAGE_OFFSET);
> +#else
> + PPC_LI64(b2p[TMP_REG_2], TASK_SIZE);
> +#endif
PPC64 is either CONFIG_PPC_BOOK3S_64 or CONFIG_PPC_BOOK3E_64. The else
is PPC32.
> + EMIT(PPC_RAW_CMPLD(b2p[TMP_REG_1], b2p[TMP_REG_2]));
> + PPC_BCC(COND_GT, (ctx->idx + 4) * 4);
> + EMIT(PPC_RAW_LI(dst_reg, 0));
> + /*
> + * Check if 'off' is word aligned because PPC_BPF_LL()
> + * (BPF_DW case) generates two instructions if 'off' is not
> + * word-aligned and one instruction otherwise.
> + */
> + if (BPF_SIZE(code) == BPF_DW && (off & 3))
> + PPC_JMP((ctx->idx + 3) * 4);
> + else
> + PPC_JMP((ctx->idx + 2) * 4);
> + }
> +
> switch (size) {
> case BPF_B:
> EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
>
^ permalink raw reply
* Re: [PATCH v3 5/8] bpf ppc64: Add BPF_PROBE_MEM support for JIT
From: Christophe Leroy @ 2021-09-21 14:18 UTC (permalink / raw)
To: Hari Bathini, naveen.n.rao, mpe, ast, daniel
Cc: Ravi Bangoria, songliubraving, netdev, john.fastabend, andrii,
kpsingh, paulus, yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-6-hbathini@linux.ibm.com>
Le 21/09/2021 à 15:29, Hari Bathini a écrit :
> From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>
> BPF load instruction with BPF_PROBE_MEM mode can cause a fault
> inside kernel. Append exception table for such instructions
> within BPF program.
>
> Unlike other archs which uses extable 'fixup' field to pass dest_reg
> and nip, BPF exception table on PowerPC follows the generic PowerPC
> exception table design, where it populates both fixup and extable
> sections within BPF program. fixup section contains two instructions,
> first instruction clears dest_reg and 2nd jumps to next instruction
> in the BPF code. extable 'insn' field contains relative offset of
> the instruction and 'fixup' field contains relative offset of the
> fixup entry. Example layout of BPF program with extable present:
>
> +------------------+
> | |
> | |
> 0x4020 -->| ld r27,4(r3) |
> | |
> | |
> 0x40ac -->| lwz r3,0(r4) |
> | |
> | |
> |------------------|
> 0x4280 -->| li r27,0 | \ fixup entry
> | b 0x4024 | /
> 0x4288 -->| li r3,0 |
> | b 0x40b0 |
> |------------------|
> 0x4290 -->| insn=0xfffffd90 | \ extable entry
> | fixup=0xffffffec | /
> 0x4298 -->| insn=0xfffffe14 |
> | fixup=0xffffffec |
> +------------------+
>
> (Addresses shown here are chosen random, not real)
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v3:
> * Made all changes for bpf_add_extable_entry() in this patch instead
> of updating it again in patch #7.
> * Fixed a coding style issue.
>
>
> arch/powerpc/net/bpf_jit.h | 8 +++-
> arch/powerpc/net/bpf_jit_comp.c | 70 ++++++++++++++++++++++++++++---
> arch/powerpc/net/bpf_jit_comp32.c | 2 +-
> arch/powerpc/net/bpf_jit_comp64.c | 17 +++++++-
> 4 files changed, 88 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
> index 0c8f885b8f48..561689a2abdf 100644
> --- a/arch/powerpc/net/bpf_jit.h
> +++ b/arch/powerpc/net/bpf_jit.h
> @@ -141,8 +141,11 @@ struct codegen_context {
> unsigned int idx;
> unsigned int stack_size;
> int b2p[ARRAY_SIZE(b2p)];
> + unsigned int exentry_idx;
> };
>
> +#define BPF_FIXUP_LEN 2 /* Two instructions => 8 bytes */
> +
> static inline void bpf_flush_icache(void *start, void *end)
> {
> smp_wmb(); /* smp write barrier */
> @@ -166,11 +169,14 @@ static inline void bpf_clear_seen_register(struct codegen_context *ctx, int i)
>
> void bpf_jit_emit_func_call_rel(u32 *image, struct codegen_context *ctx, u64 func);
> int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
> - u32 *addrs);
> + u32 *addrs, int pass);
> void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx);
> void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx);
> void bpf_jit_realloc_regs(struct codegen_context *ctx);
>
> +int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct codegen_context *ctx,
> + int insn_idx, int jmp_off, int dst_reg);
> +
> #endif
>
> #endif
> diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
> index c5c9e8ad1de7..f02457c6b54f 100644
> --- a/arch/powerpc/net/bpf_jit_comp.c
> +++ b/arch/powerpc/net/bpf_jit_comp.c
> @@ -101,6 +101,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> struct bpf_prog *tmp_fp;
> bool bpf_blinded = false;
> bool extra_pass = false;
> + u32 extable_len;
> + u32 fixup_len;
>
> if (!fp->jit_requested)
> return org_fp;
> @@ -131,7 +133,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> image = jit_data->image;
> bpf_hdr = jit_data->header;
> proglen = jit_data->proglen;
> - alloclen = proglen + FUNCTION_DESCR_SIZE;
> extra_pass = true;
> goto skip_init_ctx;
> }
> @@ -149,7 +150,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
>
> /* Scouting faux-generate pass 0 */
> - if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
> + if (bpf_jit_build_body(fp, 0, &cgctx, addrs, 0)) {
> /* We hit something illegal or unsupported. */
> fp = org_fp;
> goto out_addrs;
> @@ -162,7 +163,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> */
> if (cgctx.seen & SEEN_TAILCALL) {
> cgctx.idx = 0;
> - if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
> + if (bpf_jit_build_body(fp, 0, &cgctx, addrs, 0)) {
> fp = org_fp;
> goto out_addrs;
> }
> @@ -177,8 +178,11 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> bpf_jit_build_prologue(0, &cgctx);
> bpf_jit_build_epilogue(0, &cgctx);
>
> + fixup_len = fp->aux->num_exentries * BPF_FIXUP_LEN * 4;
> + extable_len = fp->aux->num_exentries * sizeof(struct exception_table_entry);
> +
> proglen = cgctx.idx * 4;
> - alloclen = proglen + FUNCTION_DESCR_SIZE;
> + alloclen = proglen + FUNCTION_DESCR_SIZE + fixup_len + extable_len;
>
> bpf_hdr = bpf_jit_binary_alloc(alloclen, &image, 4, bpf_jit_fill_ill_insns);
> if (!bpf_hdr) {
> @@ -186,6 +190,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> goto out_addrs;
> }
>
> + if (extable_len)
> + fp->aux->extable = (void *)image + FUNCTION_DESCR_SIZE + proglen + fixup_len;
> +
> skip_init_ctx:
> code_base = (u32 *)(image + FUNCTION_DESCR_SIZE);
>
> @@ -210,7 +217,11 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
> /* Now build the prologue, body code & epilogue for real. */
> cgctx.idx = 0;
> bpf_jit_build_prologue(code_base, &cgctx);
> - bpf_jit_build_body(fp, code_base, &cgctx, addrs);
> + if (bpf_jit_build_body(fp, code_base, &cgctx, addrs, pass)) {
> + bpf_jit_binary_free(bpf_hdr);
> + fp = org_fp;
> + goto out_addrs;
> + }
> bpf_jit_build_epilogue(code_base, &cgctx);
>
> if (bpf_jit_enable > 1)
> @@ -234,7 +245,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>
> fp->bpf_func = (void *)image;
> fp->jited = 1;
> - fp->jited_len = alloclen;
> + fp->jited_len = proglen + FUNCTION_DESCR_SIZE;
>
> bpf_flush_icache(bpf_hdr, (u8 *)bpf_hdr + (bpf_hdr->pages * PAGE_SIZE));
> bpf_jit_binary_lock_ro(bpf_hdr);
> @@ -258,3 +269,50 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
>
> return fp;
> }
> +
> +/*
> + * The caller should check for (BPF_MODE(code) == BPF_PROBE_MEM) before calling
> + * this function, as this only applies to BPF_PROBE_MEM, for now.
> + */
> +int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct codegen_context *ctx,
> + int insn_idx, int jmp_off, int dst_reg)
> +{
> + off_t offset;
> + unsigned long pc;
> + struct exception_table_entry *ex;
> + u32 *fixup;
> +
> + /* Populate extable entries only in the last pass */
> + if (pass != 2)
> + return 0;
> +
> + if (!fp->aux->extable ||
> + WARN_ON_ONCE(ctx->exentry_idx >= fp->aux->num_exentries))
> + return -EINVAL;
> +
> + pc = (unsigned long)&image[insn_idx];
> +
> + fixup = (void *)fp->aux->extable -
> + (fp->aux->num_exentries * BPF_FIXUP_LEN * 4) +
> + (ctx->exentry_idx * BPF_FIXUP_LEN * 4);
> +
> + fixup[0] = PPC_RAW_LI(dst_reg, 0);
> +
> + fixup[BPF_FIXUP_LEN - 1] =
> + PPC_RAW_BRANCH((long)(pc + jmp_off) - (long)&fixup[BPF_FIXUP_LEN - 1]);
> +
> + ex = &fp->aux->extable[ctx->exentry_idx];
> +
> + offset = pc - (long)&ex->insn;
> + if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN))
> + return -ERANGE;
> + ex->insn = offset;
> +
> + offset = (long)fixup - (long)&ex->fixup;
> + if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN))
> + return -ERANGE;
> + ex->fixup = offset;
> +
> + ctx->exentry_idx++;
> + return 0;
> +}
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index 6e4956cd52e7..820c7848434e 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -266,7 +266,7 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
>
> /* Assemble the body code between the prologue & epilogue */
> int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
> - u32 *addrs)
> + u32 *addrs, int pass)
> {
> const struct bpf_insn *insn = fp->insnsi;
> int flen = fp->len;
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 991eb43d4cd2..506934c13ef7 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -272,7 +272,7 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
>
> /* Assemble the body code between the prologue & epilogue */
> int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
> - u32 *addrs)
> + u32 *addrs, int pass)
> {
> const struct bpf_insn *insn = fp->insnsi;
> int flen = fp->len;
> @@ -718,14 +718,22 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> /* dst = *(u8 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_B:
> fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_B:
> + fallthrough;
fallthrough not needed (see my comment to patch 4)
> /* dst = *(u16 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_H:
> fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_H:
> + fallthrough;
> /* dst = *(u32 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_W:
> fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_W:
> + fallthrough;
> /* dst = *(u64 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_DW:
> + fallthrough;
> + case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
> switch (size) {
> case BPF_B:
> EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> @@ -749,6 +757,13 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
>
> if (size != BPF_DW && insn_is_zext(&insn[i + 1]))
> addrs[++i] = ctx->idx * 4;
> +
> + if (BPF_MODE(code) == BPF_PROBE_MEM) {
> + ret = bpf_add_extable_entry(fp, image, pass, ctx, ctx->idx - 1,
> + 4, dst_reg);
> + if (ret)
> + return ret;
> + }
> break;
>
> /*
>
^ permalink raw reply
* Re: [PATCH v3 3/8] bpf powerpc: refactor JIT compiler code
From: Christophe Leroy @ 2021-09-21 14:16 UTC (permalink / raw)
To: Hari Bathini, naveen.n.rao, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-4-hbathini@linux.ibm.com>
Le 21/09/2021 à 15:29, Hari Bathini a écrit :
> Refactor powerpc LDX JITing code to simplify adding BPF_PROBE_MEM
> support.
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v3:
> * Dropped ST(X) JITing coderefactoring and ensured the changes are
> minimal and relevant to BPF_PROBE_MEM support.
> * Added a default for size switch case to ensure compiler would
> not complain.
>
>
> arch/powerpc/net/bpf_jit_comp32.c | 42 ++++++++++++++++++++-----------
> arch/powerpc/net/bpf_jit_comp64.c | 40 +++++++++++++++++++----------
> 2 files changed, 55 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index b60b59426a24..6e4956cd52e7 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -282,6 +282,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> u32 src_reg = bpf_to_ppc(ctx, insn[i].src_reg);
> u32 src_reg_h = src_reg - 1;
> u32 tmp_reg = bpf_to_ppc(ctx, TMP_REG);
> + u32 size = BPF_SIZE(code);
> s16 off = insn[i].off;
> s32 imm = insn[i].imm;
> bool func_addr_fixed;
> @@ -810,23 +811,36 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> * BPF_LDX
> */
> case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> - if (!fp->aux->verifier_zext)
> - EMIT(PPC_RAW_LI(dst_reg_h, 0));
> - break;
> + fallthrough;
I know I commented to add 'fallthrough' ... In fact I missinterpreted
what I saw.
fallthrough is required only if you have code inbetween two 'case'.
When you have two following 'case' without code, you don't need
'fallthrough' I think.
> case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> - if (!fp->aux->verifier_zext)
> - EMIT(PPC_RAW_LI(dst_reg_h, 0));
> - break;
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> - if (!fp->aux->verifier_zext)
> - EMIT(PPC_RAW_LI(dst_reg_h, 0));
> - break;
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LWZ(dst_reg_h, src_reg, off));
> - EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off + 4));
> + switch (size) {
> + case BPF_B:
> + EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> + break;
> + case BPF_H:
> + EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> + break;
> + case BPF_W:
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> + break;
> + case BPF_DW:
> + EMIT(PPC_RAW_LWZ(dst_reg_h, src_reg, off));
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off + 4));
> + break;
> + /*
> + * With size not being an enum and BPF_B/H/W/DW being macros, ensure no
> + * compiler warning/error by adding a default case that never reaches.
> + */
> + default:
> + break;
> + }
> +
> + if (size != BPF_DW && !fp->aux->verifier_zext)
> + EMIT(PPC_RAW_LI(dst_reg_h, 0));
> break;
>
> /*
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 2a87da50d9a4..991eb43d4cd2 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -285,6 +285,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> u32 code = insn[i].code;
> u32 dst_reg = b2p[insn[i].dst_reg];
> u32 src_reg = b2p[insn[i].src_reg];
> + u32 size = BPF_SIZE(code);
> s16 off = insn[i].off;
> s32 imm = insn[i].imm;
> bool func_addr_fixed;
> @@ -716,25 +717,38 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> */
> /* dst = *(u8 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_B:
> - EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> - if (insn_is_zext(&insn[i + 1]))
> - addrs[++i] = ctx->idx * 4;
> - break;
> + fallthrough;
> /* dst = *(u16 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_H:
> - EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> - if (insn_is_zext(&insn[i + 1]))
> - addrs[++i] = ctx->idx * 4;
> - break;
> + fallthrough;
> /* dst = *(u32 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_W:
> - EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> - if (insn_is_zext(&insn[i + 1]))
> - addrs[++i] = ctx->idx * 4;
> - break;
> + fallthrough;
> /* dst = *(u64 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_DW:
> - PPC_BPF_LL(dst_reg, src_reg, off);
> + switch (size) {
> + case BPF_B:
> + EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> + break;
> + case BPF_H:
> + EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> + break;
> + case BPF_W:
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> + break;
> + case BPF_DW:
> + PPC_BPF_LL(dst_reg, src_reg, off);
> + break;
> + /*
> + * With size not being an enum and BPF_B/H/W/DW being macros, ensure no
> + * compiler warning/error by adding a default case that never reaches.
> + */
> + default:
> + break;
> + }
> +
> + if (size != BPF_DW && insn_is_zext(&insn[i + 1]))
> + addrs[++i] = ctx->idx * 4;
> break;
>
> /*
>
^ permalink raw reply
* Re: [PATCH v3 3/8] bpf powerpc: refactor JIT compiler code
From: Christophe Leroy @ 2021-09-21 14:02 UTC (permalink / raw)
To: Hari Bathini, naveen.n.rao, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-4-hbathini@linux.ibm.com>
Le 21/09/2021 à 15:29, Hari Bathini a écrit :
> Refactor powerpc LDX JITing code to simplify adding BPF_PROBE_MEM
> support.
>
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>
> Changes in v3:
> * Dropped ST(X) JITing coderefactoring and ensured the changes are
> minimal and relevant to BPF_PROBE_MEM support.
> * Added a default for size switch case to ensure compiler would
> not complain.
>
>
> arch/powerpc/net/bpf_jit_comp32.c | 42 ++++++++++++++++++++-----------
> arch/powerpc/net/bpf_jit_comp64.c | 40 +++++++++++++++++++----------
> 2 files changed, 55 insertions(+), 27 deletions(-)
>
> diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
> index b60b59426a24..6e4956cd52e7 100644
> --- a/arch/powerpc/net/bpf_jit_comp32.c
> +++ b/arch/powerpc/net/bpf_jit_comp32.c
> @@ -282,6 +282,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> u32 src_reg = bpf_to_ppc(ctx, insn[i].src_reg);
> u32 src_reg_h = src_reg - 1;
> u32 tmp_reg = bpf_to_ppc(ctx, TMP_REG);
> + u32 size = BPF_SIZE(code);
> s16 off = insn[i].off;
> s32 imm = insn[i].imm;
> bool func_addr_fixed;
> @@ -810,23 +811,36 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> * BPF_LDX
> */
> case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> - if (!fp->aux->verifier_zext)
> - EMIT(PPC_RAW_LI(dst_reg_h, 0));
> - break;
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> - if (!fp->aux->verifier_zext)
> - EMIT(PPC_RAW_LI(dst_reg_h, 0));
> - break;
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> - if (!fp->aux->verifier_zext)
> - EMIT(PPC_RAW_LI(dst_reg_h, 0));
> - break;
> + fallthrough;
> case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
> - EMIT(PPC_RAW_LWZ(dst_reg_h, src_reg, off));
> - EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off + 4));
> + switch (size) {
> + case BPF_B:
> + EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> + break;
> + case BPF_H:
> + EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> + break;
> + case BPF_W:
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> + break;
> + case BPF_DW:
> + EMIT(PPC_RAW_LWZ(dst_reg_h, src_reg, off));
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off + 4));
> + break;
> + /*
> + * With size not being an enum and BPF_B/H/W/DW being macros, ensure no
> + * compiler warning/error by adding a default case that never reaches.
> + */
Thinking about it once more, in fact this is already bounded by the
upper switch/case, so there is no possibility to end up here with
something else than the 4 cases and that's probably the reason why GCC
says nothing about it, so I now think that a default is unnecessary and
should not be added. Sorry for that.
> + default:
> + break;
> + }
> +
> + if (size != BPF_DW && !fp->aux->verifier_zext)
> + EMIT(PPC_RAW_LI(dst_reg_h, 0));
> break;
>
> /*
> diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
> index 2a87da50d9a4..991eb43d4cd2 100644
> --- a/arch/powerpc/net/bpf_jit_comp64.c
> +++ b/arch/powerpc/net/bpf_jit_comp64.c
> @@ -285,6 +285,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> u32 code = insn[i].code;
> u32 dst_reg = b2p[insn[i].dst_reg];
> u32 src_reg = b2p[insn[i].src_reg];
> + u32 size = BPF_SIZE(code);
> s16 off = insn[i].off;
> s32 imm = insn[i].imm;
> bool func_addr_fixed;
> @@ -716,25 +717,38 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
> */
> /* dst = *(u8 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_B:
> - EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> - if (insn_is_zext(&insn[i + 1]))
> - addrs[++i] = ctx->idx * 4;
> - break;
> + fallthrough;
> /* dst = *(u16 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_H:
> - EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> - if (insn_is_zext(&insn[i + 1]))
> - addrs[++i] = ctx->idx * 4;
> - break;
> + fallthrough;
> /* dst = *(u32 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_W:
> - EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> - if (insn_is_zext(&insn[i + 1]))
> - addrs[++i] = ctx->idx * 4;
> - break;
> + fallthrough;
> /* dst = *(u64 *)(ul) (src + off) */
> case BPF_LDX | BPF_MEM | BPF_DW:
> - PPC_BPF_LL(dst_reg, src_reg, off);
> + switch (size) {
> + case BPF_B:
> + EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
> + break;
> + case BPF_H:
> + EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
> + break;
> + case BPF_W:
> + EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
> + break;
> + case BPF_DW:
> + PPC_BPF_LL(dst_reg, src_reg, off);
> + break;
> + /*
> + * With size not being an enum and BPF_B/H/W/DW being macros, ensure no
> + * compiler warning/error by adding a default case that never reaches.
> + */
Same
> + default:
> + break;
> + }
> +
> + if (size != BPF_DW && insn_is_zext(&insn[i + 1]))
> + addrs[++i] = ctx->idx * 4;
> break;
>
> /*
>
^ permalink raw reply
* [PATCH v3 8/8] bpf ppc32: Access only if addr is kernel address
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai, Hari Bathini
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
With KUAP enabled, any kernel code which wants to access userspace
needs to be surrounded by disable-enable KUAP. But that is not
happening for BPF_PROBE_MEM load instruction. Though PPC32 does not
support read protection, considering the fact that PTR_TO_BTF_ID
(which uses BPF_PROBE_MEM mode) could either be a valid kernel pointer
or NULL but should never be a pointer to userspace address, execute
BPF_PROBE_MEM load only if addr is kernel address, otherwise set
dst_reg=0 and move on.
This will catch NULL, valid or invalid userspace pointers. Only bad
kernel pointer will be handled by BPF exception table.
[Alexei suggested for x86]
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v3:
* Updated jump for PPC_BCC to always be the same while emitting
a NOP instruction when needed.
arch/powerpc/net/bpf_jit_comp32.c | 35 +++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index 1239643f532c..59849e1230d2 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -825,6 +825,41 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
fallthrough;
case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
+ /*
+ * As PTR_TO_BTF_ID that uses BPF_PROBE_MEM mode could either be a valid
+ * kernel pointer or NULL but not a userspace address, execute BPF_PROBE_MEM
+ * load only if addr is kernel address (see is_kernel_addr()), otherwise
+ * set dst_reg=0 and move on.
+ */
+ if (BPF_MODE(code) == BPF_PROBE_MEM) {
+ EMIT(PPC_RAW_ADDI(b2p[TMP_REG], src_reg, off));
+ PPC_LI32(_R0, TASK_SIZE);
+ EMIT(PPC_RAW_CMPLW(b2p[TMP_REG], _R0));
+ PPC_BCC(COND_GT, (ctx->idx + 5) * 4);
+ EMIT(PPC_RAW_LI(dst_reg, 0));
+ /*
+ * For BPF_DW case, "li reg_h,0" would be needed when
+ * !fp->aux->verifier_zext. Emit NOP otherwise.
+ *
+ * Note that "li reg_h,0" is emitted for BPF_B/H/W case,
+ * if necessary. So, jump there insted of emitting an
+ * additional "li reg_h,0" instruction.
+ */
+ if (size == BPF_DW && !fp->aux->verifier_zext)
+ EMIT(PPC_RAW_LI(dst_reg_h, 0));
+ else
+ EMIT(PPC_RAW_NOP());
+ /*
+ * Need to jump two instructions instead of one for BPF_DW case
+ * as there are two load instructions for dst_reg_h & dst_reg
+ * respectively.
+ */
+ if (size == BPF_DW)
+ PPC_JMP((ctx->idx + 3) * 4);
+ else
+ PPC_JMP((ctx->idx + 2) * 4);
+ }
+
switch (size) {
case BPF_B:
EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
--
2.31.1
^ permalink raw reply related
* [PATCH v3 7/8] bpf ppc32: Add BPF_PROBE_MEM support for JIT
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai, Hari Bathini
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
BPF load instruction with BPF_PROBE_MEM mode can cause a fault
inside kernel. Append exception table for such instructions
within BPF program.
Unlike other archs which uses extable 'fixup' field to pass dest_reg
and nip, BPF exception table on PowerPC follows the generic PowerPC
exception table design, where it populates both fixup and extable
sections within BPF program. fixup section contains 3 instructions,
first 2 instructions clear dest_reg (lower & higher 32-bit registers)
and last instruction jumps to next instruction in the BPF code.
extable 'insn' field contains relative offset of the instruction and
'fixup' field contains relative offset of the fixup entry. Example
layout of BPF program with extable present:
+------------------+
| |
| |
0x4020 -->| lwz r28,4(r4) |
| |
| |
0x40ac -->| lwz r3,0(r24) |
| lwz r4,4(r24) |
| |
| |
|------------------|
0x4278 -->| li r28,0 | \
| li r27,0 | | fixup entry
| b 0x4024 | /
0x4284 -->| li r4,0 |
| li r3,0 |
| b 0x40b4 |
|------------------|
0x4290 -->| insn=0xfffffd90 | \ extable entry
| fixup=0xffffffe4 | /
0x4298 -->| insn=0xfffffe14 |
| fixup=0xffffffe8 |
+------------------+
(Addresses shown here are chosen random, not real)
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v3:
* Changed how BPF_FIXUP_LEN is defined based on Chris' suggestion.
arch/powerpc/net/bpf_jit.h | 4 ++++
arch/powerpc/net/bpf_jit_comp.c | 2 ++
arch/powerpc/net/bpf_jit_comp32.c | 34 +++++++++++++++++++++++++++++++
3 files changed, 40 insertions(+)
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 561689a2abdf..800734056200 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -144,7 +144,11 @@ struct codegen_context {
unsigned int exentry_idx;
};
+#ifdef CONFIG_PPC32
+#define BPF_FIXUP_LEN 3 /* Three instructions => 12 bytes */
+#else
#define BPF_FIXUP_LEN 2 /* Two instructions => 8 bytes */
+#endif
static inline void bpf_flush_icache(void *start, void *end)
{
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index f02457c6b54f..1a0041997050 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -297,6 +297,8 @@ int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct code
(ctx->exentry_idx * BPF_FIXUP_LEN * 4);
fixup[0] = PPC_RAW_LI(dst_reg, 0);
+ if (IS_ENABLED(CONFIG_PPC32))
+ fixup[1] = PPC_RAW_LI(dst_reg - 1, 0); /* clear higher 32-bit register too */
fixup[BPF_FIXUP_LEN - 1] =
PPC_RAW_BRANCH((long)(pc + jmp_off) - (long)&fixup[BPF_FIXUP_LEN - 1]);
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index 820c7848434e..1239643f532c 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -812,11 +812,19 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
*/
case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */
fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_B:
+ fallthrough;
case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */
fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_H:
+ fallthrough;
case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */
fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_W:
+ fallthrough;
case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
+ fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
switch (size) {
case BPF_B:
EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
@@ -841,6 +849,32 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
if (size != BPF_DW && !fp->aux->verifier_zext)
EMIT(PPC_RAW_LI(dst_reg_h, 0));
+
+ if (BPF_MODE(code) == BPF_PROBE_MEM) {
+ int insn_idx = ctx->idx - 1;
+ int jmp_off = 4;
+
+ /*
+ * In case of BPF_DW, two lwz instructions are emitted, one
+ * for higher 32-bit and another for lower 32-bit. So, set
+ * ex->insn to the first of the two and jump over both
+ * instructions in fixup.
+ *
+ * Similarly, with !verifier_zext, two instructions are
+ * emitted for BPF_B/H/W case. So, set ex->insn to the
+ * instruction that could fault and skip over both
+ * instructions.
+ */
+ if (size == BPF_DW || !fp->aux->verifier_zext) {
+ insn_idx -= 1;
+ jmp_off += 4;
+ }
+
+ ret = bpf_add_extable_entry(fp, image, pass, ctx, insn_idx,
+ jmp_off, dst_reg);
+ if (ret)
+ return ret;
+ }
break;
/*
--
2.31.1
^ permalink raw reply related
* [PATCH v3 6/8] bpf ppc64: Access only if addr is kernel address
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: Ravi Bangoria, songliubraving, netdev, john.fastabend, andrii,
kpsingh, paulus, yhs, bpf, linuxppc-dev, kafai, Hari Bathini
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
On PPC64 with KUAP enabled, any kernel code which wants to
access userspace needs to be surrounded by disable-enable KUAP.
But that is not happening for BPF_PROBE_MEM load instruction.
So, when BPF program tries to access invalid userspace address,
page-fault handler considers it as bad KUAP fault:
Kernel attempted to read user page (d0000000) - exploit attempt? (uid: 0)
Considering the fact that PTR_TO_BTF_ID (which uses BPF_PROBE_MEM
mode) could either be a valid kernel pointer or NULL but should
never be a pointer to userspace address, execute BPF_PROBE_MEM load
only if addr is kernel address, otherwise set dst_reg=0 and move on.
This will catch NULL, valid or invalid userspace pointers. Only bad
kernel pointer will be handled by BPF exception table.
[Alexei suggested for x86]
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v3:
* Used is_kernel_addr() logic instead of using TASK_SIZE_MAX check
all the time.
* Addressed other comments from Christophe.
arch/powerpc/net/bpf_jit_comp64.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 506934c13ef7..06e1206a4266 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -734,6 +734,35 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
case BPF_LDX | BPF_MEM | BPF_DW:
fallthrough;
case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
+ /*
+ * As PTR_TO_BTF_ID that uses BPF_PROBE_MEM mode could either be a valid
+ * kernel pointer or NULL but not a userspace address, execute BPF_PROBE_MEM
+ * load only if addr is kernel address (see is_kernel_addr()), otherwise
+ * set dst_reg=0 and move on.
+ */
+ if (BPF_MODE(code) == BPF_PROBE_MEM) {
+ EMIT(PPC_RAW_ADDI(b2p[TMP_REG_1], src_reg, off));
+#ifdef CONFIG_PPC_BOOK3E_64
+ PPC_LI64(b2p[TMP_REG_2], 0x8000000000000000ul);
+#elif defined(CONFIG_PPC_BOOK3S_64)
+ PPC_LI64(b2p[TMP_REG_2], PAGE_OFFSET);
+#else
+ PPC_LI64(b2p[TMP_REG_2], TASK_SIZE);
+#endif
+ EMIT(PPC_RAW_CMPLD(b2p[TMP_REG_1], b2p[TMP_REG_2]));
+ PPC_BCC(COND_GT, (ctx->idx + 4) * 4);
+ EMIT(PPC_RAW_LI(dst_reg, 0));
+ /*
+ * Check if 'off' is word aligned because PPC_BPF_LL()
+ * (BPF_DW case) generates two instructions if 'off' is not
+ * word-aligned and one instruction otherwise.
+ */
+ if (BPF_SIZE(code) == BPF_DW && (off & 3))
+ PPC_JMP((ctx->idx + 3) * 4);
+ else
+ PPC_JMP((ctx->idx + 2) * 4);
+ }
+
switch (size) {
case BPF_B:
EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
--
2.31.1
^ permalink raw reply related
* [PATCH v3 5/8] bpf ppc64: Add BPF_PROBE_MEM support for JIT
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: Ravi Bangoria, songliubraving, netdev, john.fastabend, andrii,
kpsingh, paulus, yhs, bpf, linuxppc-dev, kafai, Hari Bathini
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
BPF load instruction with BPF_PROBE_MEM mode can cause a fault
inside kernel. Append exception table for such instructions
within BPF program.
Unlike other archs which uses extable 'fixup' field to pass dest_reg
and nip, BPF exception table on PowerPC follows the generic PowerPC
exception table design, where it populates both fixup and extable
sections within BPF program. fixup section contains two instructions,
first instruction clears dest_reg and 2nd jumps to next instruction
in the BPF code. extable 'insn' field contains relative offset of
the instruction and 'fixup' field contains relative offset of the
fixup entry. Example layout of BPF program with extable present:
+------------------+
| |
| |
0x4020 -->| ld r27,4(r3) |
| |
| |
0x40ac -->| lwz r3,0(r4) |
| |
| |
|------------------|
0x4280 -->| li r27,0 | \ fixup entry
| b 0x4024 | /
0x4288 -->| li r3,0 |
| b 0x40b0 |
|------------------|
0x4290 -->| insn=0xfffffd90 | \ extable entry
| fixup=0xffffffec | /
0x4298 -->| insn=0xfffffe14 |
| fixup=0xffffffec |
+------------------+
(Addresses shown here are chosen random, not real)
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v3:
* Made all changes for bpf_add_extable_entry() in this patch instead
of updating it again in patch #7.
* Fixed a coding style issue.
arch/powerpc/net/bpf_jit.h | 8 +++-
arch/powerpc/net/bpf_jit_comp.c | 70 ++++++++++++++++++++++++++++---
arch/powerpc/net/bpf_jit_comp32.c | 2 +-
arch/powerpc/net/bpf_jit_comp64.c | 17 +++++++-
4 files changed, 88 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 0c8f885b8f48..561689a2abdf 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -141,8 +141,11 @@ struct codegen_context {
unsigned int idx;
unsigned int stack_size;
int b2p[ARRAY_SIZE(b2p)];
+ unsigned int exentry_idx;
};
+#define BPF_FIXUP_LEN 2 /* Two instructions => 8 bytes */
+
static inline void bpf_flush_icache(void *start, void *end)
{
smp_wmb(); /* smp write barrier */
@@ -166,11 +169,14 @@ static inline void bpf_clear_seen_register(struct codegen_context *ctx, int i)
void bpf_jit_emit_func_call_rel(u32 *image, struct codegen_context *ctx, u64 func);
int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
- u32 *addrs);
+ u32 *addrs, int pass);
void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx);
void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx);
void bpf_jit_realloc_regs(struct codegen_context *ctx);
+int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct codegen_context *ctx,
+ int insn_idx, int jmp_off, int dst_reg);
+
#endif
#endif
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index c5c9e8ad1de7..f02457c6b54f 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -101,6 +101,8 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
struct bpf_prog *tmp_fp;
bool bpf_blinded = false;
bool extra_pass = false;
+ u32 extable_len;
+ u32 fixup_len;
if (!fp->jit_requested)
return org_fp;
@@ -131,7 +133,6 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
image = jit_data->image;
bpf_hdr = jit_data->header;
proglen = jit_data->proglen;
- alloclen = proglen + FUNCTION_DESCR_SIZE;
extra_pass = true;
goto skip_init_ctx;
}
@@ -149,7 +150,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
/* Scouting faux-generate pass 0 */
- if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
+ if (bpf_jit_build_body(fp, 0, &cgctx, addrs, 0)) {
/* We hit something illegal or unsupported. */
fp = org_fp;
goto out_addrs;
@@ -162,7 +163,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
*/
if (cgctx.seen & SEEN_TAILCALL) {
cgctx.idx = 0;
- if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
+ if (bpf_jit_build_body(fp, 0, &cgctx, addrs, 0)) {
fp = org_fp;
goto out_addrs;
}
@@ -177,8 +178,11 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
bpf_jit_build_prologue(0, &cgctx);
bpf_jit_build_epilogue(0, &cgctx);
+ fixup_len = fp->aux->num_exentries * BPF_FIXUP_LEN * 4;
+ extable_len = fp->aux->num_exentries * sizeof(struct exception_table_entry);
+
proglen = cgctx.idx * 4;
- alloclen = proglen + FUNCTION_DESCR_SIZE;
+ alloclen = proglen + FUNCTION_DESCR_SIZE + fixup_len + extable_len;
bpf_hdr = bpf_jit_binary_alloc(alloclen, &image, 4, bpf_jit_fill_ill_insns);
if (!bpf_hdr) {
@@ -186,6 +190,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
goto out_addrs;
}
+ if (extable_len)
+ fp->aux->extable = (void *)image + FUNCTION_DESCR_SIZE + proglen + fixup_len;
+
skip_init_ctx:
code_base = (u32 *)(image + FUNCTION_DESCR_SIZE);
@@ -210,7 +217,11 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
/* Now build the prologue, body code & epilogue for real. */
cgctx.idx = 0;
bpf_jit_build_prologue(code_base, &cgctx);
- bpf_jit_build_body(fp, code_base, &cgctx, addrs);
+ if (bpf_jit_build_body(fp, code_base, &cgctx, addrs, pass)) {
+ bpf_jit_binary_free(bpf_hdr);
+ fp = org_fp;
+ goto out_addrs;
+ }
bpf_jit_build_epilogue(code_base, &cgctx);
if (bpf_jit_enable > 1)
@@ -234,7 +245,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
fp->bpf_func = (void *)image;
fp->jited = 1;
- fp->jited_len = alloclen;
+ fp->jited_len = proglen + FUNCTION_DESCR_SIZE;
bpf_flush_icache(bpf_hdr, (u8 *)bpf_hdr + (bpf_hdr->pages * PAGE_SIZE));
bpf_jit_binary_lock_ro(bpf_hdr);
@@ -258,3 +269,50 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
return fp;
}
+
+/*
+ * The caller should check for (BPF_MODE(code) == BPF_PROBE_MEM) before calling
+ * this function, as this only applies to BPF_PROBE_MEM, for now.
+ */
+int bpf_add_extable_entry(struct bpf_prog *fp, u32 *image, int pass, struct codegen_context *ctx,
+ int insn_idx, int jmp_off, int dst_reg)
+{
+ off_t offset;
+ unsigned long pc;
+ struct exception_table_entry *ex;
+ u32 *fixup;
+
+ /* Populate extable entries only in the last pass */
+ if (pass != 2)
+ return 0;
+
+ if (!fp->aux->extable ||
+ WARN_ON_ONCE(ctx->exentry_idx >= fp->aux->num_exentries))
+ return -EINVAL;
+
+ pc = (unsigned long)&image[insn_idx];
+
+ fixup = (void *)fp->aux->extable -
+ (fp->aux->num_exentries * BPF_FIXUP_LEN * 4) +
+ (ctx->exentry_idx * BPF_FIXUP_LEN * 4);
+
+ fixup[0] = PPC_RAW_LI(dst_reg, 0);
+
+ fixup[BPF_FIXUP_LEN - 1] =
+ PPC_RAW_BRANCH((long)(pc + jmp_off) - (long)&fixup[BPF_FIXUP_LEN - 1]);
+
+ ex = &fp->aux->extable[ctx->exentry_idx];
+
+ offset = pc - (long)&ex->insn;
+ if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN))
+ return -ERANGE;
+ ex->insn = offset;
+
+ offset = (long)fixup - (long)&ex->fixup;
+ if (WARN_ON_ONCE(offset >= 0 || offset < INT_MIN))
+ return -ERANGE;
+ ex->fixup = offset;
+
+ ctx->exentry_idx++;
+ return 0;
+}
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index 6e4956cd52e7..820c7848434e 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -266,7 +266,7 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
/* Assemble the body code between the prologue & epilogue */
int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
- u32 *addrs)
+ u32 *addrs, int pass)
{
const struct bpf_insn *insn = fp->insnsi;
int flen = fp->len;
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 991eb43d4cd2..506934c13ef7 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -272,7 +272,7 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
/* Assemble the body code between the prologue & epilogue */
int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
- u32 *addrs)
+ u32 *addrs, int pass)
{
const struct bpf_insn *insn = fp->insnsi;
int flen = fp->len;
@@ -718,14 +718,22 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
/* dst = *(u8 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_B:
fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_B:
+ fallthrough;
/* dst = *(u16 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_H:
fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_H:
+ fallthrough;
/* dst = *(u32 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_W:
fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_W:
+ fallthrough;
/* dst = *(u64 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_DW:
+ fallthrough;
+ case BPF_LDX | BPF_PROBE_MEM | BPF_DW:
switch (size) {
case BPF_B:
EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
@@ -749,6 +757,13 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
if (size != BPF_DW && insn_is_zext(&insn[i + 1]))
addrs[++i] = ctx->idx * 4;
+
+ if (BPF_MODE(code) == BPF_PROBE_MEM) {
+ ret = bpf_add_extable_entry(fp, image, pass, ctx, ctx->idx - 1,
+ 4, dst_reg);
+ if (ret)
+ return ret;
+ }
break;
/*
--
2.31.1
^ permalink raw reply related
* [PATCH v3 4/8] powerpc/ppc-opcode: introduce PPC_RAW_BRANCH() macro
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai, Hari Bathini
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
Define and use PPC_RAW_BRANCH() macro instead of open coding it. This
macro is used while adding BPF_PROBE_MEM support.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
Changes in v3:
* Added Reviewed-by tag from Chris.
arch/powerpc/include/asm/ppc-opcode.h | 2 ++
arch/powerpc/net/bpf_jit.h | 4 ++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index baea657bc868..f50213e2a3e0 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h
@@ -566,6 +566,8 @@
#define PPC_RAW_MTSPR(spr, d) (0x7c0003a6 | ___PPC_RS(d) | __PPC_SPR(spr))
#define PPC_RAW_EIEIO() (0x7c0006ac)
+#define PPC_RAW_BRANCH(addr) (PPC_INST_BRANCH | ((addr) & 0x03fffffc))
+
/* Deal with instructions that older assemblers aren't aware of */
#define PPC_BCCTR_FLUSH stringify_in_c(.long PPC_INST_BCCTR_FLUSH)
#define PPC_CP_ABORT stringify_in_c(.long PPC_RAW_CP_ABORT)
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 411c63d945c7..0c8f885b8f48 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -24,8 +24,8 @@
#define EMIT(instr) PLANT_INSTR(image, ctx->idx, instr)
/* Long jump; (unconditional 'branch') */
-#define PPC_JMP(dest) EMIT(PPC_INST_BRANCH | \
- (((dest) - (ctx->idx * 4)) & 0x03fffffc))
+#define PPC_JMP(dest) EMIT(PPC_RAW_BRANCH((dest) - (ctx->idx * 4)))
+
/* blr; (unconditional 'branch' with link) to absolute address */
#define PPC_BL_ABS(dest) EMIT(PPC_INST_BL | \
(((dest) - (unsigned long)(image + ctx->idx)) & 0x03fffffc))
--
2.31.1
^ permalink raw reply related
* [PATCH v3 3/8] bpf powerpc: refactor JIT compiler code
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai, Hari Bathini
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
Refactor powerpc LDX JITing code to simplify adding BPF_PROBE_MEM
support.
Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
---
Changes in v3:
* Dropped ST(X) JITing coderefactoring and ensured the changes are
minimal and relevant to BPF_PROBE_MEM support.
* Added a default for size switch case to ensure compiler would
not complain.
arch/powerpc/net/bpf_jit_comp32.c | 42 ++++++++++++++++++++-----------
arch/powerpc/net/bpf_jit_comp64.c | 40 +++++++++++++++++++----------
2 files changed, 55 insertions(+), 27 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index b60b59426a24..6e4956cd52e7 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -282,6 +282,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
u32 src_reg = bpf_to_ppc(ctx, insn[i].src_reg);
u32 src_reg_h = src_reg - 1;
u32 tmp_reg = bpf_to_ppc(ctx, TMP_REG);
+ u32 size = BPF_SIZE(code);
s16 off = insn[i].off;
s32 imm = insn[i].imm;
bool func_addr_fixed;
@@ -810,23 +811,36 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
* BPF_LDX
*/
case BPF_LDX | BPF_MEM | BPF_B: /* dst = *(u8 *)(ul) (src + off) */
- EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
- if (!fp->aux->verifier_zext)
- EMIT(PPC_RAW_LI(dst_reg_h, 0));
- break;
+ fallthrough;
case BPF_LDX | BPF_MEM | BPF_H: /* dst = *(u16 *)(ul) (src + off) */
- EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
- if (!fp->aux->verifier_zext)
- EMIT(PPC_RAW_LI(dst_reg_h, 0));
- break;
+ fallthrough;
case BPF_LDX | BPF_MEM | BPF_W: /* dst = *(u32 *)(ul) (src + off) */
- EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
- if (!fp->aux->verifier_zext)
- EMIT(PPC_RAW_LI(dst_reg_h, 0));
- break;
+ fallthrough;
case BPF_LDX | BPF_MEM | BPF_DW: /* dst = *(u64 *)(ul) (src + off) */
- EMIT(PPC_RAW_LWZ(dst_reg_h, src_reg, off));
- EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off + 4));
+ switch (size) {
+ case BPF_B:
+ EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
+ break;
+ case BPF_H:
+ EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
+ break;
+ case BPF_W:
+ EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
+ break;
+ case BPF_DW:
+ EMIT(PPC_RAW_LWZ(dst_reg_h, src_reg, off));
+ EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off + 4));
+ break;
+ /*
+ * With size not being an enum and BPF_B/H/W/DW being macros, ensure no
+ * compiler warning/error by adding a default case that never reaches.
+ */
+ default:
+ break;
+ }
+
+ if (size != BPF_DW && !fp->aux->verifier_zext)
+ EMIT(PPC_RAW_LI(dst_reg_h, 0));
break;
/*
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index 2a87da50d9a4..991eb43d4cd2 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -285,6 +285,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
u32 code = insn[i].code;
u32 dst_reg = b2p[insn[i].dst_reg];
u32 src_reg = b2p[insn[i].src_reg];
+ u32 size = BPF_SIZE(code);
s16 off = insn[i].off;
s32 imm = insn[i].imm;
bool func_addr_fixed;
@@ -716,25 +717,38 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
*/
/* dst = *(u8 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_B:
- EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
- if (insn_is_zext(&insn[i + 1]))
- addrs[++i] = ctx->idx * 4;
- break;
+ fallthrough;
/* dst = *(u16 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_H:
- EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
- if (insn_is_zext(&insn[i + 1]))
- addrs[++i] = ctx->idx * 4;
- break;
+ fallthrough;
/* dst = *(u32 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_W:
- EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
- if (insn_is_zext(&insn[i + 1]))
- addrs[++i] = ctx->idx * 4;
- break;
+ fallthrough;
/* dst = *(u64 *)(ul) (src + off) */
case BPF_LDX | BPF_MEM | BPF_DW:
- PPC_BPF_LL(dst_reg, src_reg, off);
+ switch (size) {
+ case BPF_B:
+ EMIT(PPC_RAW_LBZ(dst_reg, src_reg, off));
+ break;
+ case BPF_H:
+ EMIT(PPC_RAW_LHZ(dst_reg, src_reg, off));
+ break;
+ case BPF_W:
+ EMIT(PPC_RAW_LWZ(dst_reg, src_reg, off));
+ break;
+ case BPF_DW:
+ PPC_BPF_LL(dst_reg, src_reg, off);
+ break;
+ /*
+ * With size not being an enum and BPF_B/H/W/DW being macros, ensure no
+ * compiler warning/error by adding a default case that never reaches.
+ */
+ default:
+ break;
+ }
+
+ if (size != BPF_DW && insn_is_zext(&insn[i + 1]))
+ addrs[++i] = ctx->idx * 4;
break;
/*
--
2.31.1
^ permalink raw reply related
* [PATCH v3 2/8] bpf powerpc: Remove extra_pass from bpf_jit_build_body()
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: Ravi Bangoria, songliubraving, netdev, john.fastabend, andrii,
kpsingh, paulus, yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
In case of extra_pass, usual JIT passes are always skipped. So,
extra_pass is always false while calling bpf_jit_build_body() and
can be removed.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
Changes in v3:
* Updated the changelog wording a bit.
arch/powerpc/net/bpf_jit.h | 2 +-
arch/powerpc/net/bpf_jit_comp.c | 6 +++---
arch/powerpc/net/bpf_jit_comp32.c | 4 ++--
arch/powerpc/net/bpf_jit_comp64.c | 4 ++--
4 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index d6267e93027a..411c63d945c7 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -166,7 +166,7 @@ static inline void bpf_clear_seen_register(struct codegen_context *ctx, int i)
void bpf_jit_emit_func_call_rel(u32 *image, struct codegen_context *ctx, u64 func);
int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
- u32 *addrs, bool extra_pass);
+ u32 *addrs);
void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx);
void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx);
void bpf_jit_realloc_regs(struct codegen_context *ctx);
diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 53aefee3fe70..c5c9e8ad1de7 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -149,7 +149,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
cgctx.stack_size = round_up(fp->aux->stack_depth, 16);
/* Scouting faux-generate pass 0 */
- if (bpf_jit_build_body(fp, 0, &cgctx, addrs, false)) {
+ if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
/* We hit something illegal or unsupported. */
fp = org_fp;
goto out_addrs;
@@ -162,7 +162,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
*/
if (cgctx.seen & SEEN_TAILCALL) {
cgctx.idx = 0;
- if (bpf_jit_build_body(fp, 0, &cgctx, addrs, false)) {
+ if (bpf_jit_build_body(fp, 0, &cgctx, addrs)) {
fp = org_fp;
goto out_addrs;
}
@@ -210,7 +210,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp)
/* Now build the prologue, body code & epilogue for real. */
cgctx.idx = 0;
bpf_jit_build_prologue(code_base, &cgctx);
- bpf_jit_build_body(fp, code_base, &cgctx, addrs, extra_pass);
+ bpf_jit_build_body(fp, code_base, &cgctx, addrs);
bpf_jit_build_epilogue(code_base, &cgctx);
if (bpf_jit_enable > 1)
diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c
index beb12cbc8c29..b60b59426a24 100644
--- a/arch/powerpc/net/bpf_jit_comp32.c
+++ b/arch/powerpc/net/bpf_jit_comp32.c
@@ -266,7 +266,7 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
/* Assemble the body code between the prologue & epilogue */
int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
- u32 *addrs, bool extra_pass)
+ u32 *addrs)
{
const struct bpf_insn *insn = fp->insnsi;
int flen = fp->len;
@@ -860,7 +860,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
case BPF_JMP | BPF_CALL:
ctx->seen |= SEEN_FUNC;
- ret = bpf_jit_get_func_addr(fp, &insn[i], extra_pass,
+ ret = bpf_jit_get_func_addr(fp, &insn[i], false,
&func_addr, &func_addr_fixed);
if (ret < 0)
return ret;
diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c
index b87a63dba9c8..2a87da50d9a4 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -272,7 +272,7 @@ static void bpf_jit_emit_tail_call(u32 *image, struct codegen_context *ctx, u32
/* Assemble the body code between the prologue & epilogue */
int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *ctx,
- u32 *addrs, bool extra_pass)
+ u32 *addrs)
{
const struct bpf_insn *insn = fp->insnsi;
int flen = fp->len;
@@ -769,7 +769,7 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, struct codegen_context *
case BPF_JMP | BPF_CALL:
ctx->seen |= SEEN_FUNC;
- ret = bpf_jit_get_func_addr(fp, &insn[i], extra_pass,
+ ret = bpf_jit_get_func_addr(fp, &insn[i], false,
&func_addr, &func_addr_fixed);
if (ret < 0)
return ret;
--
2.31.1
^ permalink raw reply related
* [PATCH v3 1/8] bpf powerpc: Remove unused SEEN_STACK
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: Ravi Bangoria, songliubraving, netdev, john.fastabend, andrii,
kpsingh, paulus, yhs, bpf, linuxppc-dev, kafai
In-Reply-To: <20210921132943.489732-1-hbathini@linux.ibm.com>
From: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
SEEN_STACK is unused on PowerPC. Remove it. Also, have
SEEN_TAILCALL use 0x40000000.
Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
Changes in v3:
* Added Reviewed-by tag from Chris.
arch/powerpc/net/bpf_jit.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 99fad093f43e..d6267e93027a 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h
@@ -116,8 +116,7 @@ static inline bool is_nearbranch(int offset)
#define COND_LE (CR0_GT | COND_CMP_FALSE)
#define SEEN_FUNC 0x20000000 /* might call external helpers */
-#define SEEN_STACK 0x40000000 /* uses BPF stack */
-#define SEEN_TAILCALL 0x80000000 /* uses tail calls */
+#define SEEN_TAILCALL 0x40000000 /* uses tail calls */
#define SEEN_VREG_MASK 0x1ff80000 /* Volatile registers r3-r12 */
#define SEEN_NVREG_MASK 0x0003ffff /* Non volatile registers r14-r31 */
--
2.31.1
^ permalink raw reply related
* [PATCH v3 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
From: Hari Bathini @ 2021-09-21 13:29 UTC (permalink / raw)
To: naveen.n.rao, christophe.leroy, mpe, ast, daniel
Cc: songliubraving, netdev, john.fastabend, andrii, kpsingh, paulus,
yhs, bpf, linuxppc-dev, kafai, Hari Bathini
Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT
compiler code with the aim to simplify adding BPF_PROBE_MEM support.
Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding
branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64
& PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace
pointers for PPC64 & PPC32 cases respectively.
Changes in v3:
* Addressed all the review comments from Christophe.
Hari Bathini (4):
bpf powerpc: refactor JIT compiler code
powerpc/ppc-opcode: introduce PPC_RAW_BRANCH() macro
bpf ppc32: Add BPF_PROBE_MEM support for JIT
bpf ppc32: Access only if addr is kernel address
Ravi Bangoria (4):
bpf powerpc: Remove unused SEEN_STACK
bpf powerpc: Remove extra_pass from bpf_jit_build_body()
bpf ppc64: Add BPF_PROBE_MEM support for JIT
bpf ppc64: Access only if addr is kernel address
arch/powerpc/include/asm/ppc-opcode.h | 2 +
arch/powerpc/net/bpf_jit.h | 19 +++--
arch/powerpc/net/bpf_jit_comp.c | 72 ++++++++++++++--
arch/powerpc/net/bpf_jit_comp32.c | 115 ++++++++++++++++++++++----
arch/powerpc/net/bpf_jit_comp64.c | 88 ++++++++++++++++----
5 files changed, 254 insertions(+), 42 deletions(-)
--
2.31.1
^ permalink raw reply
* Re: [RFC PATCH 5/8] sched: move CPU field back into thread_info if THREAD_INFO_IN_TASK=y
From: Catalin Marinas @ 2021-09-21 13:12 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
linux-s390, Arnd Bergmann, Russell King, Christian Borntraeger,
Ingo Molnar, Albert Ou, Kees Cook, Vasily Gorbik, Heiko Carstens,
Keith Packard, Borislav Petkov, Andy Lutomirski, Paul Walmsley,
Thomas Gleixner, linux-arm-kernel, linuxppc-dev, linux-kernel,
Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-6-ardb@kernel.org>
On Tue, Sep 14, 2021 at 02:10:33PM +0200, Ard Biesheuvel wrote:
> THREAD_INFO_IN_TASK moved the CPU field out of thread_info, but this
> causes some issues on architectures that define raw_smp_processor_id()
> in terms of this field, due to the fact that #include'ing linux/sched.h
> to get at struct task_struct is problematic in terms of circular
> dependencies.
>
> Given that thread_info and task_struct are the same data structure
> anyway when THREAD_INFO_IN_TASK=y, let's move it back so that having
> access to the type definition of struct thread_info is sufficient to
> reference the CPU number of the current task.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
^ permalink raw reply
* Re: [RFC PATCH 2/8] x86: add CPU field to struct thread_info
From: Borislav Petkov @ 2021-09-21 12:54 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
linux-s390, Arnd Bergmann, Russell King, Christian Borntraeger,
Ingo Molnar, Catalin Marinas, Albert Ou, Kees Cook, Vasily Gorbik,
Heiko Carstens, Keith Packard, Andy Lutomirski, Paul Walmsley,
Thomas Gleixner, linux-arm-kernel, linuxppc-dev, linux-kernel,
Palmer Dabbelt, Linus Torvalds
In-Reply-To: <20210914121036.3975026-3-ardb@kernel.org>
On Tue, Sep 14, 2021 at 02:10:30PM +0200, Ard Biesheuvel wrote:
> The CPU field will be moved back into thread_info even when
> THREAD_INFO_IN_TASK is enabled, so add it back to x86's definition of
> struct thread_info.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
> arch/x86/include/asm/thread_info.h | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/include/asm/thread_info.h b/arch/x86/include/asm/thread_info.h
> index cf132663c219..ebec69c35e95 100644
> --- a/arch/x86/include/asm/thread_info.h
> +++ b/arch/x86/include/asm/thread_info.h
> @@ -57,6 +57,9 @@ struct thread_info {
> unsigned long flags; /* low level flags */
> unsigned long syscall_work; /* SYSCALL_WORK_ flags */
> u32 status; /* thread synchronous flags */
> +#ifdef CONFIG_SMP
> + u32 cpu; /* current CPU */
> +#endif
> };
>
> #define INIT_THREAD_INFO(tsk) \
> --
Acked-by: Borislav Petkov <bp@suse.de>
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply
* Re: [RFC PATCH 0/8] Move task_struct::cpu back into thread_info
From: Ard Biesheuvel @ 2021-09-21 12:06 UTC (permalink / raw)
To: Mark Rutland, Michael Ellerman, Benjamin Herrenschmidt,
Palmer Dabbelt, Paul Walmsley, Albert Ou, Heiko Carstens,
Paul Mackerras, Borislav Petkov, Peter Zijlstra, Thomas Gleixner,
Christian Borntraeger, Vasily Gorbik, Ingo Molnar
Cc: open list:S390, Kees Cook, Arnd Bergmann, Catalin Marinas,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
Linux Kernel Mailing List, Russell King, Linus Torvalds,
Keith Packard, Andy Lutomirski, linux-riscv, Will Deacon,
Linux ARM
In-Reply-To: <20210914135527.GC30247@C02TD0UTHF1T.local>
On Tue, 14 Sept 2021 at 15:55, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, Sep 14, 2021 at 02:10:28PM +0200, Ard Biesheuvel wrote:
> > Commit c65eacbe290b ("sched/core: Allow putting thread_info into
> > task_struct") mentions that, along with moving thread_info into
> > task_struct, the cpu field is moved out of the former into the latter,
> > but does not explain why.
>
> From what I recall of talking to Andy around that time, when converting
> arm64 over, the theory was that over time we'd move more and more out of
> thread_info and into task_struct or thread_struct, until task_struct
> supplanted thread_info entirely, and that all became generic.
>
> I think the key gain there was making things more *generic*, and there
> are other ways we could do that in future without moving more into
> task_struct (e.g. with a geenric thread_info and arch_thread_info inside
> that).
>
> With that in mind, and given the diffstat, I think this is worthwhile.
>
> FWIW, for the series:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>
Thanks.
Any comments on this from the various arch maintainers? Especially
power, as Christophe seems happy with this but there are 3 different
patches affecting power that need a maintainer ack.
^ permalink raw reply
* Re: [RFC PATCH 1/8] arm64: add CPU field to struct thread_info
From: Ard Biesheuvel @ 2021-09-21 12:04 UTC (permalink / raw)
To: Catalin Marinas
Cc: Peter Zijlstra, Paul Mackerras, linux-riscv, Will Deacon,
open list:S390, Arnd Bergmann, Russell King,
Christian Borntraeger, Ingo Molnar, Albert Ou, Kees Cook,
Vasily Gorbik, Heiko Carstens, Keith Packard, Borislav Petkov,
Andy Lutomirski, Paul Walmsley, Thomas Gleixner, Linux ARM,
open list:LINUX FOR POWERPC (32-BIT AND 64-BIT),
Linux Kernel Mailing List, Palmer Dabbelt, Linus Torvalds
In-Reply-To: <YUNXfWKZ7XYvw2EK@arm.com>
On Thu, 16 Sept 2021 at 16:41, Catalin Marinas <catalin.marinas@arm.com> wrote:
>
> On Tue, Sep 14, 2021 at 02:10:29PM +0200, Ard Biesheuvel wrote:
> > The CPU field will be moved back into thread_info even when
> > THREAD_INFO_IN_TASK is enabled, so add it back to arm64's definition of
> > struct thread_info.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Thanks. I take it this applies to patch #5 as well?
^ permalink raw reply
* Re: [PATCH v3] lib/zlib_inflate/inffast: Check config in C to avoid unused function warning
From: Paul Menzel @ 2021-09-21 10:11 UTC (permalink / raw)
To: Nathan Chancellor
Cc: llvm, Zhen Lei, Nick Desaulniers, linux-kernel, Paul Mackerras,
Andrew Morton, linuxppc-dev
In-Reply-To: <YUishGbHeaDMJDj+@archlinux-ax161>
Dear Linux folks,
Am 20.09.21 um 17:45 schrieb Nathan Chancellor:
> On Mon, Sep 20, 2021 at 10:43:33AM +0200, Paul Menzel wrote:
>> Building Linux for ppc64le with Ubuntu clang version 12.0.0-3ubuntu1~21.04.1
>> shows the warning below.
>>
>> arch/powerpc/boot/inffast.c:20:1: warning: unused function 'get_unaligned16' [-Wunused-function]
>> get_unaligned16(const unsigned short *p)
>> ^
>> 1 warning generated.
>>
>> Fix it, by moving the check from the preprocessor to C, so the compiler
>> sees the use.
>>
>> Signed-off-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org>
>
>> ---
>> v2: Use IS_ENABLED
>> v3: Use if statement over ternary operator as requested by Christophe
>>
>> lib/zlib_inflate/inffast.c | 13 ++++++-------
>> 1 file changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/lib/zlib_inflate/inffast.c b/lib/zlib_inflate/inffast.c
>> index f19c4fbe1be7..2843f9bb42ac 100644
>> --- a/lib/zlib_inflate/inffast.c
>> +++ b/lib/zlib_inflate/inffast.c
>> @@ -253,13 +253,12 @@ void inflate_fast(z_streamp strm, unsigned start)
>>
>> sfrom = (unsigned short *)(from);
>> loops = len >> 1;
>> - do
>> -#ifdef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
>> - *sout++ = *sfrom++;
>> -#else
>> - *sout++ = get_unaligned16(sfrom++);
>> -#endif
>> - while (--loops);
>> + do {
>> + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS))
>> + *sout++ = *sfrom++;
>> + else
>> + *sout++ = get_unaligned16(sfrom++);
>> + } while (--loops);
>> out = (unsigned char *)sout;
>> from = (unsigned char *)sfrom;
>> } else { /* dist == 1 or dist == 2 */
>> --
>> 2.33.0
Just for the record,
I compared both object files by running `objdump -d`, and the result is
the same.
The binary differed (`vbindiff`), but I guess this is due to the
increased revision (`make bindeb-pkg`).
without a change (Linus’ current master):
0000 0B50: 00 00 00 00 00 00 00 00 1F 01 00 00 36 00 00 00 ........
....6...
^
0000 0B60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........
........
0000 0B70: 00 00 00 00 00 00 00 00 29 01 00 00 32 00 00 00 ........
)...2...
^
v2 (ternary operator):
0000 0B50: 00 00 00 00 00 00 00 00 1C 01 00 00 36 00 00 00 ........
....6...
0000 0B60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........
........
0000 0B70: 00 00 00 00 00 00 00 00 26 01 00 00 32 00 00 00 ........
&...2...
v3 (if-else statement):
0000 0B50: 00 00 00 00 00 00 00 00 1E 01 00 00 36 00 00 00 ........
....6...
0000 0B60: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ........
........
0000 0B70: 00 00 00 00 00 00 00 00 28 01 00 00 32 00 00 00 ........
(...2...
Kind regards,
Paul
^ permalink raw reply
* [PATCH] mm: Remove HARDENED_USERCOPY_FALLBACK
From: Stephen Kitt @ 2021-09-21 6:11 UTC (permalink / raw)
To: Christoph Lameter, Pekka Enberg, David Rientjes, Joonsoo Kim,
Andrew Morton, Vlastimil Babka, James Morris, Serge E . Hallyn
Cc: Stephen Kitt, Kees Cook, linux-kernel, linux-mm,
linux-security-module, linux-hardening, linuxppc-dev
This has served its purpose and is no longer used. All usercopy
violations appear to have been handled by now, any remaining
instances (or new bugs) will cause copies to be rejected.
This isn't a direct revert of commit 2d891fbc3bb6 ("usercopy: Allow
strict enforcement of whitelists"); since usercopy_fallback is
effectively 0, the fallback handling is removed too.
This also removes the usercopy_fallback module parameter on
slab_common.
Link: https://github.com/KSPP/linux/issues/153
Signed-off-by: Stephen Kitt <steve@sk2.org>
Suggested-by: Kees Cook <keescook@chromium.org>
---
arch/powerpc/configs/skiroot_defconfig | 1 -
include/linux/slab.h | 2 --
mm/slab.c | 13 -------------
mm/slab_common.c | 8 --------
mm/slub.c | 14 --------------
security/Kconfig | 14 --------------
6 files changed, 52 deletions(-)
diff --git a/arch/powerpc/configs/skiroot_defconfig b/arch/powerpc/configs/skiroot_defconfig
index b806a5d3a695..c3ba614c973d 100644
--- a/arch/powerpc/configs/skiroot_defconfig
+++ b/arch/powerpc/configs/skiroot_defconfig
@@ -275,7 +275,6 @@ CONFIG_NLS_UTF8=y
CONFIG_ENCRYPTED_KEYS=y
CONFIG_SECURITY=y
CONFIG_HARDENED_USERCOPY=y
-# CONFIG_HARDENED_USERCOPY_FALLBACK is not set
CONFIG_HARDENED_USERCOPY_PAGESPAN=y
CONFIG_FORTIFY_SOURCE=y
CONFIG_SECURITY_LOCKDOWN_LSM=y
diff --git a/include/linux/slab.h b/include/linux/slab.h
index 0c97d788762c..5b21515afae0 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -142,8 +142,6 @@ struct mem_cgroup;
void __init kmem_cache_init(void);
bool slab_is_available(void);
-extern bool usercopy_fallback;
-
struct kmem_cache *kmem_cache_create(const char *name, unsigned int size,
unsigned int align, slab_flags_t flags,
void (*ctor)(void *));
diff --git a/mm/slab.c b/mm/slab.c
index d0f725637663..4d826394ffcb 100644
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -4207,19 +4207,6 @@ void __check_heap_object(const void *ptr, unsigned long n, struct page *page,
n <= cachep->useroffset - offset + cachep->usersize)
return;
- /*
- * If the copy is still within the allocated object, produce
- * a warning instead of rejecting the copy. This is intended
- * to be a temporary method to find any missing usercopy
- * whitelists.
- */
- if (usercopy_fallback &&
- offset <= cachep->object_size &&
- n <= cachep->object_size - offset) {
- usercopy_warn("SLAB object", cachep->name, to_user, offset, n);
- return;
- }
-
usercopy_abort("SLAB object", cachep->name, to_user, offset, n);
}
#endif /* CONFIG_HARDENED_USERCOPY */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index a4a571428c51..925b00c1d4e8 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -37,14 +37,6 @@ LIST_HEAD(slab_caches);
DEFINE_MUTEX(slab_mutex);
struct kmem_cache *kmem_cache;
-#ifdef CONFIG_HARDENED_USERCOPY
-bool usercopy_fallback __ro_after_init =
- IS_ENABLED(CONFIG_HARDENED_USERCOPY_FALLBACK);
-module_param(usercopy_fallback, bool, 0400);
-MODULE_PARM_DESC(usercopy_fallback,
- "WARN instead of reject usercopy whitelist violations");
-#endif
-
static LIST_HEAD(slab_caches_to_rcu_destroy);
static void slab_caches_to_rcu_destroy_workfn(struct work_struct *work);
static DECLARE_WORK(slab_caches_to_rcu_destroy_work,
diff --git a/mm/slub.c b/mm/slub.c
index 3f96e099817a..77f53e76a3c3 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -4125,7 +4125,6 @@ void __check_heap_object(const void *ptr, unsigned long n, struct page *page,
{
struct kmem_cache *s;
unsigned int offset;
- size_t object_size;
bool is_kfence = is_kfence_address(ptr);
ptr = kasan_reset_tag(ptr);
@@ -4158,19 +4157,6 @@ void __check_heap_object(const void *ptr, unsigned long n, struct page *page,
n <= s->useroffset - offset + s->usersize)
return;
- /*
- * If the copy is still within the allocated object, produce
- * a warning instead of rejecting the copy. This is intended
- * to be a temporary method to find any missing usercopy
- * whitelists.
- */
- object_size = slab_ksize(s);
- if (usercopy_fallback &&
- offset <= object_size && n <= object_size - offset) {
- usercopy_warn("SLUB object", s->name, to_user, offset, n);
- return;
- }
-
usercopy_abort("SLUB object", s->name, to_user, offset, n);
}
#endif /* CONFIG_HARDENED_USERCOPY */
diff --git a/security/Kconfig b/security/Kconfig
index 0ced7fd33e4d..d9698900c9b7 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -163,20 +163,6 @@ config HARDENED_USERCOPY
or are part of the kernel text. This kills entire classes
of heap overflow exploits and similar kernel memory exposures.
-config HARDENED_USERCOPY_FALLBACK
- bool "Allow usercopy whitelist violations to fallback to object size"
- depends on HARDENED_USERCOPY
- default y
- help
- This is a temporary option that allows missing usercopy whitelists
- to be discovered via a WARN() to the kernel log, instead of
- rejecting the copy, falling back to non-whitelisted hardened
- usercopy that checks the slab allocation size instead of the
- whitelist size. This option will be removed once it seems like
- all missing usercopy whitelists have been identified and fixed.
- Booting with "slab_common.usercopy_fallback=Y/N" can change
- this setting.
-
config HARDENED_USERCOPY_PAGESPAN
bool "Refuse to copy allocations that span multiple pages"
depends on HARDENED_USERCOPY
base-commit: 368094df48e680fa51cedb68537408cfa64b788e
--
2.30.2
^ permalink raw reply related
* [PATCH] powerpc/paravirt: correct preempt debug splat in vcpu_is_preempted()
From: Nathan Lynch @ 2021-09-21 3:12 UTC (permalink / raw)
To: linuxppc-dev; +Cc: srikar, npiggin
vcpu_is_preempted() can be used outside of preempt-disabled critical
sections, yielding warnings such as:
BUG: using smp_processor_id() in preemptible [00000000] code: systemd-udevd/185
caller is rwsem_spin_on_owner+0x1cc/0x2d0
CPU: 1 PID: 185 Comm: systemd-udevd Not tainted 5.15.0-rc2+ #33
Call Trace:
[c000000012907ac0] [c000000000aa30a8] dump_stack_lvl+0xac/0x108 (unreliable)
[c000000012907b00] [c000000001371f70] check_preemption_disabled+0x150/0x160
[c000000012907b90] [c0000000001e0e8c] rwsem_spin_on_owner+0x1cc/0x2d0
[c000000012907be0] [c0000000001e1408] rwsem_down_write_slowpath+0x478/0x9a0
[c000000012907ca0] [c000000000576cf4] filename_create+0x94/0x1e0
[c000000012907d10] [c00000000057ac08] do_symlinkat+0x68/0x1a0
[c000000012907d70] [c00000000057ae18] sys_symlink+0x58/0x70
[c000000012907da0] [c00000000002e448] system_call_exception+0x198/0x3c0
[c000000012907e10] [c00000000000c54c] system_call_common+0xec/0x250
The result of vcpu_is_preempted() is always subject to invalidation by
events inside and outside of Linux; it's just a best guess at a point in
time. Use raw_smp_processor_id() to avoid such warnings.
Signed-off-by: Nathan Lynch <nathanl@linux.ibm.com>
Fixes: ca3f969dcb11 ("powerpc/paravirt: Use is_kvm_guest() in vcpu_is_preempted()")
---
arch/powerpc/include/asm/paravirt.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/include/asm/paravirt.h b/arch/powerpc/include/asm/paravirt.h
index bcb7b5f917be..e429aca566de 100644
--- a/arch/powerpc/include/asm/paravirt.h
+++ b/arch/powerpc/include/asm/paravirt.h
@@ -97,7 +97,14 @@ static inline bool vcpu_is_preempted(int cpu)
#ifdef CONFIG_PPC_SPLPAR
if (!is_kvm_guest()) {
- int first_cpu = cpu_first_thread_sibling(smp_processor_id());
+ int first_cpu;
+
+ /*
+ * This is only a guess at best, and this function may be
+ * called with preemption enabled. Using raw_smp_processor_id()
+ * does not damage accuracy.
+ */
+ first_cpu = cpu_first_thread_sibling(raw_smp_processor_id());
/*
* Preemption can only happen at core granularity. This CPU
--
2.31.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox