| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0473d5ad-1d52-43e0-ade6-34c94212a220@ghiti.fr>
Date: Thu, 1 Aug 2024 12:31:56 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Samuel Holland <samuel.holland@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, linux-riscv@...ts.infradead.org
Cc: Yury Norov <yury.norov@...il.com>,
Rasmus Villemoes <linux@...musvillemoes.dk>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] riscv: Enable bitops instrumentation
On 01/08/2024 05:37, Samuel Holland wrote:
> Instead of implementing the bitops functions directly in assembly,
> provide the arch_-prefixed versions and use the wrappers from
> asm-generic to add instrumentation. This improves KASAN coverage and
> fixes the kasan_bitops_generic() unit test.
>
> Signed-off-by: Samuel Holland <samuel.holland@...ive.com>
> ---
>
> arch/riscv/include/asm/bitops.h | 43 ++++++++++++++++++---------------
> 1 file changed, 23 insertions(+), 20 deletions(-)
>
> diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
> index 71af9ecfcfcb..fae152ea0508 100644
> --- a/arch/riscv/include/asm/bitops.h
> +++ b/arch/riscv/include/asm/bitops.h
> @@ -222,44 +222,44 @@ static __always_inline int variable_fls(unsigned int x)
> #define __NOT(x) (~(x))
>
> /**
> - * test_and_set_bit - Set a bit and return its old value
> + * arch_test_and_set_bit - Set a bit and return its old value
> * @nr: Bit to set
> * @addr: Address to count from
> *
> * This operation may be reordered on other architectures than x86.
> */
> -static inline int test_and_set_bit(int nr, volatile unsigned long *addr)
> +static inline int arch_test_and_set_bit(int nr, volatile unsigned long *addr)
> {
> return __test_and_op_bit(or, __NOP, nr, addr);
> }
>
> /**
> - * test_and_clear_bit - Clear a bit and return its old value
> + * arch_test_and_clear_bit - Clear a bit and return its old value
> * @nr: Bit to clear
> * @addr: Address to count from
> *
> * This operation can be reordered on other architectures other than x86.
> */
> -static inline int test_and_clear_bit(int nr, volatile unsigned long *addr)
> +static inline int arch_test_and_clear_bit(int nr, volatile unsigned long *addr)
> {
> return __test_and_op_bit(and, __NOT, nr, addr);
> }
>
> /**
> - * test_and_change_bit - Change a bit and return its old value
> + * arch_test_and_change_bit - Change a bit and return its old value
> * @nr: Bit to change
> * @addr: Address to count from
> *
> * This operation is atomic and cannot be reordered.
> * It also implies a memory barrier.
> */
> -static inline int test_and_change_bit(int nr, volatile unsigned long *addr)
> +static inline int arch_test_and_change_bit(int nr, volatile unsigned long *addr)
> {
> return __test_and_op_bit(xor, __NOP, nr, addr);
> }
>
> /**
> - * set_bit - Atomically set a bit in memory
> + * arch_set_bit - Atomically set a bit in memory
> * @nr: the bit to set
> * @addr: the address to start counting from
> *
> @@ -270,13 +270,13 @@ static inline int test_and_change_bit(int nr, volatile unsigned long *addr)
> * Note that @nr may be almost arbitrarily large; this function is not
> * restricted to acting on a single-word quantity.
> */
> -static inline void set_bit(int nr, volatile unsigned long *addr)
> +static inline void arch_set_bit(int nr, volatile unsigned long *addr)
> {
> __op_bit(or, __NOP, nr, addr);
> }
>
> /**
> - * clear_bit - Clears a bit in memory
> + * arch_clear_bit - Clears a bit in memory
> * @nr: Bit to clear
> * @addr: Address to start counting from
> *
> @@ -284,13 +284,13 @@ static inline void set_bit(int nr, volatile unsigned long *addr)
> * on non x86 architectures, so if you are writing portable code,
> * make sure not to rely on its reordering guarantees.
> */
> -static inline void clear_bit(int nr, volatile unsigned long *addr)
> +static inline void arch_clear_bit(int nr, volatile unsigned long *addr)
> {
> __op_bit(and, __NOT, nr, addr);
> }
>
> /**
> - * change_bit - Toggle a bit in memory
> + * arch_change_bit - Toggle a bit in memory
> * @nr: Bit to change
> * @addr: Address to start counting from
> *
> @@ -298,40 +298,40 @@ static inline void clear_bit(int nr, volatile unsigned long *addr)
> * Note that @nr may be almost arbitrarily large; this function is not
> * restricted to acting on a single-word quantity.
> */
> -static inline void change_bit(int nr, volatile unsigned long *addr)
> +static inline void arch_change_bit(int nr, volatile unsigned long *addr)
> {
> __op_bit(xor, __NOP, nr, addr);
> }
>
> /**
> - * test_and_set_bit_lock - Set a bit and return its old value, for lock
> + * arch_test_and_set_bit_lock - Set a bit and return its old value, for lock
> * @nr: Bit to set
> * @addr: Address to count from
> *
> * This operation is atomic and provides acquire barrier semantics.
> * It can be used to implement bit locks.
> */
> -static inline int test_and_set_bit_lock(
> +static inline int arch_test_and_set_bit_lock(
> unsigned long nr, volatile unsigned long *addr)
> {
> return __test_and_op_bit_ord(or, __NOP, nr, addr, .aq);
> }
>
> /**
> - * clear_bit_unlock - Clear a bit in memory, for unlock
> + * arch_clear_bit_unlock - Clear a bit in memory, for unlock
> * @nr: the bit to set
> * @addr: the address to start counting from
> *
> * This operation is atomic and provides release barrier semantics.
> */
> -static inline void clear_bit_unlock(
> +static inline void arch_clear_bit_unlock(
> unsigned long nr, volatile unsigned long *addr)
> {
> __op_bit_ord(and, __NOT, nr, addr, .rl);
> }
>
> /**
> - * __clear_bit_unlock - Clear a bit in memory, for unlock
> + * arch___clear_bit_unlock - Clear a bit in memory, for unlock
> * @nr: the bit to set
> * @addr: the address to start counting from
> *
Reviewed-by: Alexandre Ghiti <alexghiti@...osinc.com>
Tested-by: Alexandre Ghiti <alexghiti@...osinc.com>
Thanks,
Alex
> @@ -345,13 +345,13 @@ static inline void clear_bit_unlock(
> * non-atomic property here: it's a lot more instructions and we still have to
> * provide release semantics anyway.
> */
> -static inline void __clear_bit_unlock(
> +static inline void arch___clear_bit_unlock(
> unsigned long nr, volatile unsigned long *addr)
> {
> - clear_bit_unlock(nr, addr);
> + arch_clear_bit_unlock(nr, addr);
> }
>
> -static inline bool xor_unlock_is_negative_byte(unsigned long mask,
> +static inline bool arch_xor_unlock_is_negative_byte(unsigned long mask,
> volatile unsigned long *addr)
> {
> unsigned long res;
> @@ -369,6 +369,9 @@ static inline bool xor_unlock_is_negative_byte(unsigned long mask,
> #undef __NOT
> #undef __AMO
>
> +#include <asm-generic/bitops/instrumented-atomic.h>
> +#include <asm-generic/bitops/instrumented-lock.h>
> +
> #include <asm-generic/bitops/non-atomic.h>
> #include <asm-generic/bitops/le.h>
> #include <asm-generic/bitops/ext2-atomic.h>
Powered by blists - more mailing lists