Re: [PATCH v14 5/8] arm64: mte: Enable TCO in functions that can read beyond buffer limits
Hi Vincenzo, I love your patch! Yet something to improve: [auto build test ERROR on kvmarm/next] [also build test ERROR on linus/master v5.12-rc2 next-20210309] [cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next hnaz-linux-mm/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210309-001716 base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next config: arm64-randconfig-s032-20210309 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.3-262-g5e674421-dirty # https://github.com/0day-ci/linux/commit/660df126323fe5533a1be7834e1754a1adc69f13 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210309-001716 git checkout 660df126323fe5533a1be7834e1754a1adc69f13 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): aarch64-linux-ld: mm/maccess.o: in function `copy_from_kernel_nofault': >> maccess.c:(.text+0x340): undefined reference to `mte_async_mode' maccess.c:(.text+0x340): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' >> aarch64-linux-ld: maccess.c:(.text+0x344): undefined reference to >> `mte_async_mode' aarch64-linux-ld: maccess.c:(.text+0x44c): undefined reference to `mte_async_mode' maccess.c:(.text+0x44c): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' aarch64-linux-ld: maccess.c:(.text+0x450): undefined reference to `mte_async_mode' aarch64-linux-ld: maccess.c:(.text+0x474): undefined reference to `mte_async_mode' aarch64-linux-ld: mm/maccess.o:maccess.c:(.text+0x4d0): more undefined references to `mte_async_mode' follow mm/maccess.o: in function `copy_from_kernel_nofault': maccess.c:(.text+0x4d0): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' maccess.c:(.text+0x550): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' mm/maccess.o: in function `copy_to_kernel_nofault': maccess.c:(.text+0x6cc): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' maccess.c:(.text+0x7d8): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' maccess.c:(.text+0x864): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' maccess.c:(.text+0x8ec): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' mm/maccess.o: in function `strncpy_from_kernel_nofault': maccess.c:(.text+0xaac): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' fs/namei.o: in function `full_name_hash': namei.c:(.text+0x28): relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against undefined symbol `mte_async_mode' fs/namei.o: in function `hashlen_string': namei.c:(.text+0x2a28): additional relocation overflows omitted from the output --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v14 5/8] arm64: mte: Enable TCO in functions that can read beyond buffer limits
On 3/8/21 6:09 PM, Mark Rutland wrote:
>> +DECLARE_STATIC_KEY_FALSE(mte_async_mode);
> Can we please hide this behind something like:
>
> static inline bool system_uses_mte_async_mode(void)
> {
> return IS_ENABLED(CONFIG_KASAN_HW_TAGS) &&
> static_branch_unlikely(_async_mode);
> }
>
> ... like we do for system_uses_ttbr0_pan()?
>
I agree, it is a cleaner solution. I will add it to v15.
> That way the callers are easier to read, and kernels built without
> CONFIG_KASAN_HW_TAGS don't have the static branch at all. I reckon you
> can put that in one of hte mte headers and include it where needed.
--
Regards,
Vincenzo
Re: [PATCH v14 5/8] arm64: mte: Enable TCO in functions that can read beyond buffer limits
Hi Vincenzo, I love your patch! Yet something to improve: [auto build test ERROR on kvmarm/next] [also build test ERROR on linus/master v5.12-rc2 next-20210305] [cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next hnaz-linux-mm/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210309-001716 base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next config: arm64-randconfig-r021-20210308 (attached as .config) compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 3a11a41795bec548e91621caaa4cc00fc31b2212) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm64 cross compiling tool for clang build # apt-get install binutils-aarch64-linux-gnu # https://github.com/0day-ci/linux/commit/660df126323fe5533a1be7834e1754a1adc69f13 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210309-001716 git checkout 660df126323fe5533a1be7834e1754a1adc69f13 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> ld.lld: error: undefined symbol: mte_async_mode >>> referenced by maccess.c >>> maccess.o:(copy_from_kernel_nofault) in archive mm/built-in.a >>> referenced by maccess.c >>> maccess.o:(copy_from_kernel_nofault) in archive mm/built-in.a >>> referenced by maccess.c >>> maccess.o:(copy_from_kernel_nofault) in archive mm/built-in.a >>> referenced 62 more times --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v14 5/8] arm64: mte: Enable TCO in functions that can read beyond buffer limits
Hi Vincenzo, I love your patch! Yet something to improve: [auto build test ERROR on kvmarm/next] [also build test ERROR on linus/master v5.12-rc2] [cannot apply to arm64/for-next/core xlnx/master arm/for-next soc/for-next hnaz-linux-mm/master] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210309-001716 base: https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git next config: arm64-randconfig-r006-20210308 (attached as .config) compiler: aarch64-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/660df126323fe5533a1be7834e1754a1adc69f13 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Vincenzo-Frascino/arm64-ARMv8-5-A-MTE-Add-async-mode-support/20210309-001716 git checkout 660df126323fe5533a1be7834e1754a1adc69f13 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arm64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All errors (new ones prefixed by >>): >> aarch64-linux-ld: mm/maccess.o:(__jump_table+0x8): undefined reference to >> `mte_async_mode' aarch64-linux-ld: mm/maccess.o:(__jump_table+0x18): undefined reference to `mte_async_mode' aarch64-linux-ld: mm/maccess.o:(__jump_table+0x28): undefined reference to `mte_async_mode' aarch64-linux-ld: mm/maccess.o:(__jump_table+0x38): undefined reference to `mte_async_mode' aarch64-linux-ld: mm/maccess.o:(__jump_table+0x48): undefined reference to `mte_async_mode' aarch64-linux-ld: mm/maccess.o:(__jump_table+0x58): more undefined references to `mte_async_mode' follow --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH v14 5/8] arm64: mte: Enable TCO in functions that can read beyond buffer limits
On Mon, Mar 08, 2021 at 04:14:31PM +, Vincenzo Frascino wrote:
> load_unaligned_zeropad() and __get/put_kernel_nofault() functions can
> read passed some buffer limits which may include some MTE granule with a
> different tag.
s/passed/past/
> When MTE async mode is enable, the load operation crosses the boundaries
s/enabel/enabled/
> and the next granule has a different tag the PE sets the TFSR_EL1.TF1 bit
> as if an asynchronous tag fault is happened.
>
> Enable Tag Check Override (TCO) in these functions before the load and
> disable it afterwards to prevent this to happen.
>
> Note: The same condition can be hit in MTE sync mode but we deal with it
> through the exception handling.
> In the current implementation, mte_async_mode flag is set only at boot
> time but in future kasan might acquire some runtime features that
> that change the mode dynamically, hence we disable it when sync mode is
> selected for future proof.
>
> Cc: Catalin Marinas
> Cc: Will Deacon
> Reported-by: Branislav Rankov
> Tested-by: Branislav Rankov
> Signed-off-by: Vincenzo Frascino
> ---
> arch/arm64/include/asm/uaccess.h| 24
> arch/arm64/include/asm/word-at-a-time.h | 4
> arch/arm64/kernel/mte.c | 22 ++
> 3 files changed, 50 insertions(+)
>
> diff --git a/arch/arm64/include/asm/uaccess.h
> b/arch/arm64/include/asm/uaccess.h
> index 0deb88467111..a857f8f82aeb 100644
> --- a/arch/arm64/include/asm/uaccess.h
> +++ b/arch/arm64/include/asm/uaccess.h
> @@ -188,6 +188,26 @@ static inline void __uaccess_enable_tco(void)
>ARM64_MTE, CONFIG_KASAN_HW_TAGS));
> }
>
> +/* Whether the MTE asynchronous mode is enabled. */
> +DECLARE_STATIC_KEY_FALSE(mte_async_mode);
Can we please hide this behind something like:
static inline bool system_uses_mte_async_mode(void)
{
return IS_ENABLED(CONFIG_KASAN_HW_TAGS) &&
static_branch_unlikely(_async_mode);
}
... like we do for system_uses_ttbr0_pan()?
That way the callers are easier to read, and kernels built without
CONFIG_KASAN_HW_TAGS don't have the static branch at all. I reckon you
can put that in one of hte mte headers and include it where needed.
Thanks,
Mark.
> +
> +/*
> + * These functions disable tag checking only if in MTE async mode
> + * since the sync mode generates exceptions synchronously and the
> + * nofault or load_unaligned_zeropad can handle them.
> + */
> +static inline void __uaccess_disable_tco_async(void)
> +{
> + if (static_branch_unlikely(_async_mode))
> + __uaccess_disable_tco();
> +}
> +
> +static inline void __uaccess_enable_tco_async(void)
> +{
> + if (static_branch_unlikely(_async_mode))
> + __uaccess_enable_tco();
> +}
> +
> static inline void uaccess_disable_privileged(void)
> {
> __uaccess_disable_tco();
> @@ -307,8 +327,10 @@ do {
> \
> do { \
> int __gkn_err = 0; \
> \
> + __uaccess_enable_tco_async(); \
> __raw_get_mem("ldr", *((type *)(dst)), \
> (__force type *)(src), __gkn_err);\
> + __uaccess_disable_tco_async(); \
> if (unlikely(__gkn_err))\
> goto err_label; \
> } while (0)
> @@ -380,8 +402,10 @@ do {
> \
> do { \
> int __pkn_err = 0; \
> \
> + __uaccess_enable_tco_async(); \
> __raw_put_mem("str", *((type *)(src)), \
> (__force type *)(dst), __pkn_err);\
> + __uaccess_disable_tco_async(); \
> if (unlikely(__pkn_err))\
> goto err_label; \
> } while(0)
> diff --git a/arch/arm64/include/asm/word-at-a-time.h
> b/arch/arm64/include/asm/word-at-a-time.h
> index 950b5909..c62d9fa791aa 100644
> --- a/arch/arm64/include/asm/word-at-a-time.h
> +++ b/arch/arm64/include/asm/word-at-a-time.h
> @@ -55,6 +55,8 @@ static inline unsigned long load_unaligned_zeropad(const
> void *addr)
> {
> unsigned long ret, offset;
>
> + __uaccess_enable_tco_async();
> +
> /* Load word from unaligned pointer addr */
> asm(
> "1: ldr %0, %3\n"
> @@ -76,6 +78,8 @@
