Linux-Kernel Archive: [PATCH 5.4 400/475] MIPS: fix fortify panic when copying asm exception handlers [PATCH 5.4 400/475] MIPS: fix fortify panic when copying asm exception handlers From: Greg Kroah-Hartman Date: Thu Apr 14 2022 - 11:17:13 EST Next message: Greg Kroah-Hartman: "[PATCH 5.4 404/475] Bluetooth: Fix use after free in hci_send_acl" Previous message: Greg Kroah-Hartman: "[PATCH 5.4 401/475] scsi: libfc: Fix use after free in fc_exch_abts_resp()" In reply to: Greg Kroah-Hartman: "[PATCH 5.4 401/475] scsi: libfc: Fix use after free in fc_exch_abts_resp()" Next in thread: Greg Kroah-Hartman: "[PATCH 5.4 404/475] Bluetooth: Fix use after free in hci_send_acl" Messages sorted by: [ date ] [ thread ] [ subject ] [ author ] From: Alexander Lobakin
[ 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 Signed-off-by: Thomas Bogendoerfer Signed-off-by: Sasha Levin --- 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 8282d0feb0b2..749089c25d5e 100644 --- a/arch/mips/kernel/traps.c +++ b/arch/mips/kernel/traps.c @@ -2020,19 +2020,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) { /* @@ -2240,7 +2240,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 Next message: Greg Kroah-Hartman: "[PATCH 5.4 404/475] Bluetooth: Fix use after free in hci_send_acl" Previous message: Greg Kroah-Hartman: "[PATCH 5.4 401/475] scsi: libfc: Fix use after free in fc_exch_abts_resp()" In reply to: Greg Kroah-Hartman: "[PATCH 5.4 401/475] scsi: libfc: Fix use after free in fc_exch_abts_resp()" Next in thread: Greg Kroah-Hartman: "[PATCH 5.4 404/475] Bluetooth: Fix use after free in hci_send_acl" Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
