arm64: alternative:flush cache with unpatched code
PATCH 1/1 arm64: alternative:flush cache with unpatched code
In the current implementation, __apply_alternatives patches flush_icache_range and then executes it without invalidating the icache. Thus, icache can contain some of the old instructions for flush_icache_range. This can cause unpredictable behavior as during execution we can get a mix of old and new instructions for flush_icache_range.
This patch :
1. Adds a new function flush_cache_kernel_range for flushing kernel memory range. This function is not patched during boot and can be safely used to flush cache during code patching.
2. Modifies __apply_alternatives so that it uses flush_cache_kernel_range to flush the cache range after patching code.
--- arch/arm64/include/asm/cacheflush.h | 1 + arch/arm64/kernel/alternative.c | 2 +- arch/arm64/mm/cache.S | 42 +++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-)
arch/arm64/include/asm/cacheflush.h +1 -0
@@ -73,6 +73,7 @@*/ extern void flush_icache_range (unsigned long start,unsigned long end);extern int invalidate_icache_range (unsigned long start,unsigned long end); +extern void flush_cache_kernel_range (unsigned long start,unsigned long end);extern void __flush_dcache_area (void *addr,size_t len);extern void __inval_dcache_area (void *addr,size_t len);extern void __clean_dcache_area_poc (void *addr,size_t len);
arch/arm64/kernel/alternative.c +1 -1
@@ -155,7 +155,7 @@ static void __apply_alternatives(void *alt_region, bool use_linear_alias)alt_cb (alt, origptr, updptr, nr_inst); -flush_icache_range ((uintptr_t )origptr, +flush_cache_kernel_range ((uintptr_t )origptr, (uintptr_t )(origptr + nr_inst)); } }
arch/arm64/mm/cache.S +42 -0
48 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 1/1 arm64: alternative:flush cache with unpatched code
Hi Rohit,
Quoted 16 lines · Rohit Khanna
In the current implementation,
__apply_alternativespatchesflush_icache_rangeand then executes it without invalidating the icache. Thus, icache can contain some of the old instructions forflush_icache_range. This can cause unpredictable behavior as during execution we can get a mix of old and new instructions forflush_icache_range.This patch :
1. Adds a new function
flush_cache_kernel_rangefor flushing kernel memory range. This function is not patched during boot and can be safely used to flush cache during code patching.2. Modifies
__apply_alternativesso that it usesflush_cache_kernel_rangeto flush the cache range after patching code.
Quoted 6 lines
Signed-off-by Rohit Khanna <[email protected]>--- arch/arm64/include/asm/cacheflush.h | 1 + arch/arm64/kernel/alternative.c | 2 +- arch/arm64/mm/cache.S | 42 +++++++++++++++++++++++++++++++++++++ 3 files changed, 44 insertions(+), 1 deletion(-)
Thanks for the patch; this is a horrible bug :)
Comments below.
Quoted 26 lines
arch/arm64/mm/cache.S +18
-0@@ -81,6 +81,48 @@ ENDPROC(flush_icache_range) ENDPROC(__flush_cache_user_range) /* + * flush_cache_kernel_range(start,end) + * + * Ensure that the I and D caches are coherent within specified kernel + * region. + * This is typically used when code has been written to a kernel memory + * region and will be executed. + * + * NOTE - This macro cannot have "alternatives" applied to it as its + * used to update alternatives + * + * - start - virtual start address of region + * - end - virtual end address of region + */ +ENTRY(flush_cache_kernel_range) + raw_dcache_line_size x2, x3 + sub x3, x2, #1 + bic x4, x0, x3 +1:
arch/arm64/mm/cache.S ENDPROC(flush_icache_range) · line 111
Quoted 10 lines
arch/arm64/mm/cache.S +10
-0+ dc civac, x4 /* Use civac instead of cvau. This is required + * due to ARM errata 826319, 827319, 824069, + * 819472 on A53 + */ + add x4, x4, x2 + cmp x4, x1 + b.lo 1b + dsb ish + + raw_icache_line_size x2, x3
Won't this be broken on systems that misreport the line size in CTR?
arch/arm64/mm/cache.S ENDPROC(flush_icache_range) · line 123
Quoted 12 lines
arch/arm64/mm/cache.S +12
-0+ sub x3, x2, #1 + bic x4, x0, x3 +1: + ic ivau, x4 // invalidate I line PoU + add x4, x4, x2 + cmp x4, x1 + b.lo 1b + dsb ish + isb + mov x0, #0 + ret +ENDPROC(flush_cache_kernel_range)
Generally, I don't think we want to expose this function as it's not going to be the right thing to call outside of the alternatives patching. Instead, can we:
1. Code this up in alternative.c, mostly in C code and always using the sanitised feature register value for the CTR
2. Just call __flush_icache_all once after patching is complete (and add a comment to __flush_icache_all saying that it's used by the patching code)
That way, we're keeping the implementation private and simple so we can hopefully avoid running into this problem again.
Cheers,
Will