kasan: fixes for 5.11-rc
Subsystems (3)
cover kasan: fixes for 5.11-rc
Andrey Konovalov (2):
kasan, mm: fix conflicts with init_on_alloc/free kasan, arm64: fix pointer tags in KASAN reports
arch/arm64/mm/fault.c | 2 ++ mm/slub.c | 7 ++++--- 2 files changed, 6 insertions(+), 3 deletions(-)
Signature · 1 line
2.30.0.284.gd98b1dd5eaa7-goog
PATCH 1/2 kasan, mm: fix conflicts with init_on_alloc/free
A few places where SLUB accesses object's data or metadata were missed in a previous patch. This leads to false positives with hardware tag-based KASAN when bulk allocations are used with init_on_alloc/free.
Fix the false-positives by resetting pointer tags during these accesses.
--- mm/slub.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
mm/slub.c +4 -3
@@ -2791,7 +2791,8 @@ static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s, void *obj) {if (unlikely (slab_want_init_on_free (s)) && obj) -memset ((void *)((char *)obj + s->offset),0 ,sizeof (void *)); +memset ((void *)((char *)kasan_reset_tag (obj) + s->offset), +0 ,sizeof (void *)); }/*
@@ -2883,7 +2884,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,stat (s,ALLOC_FASTPATH ); } -maybe_wipe_obj_freeptr (s,kasan_reset_tag (object)); +maybe_wipe_obj_freeptr (s, object);if (unlikely (slab_want_init_on_alloc (gfpflags, s)) && object)memset (kasan_reset_tag (object),0 , s->object_size);
@@ -3329,7 +3330,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,int j;for (j =0 ; j < i; j++) -memset (p[j],0 , s->object_size); +memset (kasan_reset_tag (p[j]),0 , s->object_size); }/* memcg and kmem_cache debug support */
Signature · 1 line
2.30.0.284.gd98b1dd5eaa7-goog
Re: [PATCH 1/2] kasan, mm: fix conflicts with init_on_alloc/free
Quoted 10 lines · Andrey Konovalov
A few places where SLUB accesses object's data or metadata were missed in a previous patch. This leads to false positives with hardware tag-based KASAN when bulk allocations are used with init_on_alloc/free.
Fix the false-positives by resetting pointer tags during these accesses.
Fixes aa1ef4d7b3f67 ("kasan, mm: reset tags when accessing metadata")Reported-by Dmitry Vyukov <[email protected]>Signed-off-by Andrey Konovalov <[email protected]>
Quoted 19 lines
--- mm/slub.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)mm/slub.c +2
-1@@ -2791,7 +2791,8 @@ static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s, void *obj) {if (unlikely (slab_want_init_on_free (s)) && obj) -memset ((void *)((char *)obj + s->offset),0 ,sizeof (void *)); +memset ((void *)((char *)kasan_reset_tag (obj) + s->offset), +0 ,sizeof (void *)); }/*
mm/slub.c static __always_inline void *slab_alloc_node(struct kmem_cache *s, · line 2887
Quoted 5 lines
mm/slub.c +1
-1@@ -2883,7 +2884,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,stat (s,ALLOC_FASTPATH ); } -maybe_wipe_obj_freeptr (s,kasan_reset_tag (object)); +maybe_wipe_obj_freeptr (s, object);
And in that case the reset was unnecessary, right. (commit log only mentions adding missing resets).
Quoted 12 lines
mm/slub.c +1
-1if (unlikely (slab_want_init_on_alloc (gfpflags, s)) && object)memset (kasan_reset_tag (object),0 , s->object_size);@@ -3329,7 +3330,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *s, gfp_t flags, size_t size,int j;for (j =0 ; j < i; j++) -memset (p[j],0 , s->object_size); +memset (kasan_reset_tag (p[j]),0 , s->object_size); }/* memcg and kmem_cache debug support */
Re: [PATCH 1/2] kasan, mm: fix conflicts with init_on_alloc/free
Quoted 37 lines · Vlastimil Babka
Quoted 10 lines · Andrey Konovalov
A few places where SLUB accesses object's data or metadata were missed in a previous patch. This leads to false positives with hardware tag-based KASAN when bulk allocations are used with init_on_alloc/free.
Fix the false-positives by resetting pointer tags during these accesses.
Fixes aa1ef4d7b3f67 ("kasan, mm: reset tags when accessing metadata")Reported-by Dmitry Vyukov <[email protected]>Signed-off-by Andrey Konovalov <[email protected]>Acked-by Vlastimil Babka <[email protected]>Quoted 22 lines
--- mm/slub.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)mm/slub.c +2
-1@@ -2791,7 +2791,8 @@ static __always_inline void maybe_wipe_obj_freeptr(struct kmem_cache *s, void *obj) {if (unlikely (slab_want_init_on_free (s)) && obj) -memset ((void *)((char *)obj + s->offset),0 ,sizeof (void *)); +memset ((void *)((char *)kasan_reset_tag (obj) + s->offset), +0 ,sizeof (void *)); }/* @@ -2883,7 +2884,7 @@ static __always_inline void *slab_alloc_node(struct kmem_cache *s,stat (s,ALLOC_FASTPATH ); }
Quoted 5 lines
mm/slub.c static __always_inline void *slab_alloc_node(struct kmem_cache *s, · line 2887
Quoted 2 lines
mm/slub.c +1
-1-maybe_wipe_obj_freeptr (s,kasan_reset_tag (object)); +maybe_wipe_obj_freeptr (s, object);And in that case the reset was unnecessary, right. (commit log only mentions adding missing resets).
The reset has been moved into maybe_wipe_obj_freeptr(). I'll mention it in the changelog in v2.
Thanks!
PATCH 2/2 kasan, arm64: fix pointer tags in KASAN reports
In reply to Andrey KonovalovAs of the "arm64: expose FAR_EL1 tag bits in siginfo" patch, the address that is passed to report_tag_fault has pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).
Fix up the pointer tag before calling kasan_report.
--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)
arch/arm64/mm/fault.c +2 -0
@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );/* * SAS bits aren't set for all faults reported in EL1, so we can't * find out access size.
Signature · 1 line
2.30.0.284.gd98b1dd5eaa7-goog
Catalin Marinas
maintainerRe: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0
arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );
Ah, I see, that top 4 bits are zeroed by do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.
Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +4 -3
@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag -* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */ - far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }
Signature · 1 line
Catalin
Re: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 42 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +0
-1@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 10 lines
arch/arm64/mm/fault.c +4
-2-* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */ - far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }
Sounds good, will do in v3, thanks!
Catalin Marinas
maintainerRe: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 47 lines · Andrey Konovalov
Quoted 46 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +3
-2@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag -* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address.
Quoted 8 lines
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 6 lines
arch/arm64/mm/fault.c +1
-1*/ - far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }Sounds good, will do in v3, thanks!
I wonder if this one gives the same result (so please check):
far = u64_replace_bits(untagged_addr(far), far, MTE_TAG_MASK);
(defined in linux/bitfield.h)
Signature · 1 line
Catalin
Re: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 58 lines · Catalin Marinas
Quoted 47 lines · Andrey Konovalov
Quoted 46 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +3
-2@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag -* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. Quoted 8 lines
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 6 lines
arch/arm64/mm/fault.c +1
-1*/ - far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }Sounds good, will do in v3, thanks!
Quoted 5 lines
I wonder if this one gives the same result (so please check):
far = u64_replace_bits(untagged_addr(far), far, MTE_TAG_MASK);(defined in
linux/bitfield.h)
No, it zeroes out the tag. Not sure why. I took a brief look at the implementation and didn't get how it's supposed to work - too much bit trickery.
Re: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
In reply to Catalin MarinasQuoted 42 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +0
-1@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 10 lines
arch/arm64/mm/fault.c +4
-2-* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */ - far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }
BTW, we can do "untagged_addr(far) | (far & MTE_TAG_MASK)" here, as untagged_addr() doesn't change kernel pointers.
Catalin Marinas
maintainerRe: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 47 lines · Andrey Konovalov
Quoted 46 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +3
-2@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag -* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */
Quoted 8 lines
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 5 lines
arch/arm64/mm/fault.c +1
-1- far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }BTW, we can do "
untagged_addr(far)| (far &MTE_TAG_MASK)" here, asuntagged_addr()doesn't change kernel pointers.
untagged_addr() does change tagged kernel pointers, it sign-extends from bit 55. So the top byte becomes 0xff and you can no longer or the tag bits in.
Signature · 1 line
Catalin
Re: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 58 lines · Catalin Marinas
Quoted 47 lines · Andrey Konovalov
Quoted 46 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +3
-2@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag -* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */ Quoted 8 lines
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 5 lines
arch/arm64/mm/fault.c +1
-1- far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }BTW, we can do "
untagged_addr(far)| (far &MTE_TAG_MASK)" here, asuntagged_addr()doesn't change kernel pointers.
Quoted 3 lines
untagged_addr()does change tagged kernel pointers, it sign-extends from bit 55. So the top byte becomes0xffand you can no longer or the tag bits in.
That's __untagged_addr(), untagged_addr() keeps the bits for kernel pointers as of 597399d0cb91.
Catalin Marinas
maintainerRe: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 59 lines · Andrey Konovalov
Quoted 58 lines · Catalin Marinas
Quoted 55 lines · Andrey Konovalov
Quoted 41 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +0
-1@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 10 lines
arch/arm64/mm/fault.c +4
-2-* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */ - far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }BTW, we can do "
untagged_addr(far)| (far &MTE_TAG_MASK)" here, asuntagged_addr()doesn't change kernel pointers.
Quoted 6 lines
Quoted 3 lines
untagged_addr()does change tagged kernel pointers, it sign-extends from bit 55. So the top byte becomes0xffand you can no longer or the tag bits in.That's
__untagged_addr(),untagged_addr()keeps the bits for kernel pointers as of597399d0cb91.
Ah, you are right. In this case I think we should use __untagged_addr() above. Even if the tag check fault happened on a kernel address, bits 63:60 are still unknown.
Signature · 1 line
Catalin
Re: [PATCH 2/2] kasan, arm64: fix pointer tags in KASAN reports
Quoted 65 lines · Catalin Marinas
Quoted 63 lines · Andrey Konovalov
Quoted 61 lines · Catalin Marinas
Quoted 47 lines · Andrey Konovalov
Quoted 46 lines · Catalin Marinas
Quoted 19 lines · Andrey Konovalov
As of the "arm64: expose
FAR_EL1tag bits in siginfo" patch, the address that is passed toreport_tag_faulthas pointer tags in the format of 0x0X, while KASAN uses 0xFX format (note the difference in the top 4 bits).Fix up the pointer tag before calling
kasan_report.Fixes dceec3ff7807 ("arm64: expose FAR_EL1 tag bits in siginfo")Fixes 4291e9ee6189 ("kasan, arm64: print report from tag fault handler")Signed-off-by Andrey Konovalov <[email protected]>--- arch/arm64/mm/fault.c | 2 ++ 1 file changed, 2 insertions(+)arch/arm64/mm/fault.c +0
-0arch/arm64/mm/fault.c static void report_tag_fault(unsigned long addr, unsigned int esr, · line 308
Quoted 5 lines
arch/arm64/mm/fault.c +2
-0@@ -304,6 +304,8 @@ static void report_tag_fault(unsigned long addr, unsigned int esr, {bool is_write = ((esr & ESR_ELx_WNR) >> ESR_ELx_WNR_SHIFT) !=0 ; +/* The format of KASAN tags is 0xF<x>. */ + addr |= (0xF0UL <<MTE_TAG_SHIFT );Ah, I see, that top 4 bits are zeroed by
do_tag_check_fault(). When this was added, the only tag faults were generated for user addresses.Anyway, I'd rather fix it in there based on bit 55, something like (only compile-tested):
arch/arm64/mm/fault.c +3
-2@@ -709,10 +709,11 @@ static int do_tag_check_fault(unsigned long far, unsigned int esr, struct pt_regs *regs) {/* -* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN for tag -* check faults. Mask them out now so that userspace doesn't see them. +* The architecture specifies that bits 63:60 of FAR_EL1 are UNKNOWN +* for tag check faults. Set them to the corresponding bits in the +* untagged address. */ Quoted 8 lines
arch/arm64/mm/fault.c static int do_tag_check_fault(unsigned long far, unsigned int esr, · line 719
Quoted 5 lines
arch/arm64/mm/fault.c +1
-1- far &= (1UL <<60 ) -1 ; + far = (untagged_addr (far) & ~MTE_TAG_MASK ) | (far &MTE_TAG_MASK ) ;do_bad_area (far, esr, regs);return 0 ; }BTW, we can do "
untagged_addr(far)| (far &MTE_TAG_MASK)" here, asuntagged_addr()doesn't change kernel pointers.
untagged_addr()does change tagged kernel pointers, it sign-extends from bit 55. So the top byte becomes0xffand you can no longer or the tag bits in.
Quoted 6 lines
Quoted 2 lines
That's
__untagged_addr(),untagged_addr()keeps the bits for kernel pointers as of597399d0cb91.Ah, you are right. In this case I think we should use
__untagged_addr()above. Even if the tag check fault happened on a kernel address, bits 63:60 are still unknown.
Yeah, I keep forgetting about [__]untagged_addr() too. Maybe we need better names? Like untagged_addr() and untagged_addr_ttbr0()?
Anyway, I'll do the explicit calculation with __untagged_addr() in the next version.
Thanks!