* Kees Cook <keesc...@chromium.org> wrote:
> +config ARCH_HAS_REFCOUNT
> + bool
> + help
> + An architecture selects this when it has implemented refcount_t
> + using primitizes that provide a faster runtime at the expense
> + of some full refcount state checks. The refcount overflow condition,
> + however, must be retained. Catching overflows is the primary
> + security concern for protecting against bugs in reference counts.
s/primitizes/primitives
also, the 'faster runtime' and the whole explanation reads a bit weird to me,
how about something like:
An architecture selects this when it has implemented refcount_t
using open coded assembly primitives that provide an optimized
refcount_t implementation, possibly at the expense of some full
refcount state checks of CONFIG_REFCOUNT_FULL=y.
The refcount overflow check behavior, however, must be retained.
Catching overflows is the primary security concern for protecting
against bugs in reference counts.
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -55,6 +55,7 @@ config X86
> select ARCH_HAS_KCOV if X86_64
> select ARCH_HAS_MMIO_FLUSH
> select ARCH_HAS_PMEM_API if X86_64
> + select ARCH_HAS_REFCOUNT
> select ARCH_HAS_UACCESS_FLUSHCACHE if X86_64
> select ARCH_HAS_SET_MEMORY
> select ARCH_HAS_SG_CHAIN
Just wonderin, how was the 32-bit kernel tested?
> +/*
> + * Body of refcount error handling: in .text.unlikely, saved into CX the
> + * address of the refcount that has entered a bad state, and trigger an
> + * exception. Fixup address is back in regular execution flow in .text.
I had to read this 4 times to parse it (and even now I'm unsure whether I
parsed
it correctly) - could this explanation be transformed to simpler, more
straightforward English?
> + */
> +#define _REFCOUNT_EXCEPTION \
> + ".pushsection .text.unlikely\n" \
> + "111:\tlea %[counter], %%" _ASM_CX "\n" \
> + "112:\t" ASM_UD0 "\n" \
> + ASM_UNREACHABLE \
> + ".popsection\n" \
> + "113:\n" \
> + _ASM_EXTABLE_REFCOUNT(112b, 113b)
Would it be technically possible to use named labels instead of these random
numbered labels?
> + /*
> + * This function has been called because either a negative refcount
> + * value was seen by any of the refcount functions, or a zero
> + * refcount value was seen by refcount_dec().
> + *
> + * If we crossed from INT_MAX to INT_MIN, the OF flag (result
> + * wrapped around) will be set. Additionally, seeing the refcount
> + * reach 0 will set the ZF flag. In each of these cases we want a
> + * report, since it's a boundary condition.
Small nit: 'ZF' stands for 'zero flag' - so we should either write 'zero flag'
or
'ZF' - 'ZF flag' is kind of redundant.
> +#else
> +static inline void refcount_error_report(struct pt_regs *regs,
> + const char *msg) { }
By now you should know that for x86 code you should not break lines in such an
ugly fashion, right? :-)
Thanks,
Ingo
