| Message ID | 20210511144252.3779113-3-tabba@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Tidy up cache.S | expand |
On Tue, May 11, 2021 at 03:42:41PM +0100, Fuad Tabba wrote: > invalidate_icache_range() works on the kernel linear map, and > doesn't need uaccess. Remove the code that toggles > uaccess_ttbr0_enable, as well as the code that emits an entry > into the exception table (via the macro > invalidate_icache_by_line). Probably also worth mentioning the return type change, but regardless: Acked-by: Mark Rutland <mark.rutland@arm.com> I do worry this means we've been silently ignoring cases where this faults, and so there's the risk that this has been masking bugs elsewhere. It'd be good to throw Syzkaller and the like at this ASAP Thanks, Mark. > No functional change intended. > > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > Reported-by: Will Deacon <will@kernel.org> > Link: https://lore.kernel.org/linux-arch/20200511110014.lb9PEahJ4hVOYrbwIb_qUHXyNy9KQzNFdb_I3YlzY6A@z/ > Signed-off-by: Fuad Tabba <tabba@google.com> > --- > arch/arm64/include/asm/cacheflush.h | 2 +- > arch/arm64/mm/cache.S | 11 +---------- > 2 files changed, 2 insertions(+), 11 deletions(-) > > diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h > index 52e5c1623224..a586afa84172 100644 > --- a/arch/arm64/include/asm/cacheflush.h > +++ b/arch/arm64/include/asm/cacheflush.h > @@ -57,7 +57,7 @@ > * - size - region size > */ > extern void __flush_icache_range(unsigned long start, unsigned long end); > -extern int invalidate_icache_range(unsigned long start, unsigned long end); > +extern void invalidate_icache_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); > diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S > index 092f73acdf9a..6babaaf34f17 100644 > --- a/arch/arm64/mm/cache.S > +++ b/arch/arm64/mm/cache.S > @@ -105,21 +105,12 @@ SYM_FUNC_END(__flush_cache_user_range) > */ > SYM_FUNC_START(invalidate_icache_range) > alternative_if ARM64_HAS_CACHE_DIC > - mov x0, xzr > isb > ret > alternative_else_nop_endif > > - uaccess_ttbr0_enable x2, x3, x4 > - > - invalidate_icache_by_line x0, x1, x2, x3, 1, 2f > - mov x0, xzr > -1: > - uaccess_ttbr0_disable x1, x2 > + invalidate_icache_by_line x0, x1, x2, x3, 0, 0f > ret > -2: > - mov x0, #-EFAULT > - b 1b > SYM_FUNC_END(invalidate_icache_range) > > /* > -- > 2.31.1.607.g51e8a6a459-goog >
Hi Mark, On Tue, May 11, 2021 at 4:34 PM Mark Rutland <mark.rutland@arm.com> wrote: > > On Tue, May 11, 2021 at 03:42:41PM +0100, Fuad Tabba wrote: > > invalidate_icache_range() works on the kernel linear map, and > > doesn't need uaccess. Remove the code that toggles > > uaccess_ttbr0_enable, as well as the code that emits an entry > > into the exception table (via the macro > > invalidate_icache_by_line). > > Probably also worth mentioning the return type change, but regardless: Will do in v2. > Acked-by: Mark Rutland <mark.rutland@arm.com> > > I do worry this means we've been silently ignoring cases where this > faults, and so there's the risk that this has been masking bugs > elsewhere. It'd be good to throw Syzkaller and the like at this ASAP Good point. I'll look into that. Thanks, /fuad > Thanks, > Mark. > > > No functional change intended. > > > > Reported-by: Catalin Marinas <catalin.marinas@arm.com> > > Reported-by: Will Deacon <will@kernel.org> > > Link: https://lore.kernel.org/linux-arch/20200511110014.lb9PEahJ4hVOYrbwIb_qUHXyNy9KQzNFdb_I3YlzY6A@z/ > > Signed-off-by: Fuad Tabba <tabba@google.com> > > --- > > arch/arm64/include/asm/cacheflush.h | 2 +- > > arch/arm64/mm/cache.S | 11 +---------- > > 2 files changed, 2 insertions(+), 11 deletions(-) > > > > diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h > > index 52e5c1623224..a586afa84172 100644 > > --- a/arch/arm64/include/asm/cacheflush.h > > +++ b/arch/arm64/include/asm/cacheflush.h > > @@ -57,7 +57,7 @@ > > * - size - region size > > */ > > extern void __flush_icache_range(unsigned long start, unsigned long end); > > -extern int invalidate_icache_range(unsigned long start, unsigned long end); > > +extern void invalidate_icache_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); > > diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S > > index 092f73acdf9a..6babaaf34f17 100644 > > --- a/arch/arm64/mm/cache.S > > +++ b/arch/arm64/mm/cache.S > > @@ -105,21 +105,12 @@ SYM_FUNC_END(__flush_cache_user_range) > > */ > > SYM_FUNC_START(invalidate_icache_range) > > alternative_if ARM64_HAS_CACHE_DIC > > - mov x0, xzr > > isb > > ret > > alternative_else_nop_endif > > > > - uaccess_ttbr0_enable x2, x3, x4 > > - > > - invalidate_icache_by_line x0, x1, x2, x3, 1, 2f > > - mov x0, xzr > > -1: > > - uaccess_ttbr0_disable x1, x2 > > + invalidate_icache_by_line x0, x1, x2, x3, 0, 0f > > ret > > -2: > > - mov x0, #-EFAULT > > - b 1b > > SYM_FUNC_END(invalidate_icache_range) > > > > /* > > -- > > 2.31.1.607.g51e8a6a459-goog > >
diff --git a/arch/arm64/include/asm/cacheflush.h b/arch/arm64/include/asm/cacheflush.h index 52e5c1623224..a586afa84172 100644 --- a/arch/arm64/include/asm/cacheflush.h +++ b/arch/arm64/include/asm/cacheflush.h @@ -57,7 +57,7 @@ * - size - region size */ extern void __flush_icache_range(unsigned long start, unsigned long end); -extern int invalidate_icache_range(unsigned long start, unsigned long end); +extern void invalidate_icache_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); diff --git a/arch/arm64/mm/cache.S b/arch/arm64/mm/cache.S index 092f73acdf9a..6babaaf34f17 100644 --- a/arch/arm64/mm/cache.S +++ b/arch/arm64/mm/cache.S @@ -105,21 +105,12 @@ SYM_FUNC_END(__flush_cache_user_range) */ SYM_FUNC_START(invalidate_icache_range) alternative_if ARM64_HAS_CACHE_DIC - mov x0, xzr isb ret alternative_else_nop_endif - uaccess_ttbr0_enable x2, x3, x4 - - invalidate_icache_by_line x0, x1, x2, x3, 1, 2f - mov x0, xzr -1: - uaccess_ttbr0_disable x1, x2 + invalidate_icache_by_line x0, x1, x2, x3, 0, 0f ret -2: - mov x0, #-EFAULT - b 1b SYM_FUNC_END(invalidate_icache_range) /*
invalidate_icache_range() works on the kernel linear map, and doesn't need uaccess. Remove the code that toggles uaccess_ttbr0_enable, as well as the code that emits an entry into the exception table (via the macro invalidate_icache_by_line). No functional change intended. Reported-by: Catalin Marinas <catalin.marinas@arm.com> Reported-by: Will Deacon <will@kernel.org> Link: https://lore.kernel.org/linux-arch/20200511110014.lb9PEahJ4hVOYrbwIb_qUHXyNy9KQzNFdb_I3YlzY6A@z/ Signed-off-by: Fuad Tabba <tabba@google.com> --- arch/arm64/include/asm/cacheflush.h | 2 +- arch/arm64/mm/cache.S | 11 +---------- 2 files changed, 2 insertions(+), 11 deletions(-)