| From 60af6994b232c37cfe7b18e627a42184496ef6fd Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 23 Feb 2022 01:30:23 +0000 |
| Subject: MIPS: fix fortify panic when copying asm exception handlers |
| |
| From: Alexander Lobakin <alobakin@pm.me> |
| |
| [ Upstream commit d17b66417308996e7e64b270a3c7f3c1fbd4cfc8 ] |
| |
| With KCFLAGS="-O3", I was able to trigger a fortify-source |
| memcpy() overflow panic on set_vi_srs_handler(). |
| Although O3 level is not supported in the mainline, under some |
| conditions that may've happened with any optimization settings, |
| it's just a matter of inlining luck. The panic itself is correct, |
| more precisely, 50/50 false-positive and not at the same time. |
| From the one side, no real overflow happens. Exception handler |
| defined in asm just gets copied to some reserved places in the |
| memory. |
| But the reason behind is that C code refers to that exception |
| handler declares it as `char`, i.e. something of 1 byte length. |
| It's obvious that the asm function itself is way more than 1 byte, |
| so fortify logics thought we are going to past the symbol declared. |
| The standard way to refer to asm symbols from C code which is not |
| supposed to be called from C is to declare them as |
| `extern const u8[]`. This is fully correct from any point of view, |
| as any code itself is just a bunch of bytes (including 0 as it is |
| for syms like _stext/_etext/etc.), and the exact size is not known |
| at the moment of compilation. |
| Adjust the type of the except_vec_vi_*() and related variables. |
| Make set_handler() take `const` as a second argument to avoid |
| cast-away warnings and give a little more room for optimization. |
| |
| Signed-off-by: Alexander Lobakin <alobakin@pm.me> |
| Signed-off-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| arch/mips/include/asm/setup.h | 2 +- |
| arch/mips/kernel/traps.c | 22 +++++++++++----------- |
| 2 files changed, 12 insertions(+), 12 deletions(-) |
| |
| diff --git a/arch/mips/include/asm/setup.h b/arch/mips/include/asm/setup.h |
| index bb36a400203d..8c56b862fd9c 100644 |
| --- a/arch/mips/include/asm/setup.h |
| +++ b/arch/mips/include/asm/setup.h |
| @@ -16,7 +16,7 @@ static inline void setup_8250_early_printk_port(unsigned long base, |
| unsigned int reg_shift, unsigned int timeout) {} |
| #endif |
| |
| -extern void set_handler(unsigned long offset, void *addr, unsigned long len); |
| +void set_handler(unsigned long offset, const void *addr, unsigned long len); |
| extern void set_uncached_handler(unsigned long offset, void *addr, unsigned long len); |
| |
| typedef void (*vi_handler_t)(void); |
| diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c |
| index d26b0fb8ea06..b9b31b13062d 100644 |
| --- a/arch/mips/kernel/traps.c |
| +++ b/arch/mips/kernel/traps.c |
| @@ -2091,19 +2091,19 @@ static void *set_vi_srs_handler(int n, vi_handler_t addr, int srs) |
| * If no shadow set is selected then use the default handler |
| * that does normal register saving and standard interrupt exit |
| */ |
| - extern char except_vec_vi, except_vec_vi_lui; |
| - extern char except_vec_vi_ori, except_vec_vi_end; |
| - extern char rollback_except_vec_vi; |
| - char *vec_start = using_rollback_handler() ? |
| - &rollback_except_vec_vi : &except_vec_vi; |
| + extern const u8 except_vec_vi[], except_vec_vi_lui[]; |
| + extern const u8 except_vec_vi_ori[], except_vec_vi_end[]; |
| + extern const u8 rollback_except_vec_vi[]; |
| + const u8 *vec_start = using_rollback_handler() ? |
| + rollback_except_vec_vi : except_vec_vi; |
| #if defined(CONFIG_CPU_MICROMIPS) || defined(CONFIG_CPU_BIG_ENDIAN) |
| - const int lui_offset = &except_vec_vi_lui - vec_start + 2; |
| - const int ori_offset = &except_vec_vi_ori - vec_start + 2; |
| + const int lui_offset = except_vec_vi_lui - vec_start + 2; |
| + const int ori_offset = except_vec_vi_ori - vec_start + 2; |
| #else |
| - const int lui_offset = &except_vec_vi_lui - vec_start; |
| - const int ori_offset = &except_vec_vi_ori - vec_start; |
| + const int lui_offset = except_vec_vi_lui - vec_start; |
| + const int ori_offset = except_vec_vi_ori - vec_start; |
| #endif |
| - const int handler_len = &except_vec_vi_end - vec_start; |
| + const int handler_len = except_vec_vi_end - vec_start; |
| |
| if (handler_len > VECTORSPACING) { |
| /* |
| @@ -2311,7 +2311,7 @@ void per_cpu_trap_init(bool is_boot_cpu) |
| } |
| |
| /* Install CPU exception handler */ |
| -void set_handler(unsigned long offset, void *addr, unsigned long size) |
| +void set_handler(unsigned long offset, const void *addr, unsigned long size) |
| { |
| #ifdef CONFIG_CPU_MICROMIPS |
| memcpy((void *)(ebase + offset), ((unsigned char *)addr - 1), size); |
| -- |
| 2.35.1 |
| |