KASAN support for RISC-V
Subsystems (3)
cover KASAN support for RISC-V
KASAN is an important runtime memory debugging feature in linux kernel which can detect use-after-free and out-of- bounds problems.
There are two patches in this letter:
1. Porting the memmove string operation.
2. Porting the feature KASAN.
Nick Hu (2):
riscv: Add memmove string operation.
riscv: Add KASAN support
arch/riscv/Kconfig | 2 + arch/riscv/include/asm/kasan.h | 26 +++++++++ arch/riscv/include/asm/pgtable-64.h | 5 ++ arch/riscv/include/asm/string.h | 10 ++++ arch/riscv/kernel/head.S | 3 + arch/riscv/kernel/riscv_ksyms.c | 4 ++ arch/riscv/kernel/setup.c | 9 +++ arch/riscv/kernel/vmlinux.lds.S | 1 + arch/riscv/lib/Makefile | 1 + arch/riscv/lib/memcpy.S | 5 +- arch/riscv/lib/memmove.S | 64 ++++++++++++++++++++++ arch/riscv/lib/memset.S | 5 +- arch/riscv/mm/Makefile | 6 ++ arch/riscv/mm/kasan_init.c | 102 +++++++++++++++++++++++++++++++++++ 14 files changed, 239 insertions(+), 4 deletions(-) create mode 100644 arch/riscv/include/asm/kasan.h create mode 100644 arch/riscv/lib/memmove.S create mode 100644 arch/riscv/mm/kasan_init.c
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH 1/2 riscv: Add memmove string operation.
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
This porting refers to the 'arch/nds32/lib/memmove.S'.
--- arch/riscv/include/asm/string.h | 3 ++ arch/riscv/kernel/riscv_ksyms.c | 1 + arch/riscv/lib/Makefile | 1 + arch/riscv/lib/memmove.S | 63 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 68 insertions(+), 0 deletions(-) create mode 100644 arch/riscv/lib/memmove.S
arch/riscv/include/asm/string.h +3 -0
@@ -15,4 +15,7 @@#define __HAVE_ARCH_MEMCPY extern asmlinkage void *memcpy(void *, const void *, size_t); +#define __HAVE_ARCH_MEMMOVE +extern asmlinkage void *memmove(void *, const void *, size_t); +#endif /* _ASM_RISCV_STRING_H */
arch/riscv/kernel/riscv_ksyms.c +1 -0
@@ -14,3 +14,4 @@EXPORT_SYMBOL (__asm_copy_from_user);EXPORT_SYMBOL (memset);EXPORT_SYMBOL (memcpy); +EXPORT_SYMBOL (memmove);
arch/riscv/lib/Makefile +1 -0
@@ -2,6 +2,7 @@ lib-y += delay.o lib-y += memcpy.o lib-y += memset.o +lib-y += memmove.o lib-y += uaccess.o lib-$(CONFIG_64BIT) += tishift.o
arch/riscv/lib/memmove.S new file +63 -0
63 lines not on this page · raw message
Signature · 7 lines
1.7.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 1/2] riscv: Add memmove string operation.
Quoted 9 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
This porting refers to the '
arch/nds32/lib/memmove.S'.
This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
Re: [PATCH 1/2] riscv: Add memmove string operation.
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
Quoted 6 lines
Quoted 1 line
This porting refers to the '
arch/nds32/lib/memmove.S'.This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.
Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.
Re: [PATCH 1/2] riscv: Add memmove string operation.
Quoted 22 lines · Palmer Dabbelt
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
Quoted 6 lines
Quoted 1 line
This porting refers to the '
arch/nds32/lib/memmove.S'.This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.
Quoted 5 lines
Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in
lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.
There's a generic memmove implementation in the kernel already:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n362
Nick, could you tell us more about why the generic memmove() isn't suitable?
- Paul
Re: [PATCH 1/2] riscv: Add memmove string operation.
Quoted 32 lines · Paul Walmsley
Quoted 22 lines · Palmer Dabbelt
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
Quoted 6 lines
Quoted 1 line
This porting refers to the '
arch/nds32/lib/memmove.S'.This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.
Quoted 5 lines
Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in
lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.There's a generic memmove implementation in the kernel already:
Quoted 7 lines
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n362
Nick, could you tell us more about why the generic
memmove()isn't suitable?- Paul
Hi Paul,
KASAN has its own string operations(memcpy/memmove/memset) because it needs to hook some code to check memory region. It would undefined the original string operations and called the string operations with the prefix '__'. But the generic string operations didn't declare with the prefix. Other archs with KASAN support like arm64 and xtensa all have their own string operations and defined with the prefix.
Nick
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 1/2] riscv: Add memmove string operation.
Hi Nick,
Quoted 36 lines · Nick Hu
Quoted 28 lines · Paul Walmsley
Quoted 20 lines · Palmer Dabbelt
Quoted 5 lines · Christoph Hellwig
Quoted 4 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
Quoted 8 lines
Quoted 5 lines
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
This porting refers to the '
arch/nds32/lib/memmove.S'.This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.
Quoted 5 lines
Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in
lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.Quoted 6 lines
There's a generic memmove implementation in the kernel already:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n362
Nick, could you tell us more about why the generic
memmove()isn't suitable?
Quoted 6 lines
KASAN has its own string operations(memcpy/memmove/memset) because it needs to hook some code to check memory region. It would undefined the original string operations and called the string operations with the prefix '__'. But the generic string operations didn't declare with the prefix. Other archs with KASAN support like arm64 and xtensa all have their own string operations and defined with the prefix.
Thanks for the explanation. What do you think about Palmer's idea to define a generic C set of KASAN string operations, derived from the newlib code?
- Paul
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 1/2] riscv: Add memmove string operation.
Hi Paul,
Quoted 47 lines · Paul Walmsley
Hi Nick,
Quoted 36 lines · Nick Hu
Quoted 28 lines · Paul Walmsley
Quoted 20 lines · Palmer Dabbelt
Quoted 5 lines · Christoph Hellwig
Quoted 4 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
Quoted 8 lines
Quoted 5 lines
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
This porting refers to the '
arch/nds32/lib/memmove.S'.This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.
Quoted 5 lines
Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in
lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.Quoted 6 lines
There's a generic memmove implementation in the kernel already:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n362
Nick, could you tell us more about why the generic
memmove()isn't suitable?Quoted 6 lines
KASAN has its own string operations(memcpy/memmove/memset) because it needs to hook some code to check memory region. It would undefined the original string operations and called the string operations with the prefix '__'. But the generic string operations didn't declare with the prefix. Other archs with KASAN support like arm64 and xtensa all have their own string operations and defined with the prefix.
Quoted 6 lines
Thanks for the explanation. What do you think about Palmer's idea to define a generic C set of KASAN string operations, derived from the newlib code?
- Paul
That sounds good to me. But it should be another topic. We need to investigate it further about replacing something generic and fundamental in lib/string.c with newlib C functions. Some blind spots may exist. So I suggest, let's consider KASAN for now.
Nick
Re: [PATCH 1/2] riscv: Add memmove string operation.
Quoted 10 lines · Nick Hu
Quoted 3 lines · Paul Walmsley
Thanks for the explanation. What do you think about Palmer's idea to define a generic C set of KASAN string operations, derived from the newlib code?
That sounds good to me. But it should be another topic. We need to investigate it further about replacing something generic and fundamental in
lib/string.cwith newlib C functions. Some blind spots may exist. So I suggest, let's consider KASAN for now.
OK. Here is the problem for us as maintainers. You, Palmer, and I all agree that a C-language version would be better. We'd rather not merge a pure assembly-language version unless it had significant advantages, and right now we're not anticipating that. So that suggests that a C-language memmove() is the right way to go.
But if we merge a C-language memmove() into arch/riscv, other kernel developers would probably ask us why we're doing that, since there's nothing RISC-V-specific about it. So do you think you might reconsider sending patches to add a generic C-language memmove()?
- Paul
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 1/2] riscv: Add memmove string operation.
Hi Paul,
Quoted 19 lines · Paul Walmsley
Quoted 10 lines · Nick Hu
Quoted 3 lines · Paul Walmsley
Thanks for the explanation. What do you think about Palmer's idea to define a generic C set of KASAN string operations, derived from the newlib code?
That sounds good to me. But it should be another topic. We need to investigate it further about replacing something generic and fundamental in
lib/string.cwith newlib C functions. Some blind spots may exist. So I suggest, let's consider KASAN for now.OK. Here is the problem for us as maintainers. You, Palmer, and I all agree that a C-language version would be better. We'd rather not merge a pure assembly-language version unless it had significant advantages, and right now we're not anticipating that. So that suggests that a C-language
memmove()is the right way to go.
Quoted 7 lines
But if we merge a C-language
memmove()intoarch/riscv, other kernel developers would probably ask us why we're doing that, since there's nothing RISC-V-specific about it. So do you think you might reconsider sending patches to add a generic C-languagememmove()?- Paul
About pushing mem*() generic, let's start with the reason why in the first place KASAN needs re-implement its own string operations:
In mm/kasan/common.c:
#undef memset
void *memset(void *addr, int c, size_t len)
{
check_memory_region((unsigned long)addr, len, true, _RET_IP_);
return __memset(addr, c, len);
}KASAN would call the string operations with the prefix '__', which should be just an alias to the proper one.
In the past, every architecture that supports KASAN does this in assembly. E.g. ARM64:
In arch/arm64/lib/memset.S:
ENTRY(__memset) ENTRY(memset) ... ... EXPORT_SYMBOL(memset) EXPORT_SYMBOL(__memset) // export this as an alias
In arch/arm64/include/asm/string.h
#define __HAVE_ARCH_MEMSET extern void *memset(void *, int, __kernel_size_t); extern void *__memset(void *, int, __kernel_size_t);
Now, if we are going to replace the current string operations with newlib ones and let KASAN use them, we must provide something like this:
In lib/string.c:
void *___memset(...)
{
...
}In include/linux/string.h:
#ifndef __HAVE_ARCH_MEMCPY
#ifdef CONFIG_KASAN
static inline void* __memset(...)
{
___memset(...);
}
extern void memset(...); // force those who include this header uses the
memset wrapped by KASAN
#else
static inline void *memset(...)
{
___memset(...);
}
#endif
#endifDoes this look OK to you?
Nick
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 1/2] riscv: Add memmove string operation.
In reply to Paul WalmsleyQuoted 30 lines · Paul Walmsley
Quoted 22 lines · Palmer Dabbelt
Quoted 9 lines · Christoph Hellwig
Quoted 8 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
KASAN's string operations would replace the original string operations and call for the architecture defined string operations. Since we don't have this in current kernel, this patch provides the implementation.
Quoted 6 lines
Quoted 1 line
This porting refers to the '
arch/nds32/lib/memmove.S'.This looks sensible to me, although my stringop asm is rather rusty, so just an ack and not a real review-by:
FWIW, we just write this in C everywhere else and rely on the compiler to unroll the loops. I always prefer C to assembly when possible, so I'd prefer if we just adopt the string code from newlib. We have a RISC-V-specific memcpy in there, but just use the generic memmove.
Quoted 5 lines
Maybe the best bet here would be to adopt the newlib memcpy/memmove as generic Linux functions? They're both in C so they should be fine, and they both look faster than what's in
lib/string.c. Then everyone would benefit and we don't need this tricky RISC-V assembly. Also, from the look of it the newlib code is faster because the inner loop is unrolled.
Quoted 3 lines
There's a generic memmove implementation in the kernel already:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/string.h#n362
That ends up at __builtin_memcpy(), which ends up looking for memcpy() for large copies, which is in lib/string.c. The code in there is just byte at a time memcpy()/memmove(), which is way slower than the newlib stuff.
Quoted 6 lines
Nick, could you tell us more about why the generic
memmove()isn't suitable?- Paul
Re: [PATCH 1/2] riscv: Add memmove string operation.
In reply to Nick HuQuoted 4 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
Compilation error can be fixed by diff bellow (I didn't test it).
If you don't need memmove very early (before kasan_early_init()) than arch-specific not-instrumented memmove() isn't necessary to have.
--- mm/kasan/common.c | 2 ++ 1 file changed, 2 insertions(+)
mm/kasan/common.c +2 -0
@@ -107,6 +107,7 @@ void *memset(void *addr, int c, size_t len)return __memset (addr, c, len); } +#ifdef __HAVE_ARCH_MEMMOVE#undef memmovevoid *memmove (void *dest,const void *src,size_t len) {
@@ -115,6 +116,7 @@ void *memmove(void *dest, const void *src, size_t len) return __memmove(dest, src, len); } +#endif #undef memcpyvoid *memcpy (void *dest,const void *src,size_t len)
Signature · 1 line
2.21.0
Re: [PATCH 1/2] riscv: Add memmove string operation.
Hi Andrey
Quoted 39 lines · Andrey Ryabinin
Quoted 4 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
Compilation error can be fixed by diff bellow (I didn't test it).
If you don't need memmove very early (beforekasan_early_init()) than arch-specific not-instrumentedmemmove()isn't necessary to have.--- mm/kasan/common.c | 2 ++ 1 file changed, 2 insertions(+)mm/kasan/common.c +2
-0@@ -107,6 +107,7 @@ void *memset(void *addr, int c, size_t len)return __memset (addr, c, len); } +#ifdef __HAVE_ARCH_MEMMOVE#undef memmovevoid *memmove (void *dest,const void *src,size_t len) {@@ -115,6 +116,7 @@ void *memmove(void *dest, const void *src, size_t len) return __memmove(dest, src, len); } +#endif #undef memcpyvoid *memcpy (void *dest,const void *src,size_t len)Signature · 1 line
2.21.0
I have confirmed that the string operations are not used before kasan_early_init(). But I can't make sure whether other ARCHs would need it before kasan_early_init(). Do you have any idea to check that? Should I cc all other ARCH maintainers?
Nick
Re: [PATCH 1/2] riscv: Add memmove string operation.
Quoted 35 lines · Nick Hu
Hi Andrey
Quoted 32 lines · Andrey Ryabinin
Quoted 4 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
Compilation error can be fixed by diff bellow (I didn't test it).
If you don't need memmove very early (beforekasan_early_init()) than arch-specific not-instrumentedmemmove()isn't necessary to have.--- mm/kasan/common.c | 2 ++ 1 file changed, 2 insertions(+)mm/kasan/common.c +2
-0@@ -107,6 +107,7 @@ void *memset(void *addr, int c, size_t len)return __memset (addr, c, len); } +#ifdef __HAVE_ARCH_MEMMOVE#undef memmovevoid *memmove (void *dest,const void *src,size_t len) {@@ -115,6 +116,7 @@ void *memmove(void *dest, const void *src, size_t len) return __memmove(dest, src, len); } +#endif
Quoted 10 lines
Quoted 7 lines
mm/kasan/common.c +0
-0#undef memcpyvoid *memcpy (void *dest,const void *src,size_t len)Signature · 1 line
2.21.0I have confirmed that the string operations are not used before
kasan_early_init(). But I can't make sure whether other ARCHs would need it beforekasan_early_init(). Do you have any idea to check that? Should I cc all other ARCH maintainers?
This doesn't affect other ARCHes in any way. If other arches have their own not-instrumented memmove implementation (and they do), they will continue to be able to use it early.
Re: [PATCH 1/2] riscv: Add memmove string operation.
Hi Paul,
Quoted 50 lines · Andrey Ryabinin
Quoted 35 lines · Nick Hu
Hi Andrey
Quoted 32 lines · Andrey Ryabinin
Quoted 4 lines · Nick Hu
There are some features which need this string operation for compilation, like KASAN. So the purpose of this porting is for the features like KASAN which cannot be compiled without it.
Compilation error can be fixed by diff bellow (I didn't test it).
If you don't need memmove very early (beforekasan_early_init()) than arch-specific not-instrumentedmemmove()isn't necessary to have.--- mm/kasan/common.c | 2 ++ 1 file changed, 2 insertions(+)mm/kasan/common.c +2
-0@@ -107,6 +107,7 @@ void *memset(void *addr, int c, size_t len)return __memset (addr, c, len); } +#ifdef __HAVE_ARCH_MEMMOVE#undef memmovevoid *memmove (void *dest,const void *src,size_t len) {@@ -115,6 +116,7 @@ void *memmove(void *dest, const void *src, size_t len) return __memmove(dest, src, len); } +#endif Quoted 10 lines
Quoted 7 lines
mm/kasan/common.c +0
-0#undef memcpyvoid *memcpy (void *dest,const void *src,size_t len)Signature · 1 line
2.21.0I have confirmed that the string operations are not used before
kasan_early_init(). But I can't make sure whether other ARCHs would need it beforekasan_early_init(). Do you have any idea to check that? Should I cc all other ARCH maintainers?
Quoted 2 lines
This doesn't affect other ARCHes in any way. If other arches have their own not-instrumented memmove implementation (and they do), they will continue to be able to use it early.
I prefer Andrey's method since porting the generic string operations with newlib ones should be a separated patch from KASAN.
Nick
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
PATCH 2/2 riscv: Add KASAN support
In reply to Nick HuThis patch ports the feature Kernel Address SANitizer (KASAN).
Note: The start address of shadow memory is at the beginning of kernel space, which is 2^64 - (2^39 / 2) in SV39. The size of the kernel space is 2^38 bytes so the size of shadow memory should be 2^38 / 8. Thus, the shadow memory would not overlap with the fixmap area.
There are currently two limitations in this port,
1. RV64 only: KASAN need large address space for extra shadow memory region.
2. KASAN can't debug the modules since the modules are allocated in VMALLOC area. We mapped the shadow memory, which corresponding to VMALLOC area, to the kasan_early_shadow_page because we don't have enough physical space for all the shadow memory corresponding to VMALLOC area.
--- arch/riscv/Kconfig | 2 + arch/riscv/include/asm/kasan.h | 26 +++++++++ arch/riscv/include/asm/pgtable-64.h | 5 ++ arch/riscv/include/asm/string.h | 7 +++ arch/riscv/kernel/head.S | 3 + arch/riscv/kernel/riscv_ksyms.c | 3 + arch/riscv/kernel/setup.c | 9 +++ arch/riscv/kernel/vmlinux.lds.S | 1 + arch/riscv/lib/memcpy.S | 5 +- arch/riscv/lib/memmove.S | 5 +- arch/riscv/lib/memset.S | 5 +- arch/riscv/mm/Makefile | 6 ++ arch/riscv/mm/kasan_init.c | 102 +++++++++++++++++++++++++++++++++++ 13 files changed, 173 insertions(+), 6 deletions(-) create mode 100644 arch/riscv/include/asm/kasan.h create mode 100644 arch/riscv/mm/kasan_init.c
arch/riscv/Kconfig +2 -0
@@ -54,6 +54,8 @@ config RISCV select EDAC_SUPPORT select ARCH_HAS_GIGANTIC_PAGE select ARCH_WANT_HUGE_PMD_SHARE if 64BIT + select GENERIC_STRNCPY_FROM_USER if KASAN + select HAVE_ARCH_KASAN if MMU config MMU def_bool y
arch/riscv/include/asm/kasan.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#ifndef __ASM_KASAN_H +#define __ASM_KASAN_H + +#ifndef __ASSEMBLY__ + +#ifdef CONFIG_KASAN + +#include <asm/pgtable.h> + +#define KASAN_SHADOW_SCALE_SHIFT 3 + +#define KASAN_SHADOW_SIZE (UL(1) << (38 - KASAN_SHADOW_SCALE_SHIFT)) +#define KASAN_SHADOW_START 0xffffffc000000000 // 2^64 - 2^38 +#define KASAN_SHADOW_END (KASAN_SHADOW_START + KASAN_SHADOW_SIZE) + +#define KASAN_SHADOW_OFFSET (KASAN_SHADOW_END - (1ULL << \ + (64 - KASAN_SHADOW_SCALE_SHIFT))) + +void kasan_init (void ); +asmlinkage void kasan_early_init(void); + +#endif +#endif +#endif
arch/riscv/include/asm/pgtable-64.h +5 -0
@@ -59,6 +59,11 @@ static inline unsigned long pud_page_vaddr(pud_t pud)return (unsigned long )pfn_to_virt (pud_val (pud) >> _PAGE_PFN_SHIFT); } +static inline struct page *pud_page (pud_t pud) +{ +return pfn_to_page (pud_val (pud) >> _PAGE_PFN_SHIFT); +} +#define pmd_index (addr) (((addr) >> PMD_SHIFT) & (PTRS_PER_PMD - 1))static inline pmd_t *pmd_offset (pud_t *pud,unsigned long addr)
arch/riscv/include/asm/string.h +7 -0
@@ -11,11 +11,18 @@#define __HAVE_ARCH_MEMSET extern asmlinkage void *memset(void *, int, size_t); +extern asmlinkage void *__memset(void *, int, size_t);#define __HAVE_ARCH_MEMCPY extern asmlinkage void *memcpy(void *, const void *, size_t); +extern asmlinkage void *__memcpy(void *, const void *, size_t);#define __HAVE_ARCH_MEMMOVE extern asmlinkage void *memmove(void *, const void *, size_t); +extern asmlinkage void *__memmove(void *, const void *, size_t); + +#define memcpy (dst, src, len) __memcpy(dst, src, len) +#define memmove (dst, src, len) __memmove(dst, src, len) +#define memset (s, c, n) __memset(s, c, n)#endif /* _ASM_RISCV_STRING_H */
arch/riscv/kernel/head.S +3 -0
@@ -97,6 +97,9 @@ clear_bss_done: sw zero, TASK_TI_CPU(tp) la sp, init_thread_union + THREAD_SIZE +#ifdef CONFIG_KASAN + call kasan_early_init +#endif /* Start the kernel */ call parse_dtb tail start_kernel
arch/riscv/kernel/riscv_ksyms.c +3 -0
@@ -15,3 +15,6 @@EXPORT_SYMBOL (memset);EXPORT_SYMBOL (memcpy);EXPORT_SYMBOL (memmove); +EXPORT_SYMBOL (__memset); +EXPORT_SYMBOL (__memcpy); +EXPORT_SYMBOL (__memmove);
arch/riscv/kernel/setup.c +9 -0
@@ -24,6 +24,10 @@#include <asm/tlbflush.h> #include <asm/thread_info.h> +#ifdef CONFIG_KASAN +#include <asm/kasan.h> +#endif +#ifdef CONFIG_DUMMY_CONSOLE struct screen_info screen_info = { .orig_video_lines = 30,
@@ -64,12 +68,17 @@ void __init setup_arch(char **cmdline_p)setup_bootmem ();paging_init (); +unflatten_device_tree ();#ifdef CONFIG_SWIOTLB swiotlb_init (1 );#endif +#ifdef CONFIG_KASAN +kasan_init (); +#endif +#ifdef CONFIG_SMP setup_smp ();#endif
arch/riscv/kernel/vmlinux.lds.S +1 -0
@@ -46,6 +46,7 @@ SECTIONS KPROBES_TEXT ENTRY_TEXT IRQENTRY_TEXT + SOFTIRQENTRY_TEXT *(.fixup) _etext = .; }
arch/riscv/lib/memcpy.S +3 -2
@@ -7,7 +7,8 @@ #include <asm/asm.h> /* void *memcpy(void *, const void *, size_t) */ -ENTRY(memcpy) +ENTRY(__memcpy) +WEAK(memcpy) move t6, a0 /* Preserve return value */ /* Defer to byte-oriented copy for small sizes */
@@ -104,4 +105,4 @@ ENTRY(memcpy) bltu a1, a3, 5b 6: ret -END(memcpy) +END(__memcpy)
arch/riscv/lib/memmove.S +3 -2
@@ -3,7 +3,8 @@ #include <linux/linkage.h> #include <asm/asm.h> -ENTRY(memmove) +ENTRY(__memmove) +WEAK(memmove) move t0, a0 move t1, a1
@@ -60,4 +61,4 @@ exit_memcpy: move a1, t1 ret -END(memmove) +END(__memmove)
arch/riscv/lib/memset.S +3 -2
@@ -8,7 +8,8 @@ #include <asm/asm.h> /* void *memset(void *, int, size_t) */ -ENTRY(memset) +ENTRY(__memset) +WEAK(memset) move t0, a0 /* Preserve return value */ /* Defer to byte-oriented fill for small sizes */
@@ -109,4 +110,4 @@ ENTRY(memset) bltu t0, a3, 5b 6: ret -END(memset) +END(__memset)
arch/riscv/mm/Makefile +6 -0
@@ -14,3 +14,9 @@ obj-y += context.o obj-y += sifive_l2_cache.o obj-$(CONFIG_HUGETLB_PAGE) += hugetlbpage.o +obj-$(CONFIG_KASAN) += kasan_init.o + +ifdef CONFIG_KASAN +KASAN_SANITIZE_kasan_init.o := n +KASAN_SANITIZE_init.o := n +endif
arch/riscv/mm/kasan_init.c new file +102 -0
102 lines not on this page · raw message
Signature · 7 lines
1.7.1 _______________________________________________ linux-riscv mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 2/2] riscv: Add KASAN support
Quoted 4 lines
2. KASAN can't debug the modules since the modules are allocated in VMALLOC area. We mapped the shadow memory, which corresponding to VMALLOC area, to the
kasan_early_shadow_pagebecause we don't have enough physical space for all the shadow memory corresponding to VMALLOC area.
How do other architectures solve this problem?
arch/riscv/Kconfig config RISCV · line 57
Quoted 5 lines
arch/riscv/Kconfig +1
-0@@ -54,6 +54,8 @@ config RISCV select EDAC_SUPPORT select ARCH_HAS_GIGANTIC_PAGE select ARCH_WANT_HUGE_PMD_SHARE if 64BIT + select GENERIC_STRNCPY_FROM_USER if KASAN
Is there any reason why we can't always enabled this? Also just enabling the generic efficient strncpy_from_user should probably be a separate patch.
arch/riscv/Kconfig config RISCV · line 58
Quoted 1 line
arch/riscv/Kconfig +1
-0+ select HAVE_ARCH_KASAN if MMU
Based on your cover letter this should be if MMU && 64BIT
arch/riscv/include/asm/string.h line 26
Quoted 11 lines
arch/riscv/include/asm/string.h +6
-0#define __HAVE_ARCH_MEMCPY extern asmlinkage void *memcpy(void *, const void *, size_t); +extern asmlinkage void *__memcpy(void *, const void *, size_t);#define __HAVE_ARCH_MEMMOVE extern asmlinkage void *memmove(void *, const void *, size_t); +extern asmlinkage void *__memmove(void *, const void *, size_t); + +#define memcpy (dst, src, len) __memcpy(dst, src, len) +#define memmove (dst, src, len) __memmove(dst, src, len) +#define memset (s, c, n) __memset(s, c, n)
This looks weird and at least needs a very good comment. Also with this we effectively don't need the non-prefixed prototypes anymore. Also you probably want to split the renaming of the mem* routines into a separate patch with a proper changelog.
arch/riscv/kernel/setup.c line 29
Quoted 6 lines
#include <
asm/tlbflush.h>
#include <asm/thread_info.h>arch/riscv/kernel/setup.c +3
-0+#ifdef CONFIG_KASAN +#include <asm/kasan.h> +#endif
Any good reason to not just always include the header?
arch/riscv/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 72
Quoted 10 lines
arch/riscv/kernel/setup.c +2
-0+#ifdef CONFIG_DUMMY_CONSOLE struct screen_info screen_info = { .orig_video_lines = 30,@@ -64,12 +68,17 @@ void __init setup_arch(char **cmdline_p)setup_bootmem ();paging_init (); +unflatten_device_tree ();
spurious whitespace change.
arch/riscv/kernel/vmlinux.lds.S SECTIONS · line 49
Quoted 9 lines
arch/riscv/kernel/vmlinux.lds.S +1
-0@@ -46,6 +46,7 @@ SECTIONS KPROBES_TEXT ENTRY_TEXT IRQENTRY_TEXT + SOFTIRQENTRY_TEXT
Hmm. What is the relation to kasan here? Maybe we should add this separately with a good changelog?
arch/riscv/kernel/vmlinux.lds.S line 1
Quoted 3 lines
arch/riscv/kernel/vmlinux.lds.S +2
-0+++ b/arch/riscv/mm/kasan_init.c@@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0
This probably also wants a copyright statement.
arch/riscv/kernel/vmlinux.lds.S line 2
Quoted 1 line
arch/riscv/kernel/vmlinux.lds.S +1
-0+ // init for swapper_pg_dir
Please use /* */ style comments.
Re: [PATCH 2/2] riscv: Add KASAN support
Hi Christoph,
Thanks for your reply. I will answer one by one.
Hi Alexander,
Would you help me for the question about SOFTIRQENTRY_TEXT?
Quoted 7 lines · Christoph Hellwig
Quoted 4 lines
2. KASAN can't debug the modules since the modules are allocated in VMALLOC area. We mapped the shadow memory, which corresponding to VMALLOC area, to the
kasan_early_shadow_pagebecause we don't have enough physical space for all the shadow memory corresponding to VMALLOC area.How do other architectures solve this problem?
Other archs like arm64 and x86 allocate modules in their module region.
Quoted 10 lines
arch/riscv/Kconfig config RISCV · line 57
Quoted 5 lines
arch/riscv/Kconfig +1
-0@@ -54,6 +54,8 @@ config RISCV select EDAC_SUPPORT select ARCH_HAS_GIGANTIC_PAGE select ARCH_WANT_HUGE_PMD_SHARE if 64BIT + select GENERIC_STRNCPY_FROM_USER if KASANIs there any reason why we can't always enabled this? Also just enabling the generic efficient
strncpy_from_usershould probably be a separate patch.
You're right, always enable it would be better.
Quoted 8 lines
arch/riscv/Kconfig config RISCV · line 58
Quoted 1 line
arch/riscv/Kconfig +1
-0+ select HAVE_ARCH_KASAN if MMUBased on your cover letter this should be if MMU && 64BIT
Quoted 4 lines
arch/riscv/include/asm/string.h +1
-0#define __HAVE_ARCH_MEMCPY extern asmlinkage void *memcpy(void *, const void *, size_t); +extern asmlinkage void *__memcpy(void *, const void *, size_t);
Quoted 13 lines
arch/riscv/include/asm/string.h line 26
Quoted 7 lines
arch/riscv/include/asm/string.h +5
-0#define __HAVE_ARCH_MEMMOVE extern asmlinkage void *memmove(void *, const void *, size_t); +extern asmlinkage void *__memmove(void *, const void *, size_t); + +#define memcpy (dst, src, len) __memcpy(dst, src, len) +#define memmove (dst, src, len) __memmove(dst, src, len) +#define memset (s, c, n) __memset(s, c, n)This looks weird and at least needs a very good comment. Also with this we effectively don't need the non-prefixed prototypes anymore. Also you probably want to split the renaming of the mem* routines into a separate patch with a proper changelog.
I made some mistakes on this porting, this would be better:
#define __HAVE_ARCH_MEMSET
extern asmlinkage void *memset(void *, int, size_t); extern asmlinkage void *__memset(void *, int, size_t);
#define __HAVE_ARCH_MEMCPY
extern asmlinkage void *memcpy(void *, const void *, size_t); extern asmlinkage void *__memcpy(void *, const void *, size_t);
#define __HAVE_ARCH_MEMMOVE
extern asmlinkage void *memmove(void *, const void *, size_t); extern asmlinkage void *__memmove(void *, const void *, size_t);
#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
#define memcpy(dst, src, len) __memcpy(dst, src, len) #define memmove(dst, src, len) __memmove(dst, src, len) #define memset(s, c, n) __memset(s, c, n)
#endif
Quoted 9 lines
arch/riscv/kernel/setup.c line 29
Quoted 6 lines
#include <
asm/tlbflush.h>
#include <asm/thread_info.h>arch/riscv/kernel/setup.c +3
-0+#ifdef CONFIG_KASAN +#include <asm/kasan.h> +#endif Any good reason to not just always include the header?
Nope, I would remove the '#ifdef CONFIG_KASAN', and do the logic in the header instead.
Quoted 13 lines
arch/riscv/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 72
Quoted 10 lines
arch/riscv/kernel/setup.c +2
-0+#ifdef CONFIG_DUMMY_CONSOLE struct screen_info screen_info = { .orig_video_lines = 30,@@ -64,12 +68,17 @@ void __init setup_arch(char **cmdline_p)setup_bootmem ();paging_init (); +unflatten_device_tree ();spurious whitespace change.
Quoted 13 lines
arch/riscv/kernel/vmlinux.lds.S SECTIONS · line 49
Quoted 9 lines
arch/riscv/kernel/vmlinux.lds.S +1
-0@@ -46,6 +46,7 @@ SECTIONS KPROBES_TEXT ENTRY_TEXT IRQENTRY_TEXT + SOFTIRQENTRY_TEXTHmm. What is the relation to kasan here? Maybe we should add this separately with a good changelog?
There is a commit for it:
Author: Alexander Potapenko <[email protected]> Date: Fri Mar 25 14:22:05 2016 -0700
arch, ftrace: for KASAN put hard/soft IRQ entries into separate sections
KASAN needs to know whether the allocation happens in an IRQ handler. This lets us strip everything below the IRQ entry point to reduce the number of unique stack traces needed to be stored.
Move the definition of __irq_entry to <linux/interrupt.h> so that the users don't need to pull in <linux/ftrace.h>. Also introduce the __softirq_entry macro which is similar to __irq_entry, but puts the corresponding functions to the .softirqentry.text section.
After reading the patch I understand that soft/hard IRQ entries should be separated for KASAN to work, but why?
Alexender, do you have any comments on this?
Quoted 9 lines
arch/riscv/kernel/vmlinux.lds.S line 1
Quoted 3 lines
arch/riscv/kernel/vmlinux.lds.S +2
-0+++ b/arch/riscv/mm/kasan_init.c@@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0This probably also wants a copyright statement.
arch/riscv/kernel/vmlinux.lds.S line 2
Quoted 1 line
arch/riscv/kernel/vmlinux.lds.S +1
-0+ // init for swapper_pg_dirPlease use /* */ style comments.
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Re: [PATCH 2/2] riscv: Add KASAN support
Quoted 25 lines · Nick Hu
Quoted 14 lines
arch/riscv/kernel/vmlinux.lds.S SECTIONS · line 49
Quoted 10 lines
arch/riscv/kernel/vmlinux.lds.S +1
-0@@ -46,6 +46,7 @@ SECTIONS KPROBES_TEXT ENTRY_TEXT IRQENTRY_TEXT + SOFTIRQENTRY_TEXTHmm. What is the relation to kasan here? Maybe we should add this separately with a good changelog?
There is a commit for it:
Author: Alexander Potapenko <[email protected]> Date: Fri Mar 25 14:22:05 2016 -0700arch, ftrace: for KASAN put hard/soft IRQ entries into separate sections
KASAN needs to know whether the allocation happens in an IRQ handler. This lets us strip everything below the IRQ entry point to reduce the number of unique stack traces needed to be stored.
Quoted 8 lines
Move the definition of
__irq_entryto <linux/interrupt.h> so that the users don't need to pull in <linux/ftrace.h>. Also introduce the__softirq_entrymacro which is similar to__irq_entry, but puts the corresponding functions to the .softirqentry.text section.After reading the patch I understand that soft/hard IRQ entries should be separated for KASAN to work, but why?
KASAN doesn't need soft/hard IRQ entries separated. KASAN wants to know the entry point of IRQ (hard or soft) to filter out random non-irq part of the stacktrace before feeding it to stack_depot_save. See filter_irq_stacks().
Re: [PATCH 2/2] riscv: Add KASAN support
In reply to Nick HuQuoted 9 lines · Nick Hu
Hi Christoph,
Thanks for your reply. I will answer one by one.
Hi Alexander,
Would you help me for the question about
SOFTIRQENTRY_TEXT?On Mon, Aug 12, 2019 at 11:10:50PM +0800, Christoph Hellwig wrote:
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
2. KASAN can't debug the modules since the modules are allocated in VMALLOC area. We mapped the shadow memory, which corresponding to VMALLOC area, to the
kasan_early_shadow_pagebecause we don't have enough physical space for all the shadow memory corresponding to VMALLOC area.How do other architectures solve this problem?
Other archs like arm64 and x86 allocate modules in their module region.
I've run in to a similar difficulty in ppc64. My approach has been to add a generic feature to allow kasan to handle vmalloc areas:
https://lore.kernel.org/linux-mm/[email protected]/
I link this with ppc64 in this series:
https://lore.kernel.org/linuxppc-dev/[email protected]/
However, see Christophe Leroy's comments: he thinks I should take a different approach in a number of places, including just adding a separate module area. I haven't had time to think through all of his proposals yet; in particular I'd want to think through what the implication of a separate module area is for KASLR.
Regards,
Daniel
Quoted 129 lines
Quoted 11 lines
arch/riscv/Kconfig config RISCV · line 57
Quoted 6 lines
arch/riscv/Kconfig +1
-0@@ -54,6 +54,8 @@ config RISCV select EDAC_SUPPORT select ARCH_HAS_GIGANTIC_PAGE select ARCH_WANT_HUGE_PMD_SHARE if 64BIT + select GENERIC_STRNCPY_FROM_USER if KASANIs there any reason why we can't always enabled this? Also just enabling the generic efficient
strncpy_from_usershould probably be a separate patch.You're right, always enable it would be better.
Quoted 8 lines
arch/riscv/Kconfig config RISCV · line 58
Quoted 1 line
arch/riscv/Kconfig +1
-0+ select HAVE_ARCH_KASAN if MMUBased on your cover letter this should be if MMU && 64BIT
Quoted 4 lines
arch/riscv/include/asm/string.h +1
-0#define __HAVE_ARCH_MEMCPY extern asmlinkage void *memcpy(void *, const void *, size_t); +extern asmlinkage void *__memcpy(void *, const void *, size_t);Quoted 13 lines
arch/riscv/include/asm/string.h line 26
Quoted 7 lines
arch/riscv/include/asm/string.h +5
-0#define __HAVE_ARCH_MEMMOVE extern asmlinkage void *memmove(void *, const void *, size_t); +extern asmlinkage void *__memmove(void *, const void *, size_t); + +#define memcpy (dst, src, len) __memcpy(dst, src, len) +#define memmove (dst, src, len) __memmove(dst, src, len) +#define memset (s, c, n) __memset(s, c, n)This looks weird and at least needs a very good comment. Also with this we effectively don't need the non-prefixed prototypes anymore. Also you probably want to split the renaming of the mem* routines into a separate patch with a proper changelog.
I made some mistakes on this porting, this would be better:
#define
__HAVE_ARCH_MEMSET
extern asmlinkage void *memset(void *, int,size_t); extern asmlinkage void *__memset(void *, int,size_t);#define
__HAVE_ARCH_MEMCPY
extern asmlinkage void *memcpy(void *, const void *,size_t); extern asmlinkage void *__memcpy(void *, const void *,size_t);#define
__HAVE_ARCH_MEMMOVE
extern asmlinkage void *memmove(void *, const void *,size_t); extern asmlinkage void *__memmove(void *, const void *,size_t);#if
defined(CONFIG_KASAN)&& !defined(__SANITIZE_ADDRESS__)#define
memcpy(dst, src, len)__memcpy(dst, src, len)#definememmove(dst, src, len)__memmove(dst, src, len)#definememset(s, c, n)__memset(s, c, n)#endif
Quoted 9 lines
arch/riscv/kernel/setup.c line 29
Quoted 6 lines
#include <
asm/tlbflush.h>
#include <asm/thread_info.h>arch/riscv/kernel/setup.c +3
-0+#ifdef CONFIG_KASAN +#include <asm/kasan.h> +#endif Any good reason to not just always include the header?
Nope, I would remove the '#ifdef
CONFIG_KASAN', and do the logic in the header instead.Quoted 13 lines
arch/riscv/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 72
Quoted 10 lines
arch/riscv/kernel/setup.c +2
-0+#ifdef CONFIG_DUMMY_CONSOLE struct screen_info screen_info = { .orig_video_lines = 30,@@ -64,12 +68,17 @@ void __init setup_arch(char **cmdline_p)setup_bootmem ();paging_init (); +unflatten_device_tree ();spurious whitespace change.
Quoted 13 lines
arch/riscv/kernel/vmlinux.lds.S SECTIONS · line 49
Quoted 9 lines
arch/riscv/kernel/vmlinux.lds.S +1
-0@@ -46,6 +46,7 @@ SECTIONS KPROBES_TEXT ENTRY_TEXT IRQENTRY_TEXT + SOFTIRQENTRY_TEXTHmm. What is the relation to kasan here? Maybe we should add this separately with a good changelog?
There is a commit for it:
Author: Alexander Potapenko <[email protected]> Date: Fri Mar 25 14:22:05 2016 -0700arch, ftrace: for KASAN put hard/soft IRQ entries into separate sections
KASAN needs to know whether the allocation happens in an IRQ handler. This lets us strip everything below the IRQ entry point to reduce the number of unique stack traces needed to be stored.
Move the definition of
__irq_entryto <linux/interrupt.h> so that the users don't need to pull in <linux/ftrace.h>. Also introduce the__softirq_entrymacro which is similar to__irq_entry, but puts the corresponding functions to the .softirqentry.text section.After reading the patch I understand that soft/hard IRQ entries should be separated for KASAN to work, but why?
Alexender, do you have any comments on this?
Quoted 9 lines
arch/riscv/kernel/vmlinux.lds.S line 1
Quoted 3 lines
arch/riscv/kernel/vmlinux.lds.S +2
-0+++ b/arch/riscv/mm/kasan_init.c@@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0This probably also wants a copyright statement.
arch/riscv/kernel/vmlinux.lds.S line 2
Quoted 1 line
arch/riscv/kernel/vmlinux.lds.S +1
-0+ // init for swapper_pg_dirPlease use /* */ style comments.
Signature · 3 lines
You received this message because you are subscribed to the Google Groups "kasan-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190814074417.GA21929%40andestech.com.
Re: [PATCH 2/2] riscv: Add KASAN support
Hi Daniel,
Quoted 29 lines · Daniel Axtens
Quoted 9 lines · Nick Hu
Hi Christoph,
Thanks for your reply. I will answer one by one.
Hi Alexander,
Would you help me for the question about
SOFTIRQENTRY_TEXT?On Mon, Aug 12, 2019 at 11:10:50PM +0800, Christoph Hellwig wrote:
Quoted 8 lines
Quoted 7 lines
Quoted 4 lines
2. KASAN can't debug the modules since the modules are allocated in VMALLOC area. We mapped the shadow memory, which corresponding to VMALLOC area, to the
kasan_early_shadow_pagebecause we don't have enough physical space for all the shadow memory corresponding to VMALLOC area.How do other architectures solve this problem?
Other archs like arm64 and x86 allocate modules in their module region.
I've run in to a similar difficulty in ppc64. My approach has been to add a generic feature to allow kasan to handle vmalloc areas:
https://lore.kernel.org/linux-mm/[email protected]/
I link this with ppc64 in this series:
Quoted 9 lines
However, see Christophe Leroy's comments: he thinks I should take a different approach in a number of places, including just adding a separate module area. I haven't had time to think through all of his proposals yet; in particular I'd want to think through what the implication of a separate module area is for KASLR.
Regards,
Daniel
Thanks for the advice! I would study on it.
Regards,
Nick
Quoted 121 lines
Quoted 121 lines
Quoted 11 lines
arch/riscv/Kconfig config RISCV · line 57
Quoted 6 lines
arch/riscv/Kconfig +1
-0@@ -54,6 +54,8 @@ config RISCV select EDAC_SUPPORT select ARCH_HAS_GIGANTIC_PAGE select ARCH_WANT_HUGE_PMD_SHARE if 64BIT + select GENERIC_STRNCPY_FROM_USER if KASANIs there any reason why we can't always enabled this? Also just enabling the generic efficient
strncpy_from_usershould probably be a separate patch.You're right, always enable it would be better.
Quoted 8 lines
arch/riscv/Kconfig config RISCV · line 58
Quoted 1 line
arch/riscv/Kconfig +1
-0+ select HAVE_ARCH_KASAN if MMUBased on your cover letter this should be if MMU && 64BIT
Quoted 4 lines
arch/riscv/include/asm/string.h +1
-0#define __HAVE_ARCH_MEMCPY extern asmlinkage void *memcpy(void *, const void *, size_t); +extern asmlinkage void *__memcpy(void *, const void *, size_t);Quoted 13 lines
arch/riscv/include/asm/string.h line 26
Quoted 7 lines
arch/riscv/include/asm/string.h +5
-0#define __HAVE_ARCH_MEMMOVE extern asmlinkage void *memmove(void *, const void *, size_t); +extern asmlinkage void *__memmove(void *, const void *, size_t); + +#define memcpy (dst, src, len) __memcpy(dst, src, len) +#define memmove (dst, src, len) __memmove(dst, src, len) +#define memset (s, c, n) __memset(s, c, n)This looks weird and at least needs a very good comment. Also with this we effectively don't need the non-prefixed prototypes anymore. Also you probably want to split the renaming of the mem* routines into a separate patch with a proper changelog.
I made some mistakes on this porting, this would be better:
#define
__HAVE_ARCH_MEMSET
extern asmlinkage void *memset(void *, int,size_t); extern asmlinkage void *__memset(void *, int,size_t);#define
__HAVE_ARCH_MEMCPY
extern asmlinkage void *memcpy(void *, const void *,size_t); extern asmlinkage void *__memcpy(void *, const void *,size_t);#define
__HAVE_ARCH_MEMMOVE
extern asmlinkage void *memmove(void *, const void *,size_t); extern asmlinkage void *__memmove(void *, const void *,size_t);#if
defined(CONFIG_KASAN)&& !defined(__SANITIZE_ADDRESS__)#define
memcpy(dst, src, len)__memcpy(dst, src, len)#definememmove(dst, src, len)__memmove(dst, src, len)#definememset(s, c, n)__memset(s, c, n)#endif
Quoted 9 lines
arch/riscv/kernel/setup.c line 29
Quoted 6 lines
#include <
asm/tlbflush.h>
#include <asm/thread_info.h>arch/riscv/kernel/setup.c +3
-0+#ifdef CONFIG_KASAN +#include <asm/kasan.h> +#endif Any good reason to not just always include the header?
Nope, I would remove the '#ifdef
CONFIG_KASAN', and do the logic in the header instead.Quoted 13 lines
arch/riscv/kernel/setup.c void __init setup_arch(char **cmdline_p) · line 72
Quoted 10 lines
arch/riscv/kernel/setup.c +2
-0+#ifdef CONFIG_DUMMY_CONSOLE struct screen_info screen_info = { .orig_video_lines = 30,@@ -64,12 +68,17 @@ void __init setup_arch(char **cmdline_p)setup_bootmem ();paging_init (); +unflatten_device_tree ();spurious whitespace change.
Quoted 13 lines
arch/riscv/kernel/vmlinux.lds.S SECTIONS · line 49
Quoted 9 lines
arch/riscv/kernel/vmlinux.lds.S +1
-0@@ -46,6 +46,7 @@ SECTIONS KPROBES_TEXT ENTRY_TEXT IRQENTRY_TEXT + SOFTIRQENTRY_TEXTHmm. What is the relation to kasan here? Maybe we should add this separately with a good changelog?
There is a commit for it:
Author: Alexander Potapenko <[email protected]> Date: Fri Mar 25 14:22:05 2016 -0700arch, ftrace: for KASAN put hard/soft IRQ entries into separate sections
KASAN needs to know whether the allocation happens in an IRQ handler. This lets us strip everything below the IRQ entry point to reduce the number of unique stack traces needed to be stored.
Move the definition of
__irq_entryto <linux/interrupt.h> so that the users don't need to pull in <linux/ftrace.h>. Also introduce the__softirq_entrymacro which is similar to__irq_entry, but puts the corresponding functions to the .softirqentry.text section.After reading the patch I understand that soft/hard IRQ entries should be separated for KASAN to work, but why?
Alexender, do you have any comments on this?
Quoted 6 lines
arch/riscv/kernel/vmlinux.lds.S line 1
Quoted 3 lines
arch/riscv/kernel/vmlinux.lds.S +2
-0+++ b/arch/riscv/mm/kasan_init.c@@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0This probably also wants a copyright statement.
Quoted 8 lines
Quoted 8 lines
Quoted 3 lines
arch/riscv/kernel/vmlinux.lds.S line 2
Quoted 1 line
arch/riscv/kernel/vmlinux.lds.S +1
-0+ // init for swapper_pg_dirPlease use /* */ style comments.
Signature · 3 lines
You received this message because you are subscribed to the Google Groups "kasan-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190814074417.GA21929%40andestech.com.
_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv