* Re: [RFC PATCH v2 3/3] kasan: add interceptors for all string functions [not found] ` <51a6d9d7185de310f37ccbd7e4ebfdd6c7e9791f.1553785020.git.christophe.leroy@c-s.fr> @ 2019-04-02 9:43 ` Christophe Leroy 2019-04-02 12:58 ` Dmitry Vyukov 2019-04-02 16:14 ` Andrey Ryabinin 0 siblings, 2 replies; 5+ messages in thread From: Christophe Leroy @ 2019-04-02 9:43 UTC (permalink / raw) To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin, Alexander Potapenko, Dmitry Vyukov, Daniel Axtens Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev Hi Dmitry, Andrey and others, Do you have any comments to this series ? I'd like to know if this approach is ok or if it is better to keep doing as in https://patchwork.ozlabs.org/patch/1055788/ Thanks Christophe Le 28/03/2019 à 16:00, Christophe Leroy a écrit : > In the same spirit as commit 393f203f5fd5 ("x86_64: kasan: add > interceptors for memset/memmove/memcpy functions"), this patch > adds interceptors for string manipulation functions so that we > can compile lib/string.o without kasan support hence allow the > string functions to also be used from places where kasan has > to be disabled. > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > --- > v2: Fixed a few checkpatch stuff and added missing EXPORT_SYMBOL() and missing #undefs > > include/linux/string.h | 79 ++++++++++ > lib/Makefile | 2 + > lib/string.c | 8 + > mm/kasan/string.c | 394 +++++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 483 insertions(+) > > diff --git a/include/linux/string.h b/include/linux/string.h > index 7927b875f80c..3d2aff2ed402 100644 > --- a/include/linux/string.h > +++ b/include/linux/string.h > @@ -19,54 +19,117 @@ extern void *memdup_user_nul(const void __user *, size_t); > */ > #include <asm/string.h> > > +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__) > +/* > + * For files that are not instrumented (e.g. mm/slub.c) we > + * should use not instrumented version of mem* functions. > + */ > +#define memset16 __memset16 > +#define memset32 __memset32 > +#define memset64 __memset64 > +#define memzero_explicit __memzero_explicit > +#define strcpy __strcpy > +#define strncpy __strncpy > +#define strlcpy __strlcpy > +#define strscpy __strscpy > +#define strcat __strcat > +#define strncat __strncat > +#define strlcat __strlcat > +#define strcmp __strcmp > +#define strncmp __strncmp > +#define strcasecmp __strcasecmp > +#define strncasecmp __strncasecmp > +#define strchr __strchr > +#define strchrnul __strchrnul > +#define strrchr __strrchr > +#define strnchr __strnchr > +#define skip_spaces __skip_spaces > +#define strim __strim > +#define strstr __strstr > +#define strnstr __strnstr > +#define strlen __strlen > +#define strnlen __strnlen > +#define strpbrk __strpbrk > +#define strsep __strsep > +#define strspn __strspn > +#define strcspn __strcspn > +#define memscan __memscan > +#define memcmp __memcmp > +#define memchr __memchr > +#define memchr_inv __memchr_inv > +#define strreplace __strreplace > + > +#ifndef __NO_FORTIFY > +#define __NO_FORTIFY /* FORTIFY_SOURCE uses __builtin_memcpy, etc. */ > +#endif > + > +#endif > + > #ifndef __HAVE_ARCH_STRCPY > extern char * strcpy(char *,const char *); > +char *__strcpy(char *, const char *); > #endif > #ifndef __HAVE_ARCH_STRNCPY > extern char * strncpy(char *,const char *, __kernel_size_t); > +char *__strncpy(char *, const char *, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_STRLCPY > size_t strlcpy(char *, const char *, size_t); > +size_t __strlcpy(char *, const char *, size_t); > #endif > #ifndef __HAVE_ARCH_STRSCPY > ssize_t strscpy(char *, const char *, size_t); > +ssize_t __strscpy(char *, const char *, size_t); > #endif > #ifndef __HAVE_ARCH_STRCAT > extern char * strcat(char *, const char *); > +char *__strcat(char *, const char *); > #endif > #ifndef __HAVE_ARCH_STRNCAT > extern char * strncat(char *, const char *, __kernel_size_t); > +char *__strncat(char *, const char *, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_STRLCAT > extern size_t strlcat(char *, const char *, __kernel_size_t); > +size_t __strlcat(char *, const char *, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_STRCMP > extern int strcmp(const char *,const char *); > +int __strcmp(const char *, const char *); > #endif > #ifndef __HAVE_ARCH_STRNCMP > extern int strncmp(const char *,const char *,__kernel_size_t); > +int __strncmp(const char *, const char *, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_STRCASECMP > extern int strcasecmp(const char *s1, const char *s2); > +int __strcasecmp(const char *s1, const char *s2); > #endif > #ifndef __HAVE_ARCH_STRNCASECMP > extern int strncasecmp(const char *s1, const char *s2, size_t n); > +int __strncasecmp(const char *s1, const char *s2, size_t n); > #endif > #ifndef __HAVE_ARCH_STRCHR > extern char * strchr(const char *,int); > +char *__strchr(const char *, int); > #endif > #ifndef __HAVE_ARCH_STRCHRNUL > extern char * strchrnul(const char *,int); > +char *__strchrnul(const char *, int); > #endif > #ifndef __HAVE_ARCH_STRNCHR > extern char * strnchr(const char *, size_t, int); > +char *__strnchr(const char *, size_t, int); > #endif > #ifndef __HAVE_ARCH_STRRCHR > extern char * strrchr(const char *,int); > +char *__strrchr(const char *, int); > #endif > extern char * __must_check skip_spaces(const char *); > +char * __must_check __skip_spaces(const char *); > > extern char *strim(char *); > +char *__strim(char *); > > static inline __must_check char *strstrip(char *str) > { > @@ -75,27 +138,35 @@ static inline __must_check char *strstrip(char *str) > > #ifndef __HAVE_ARCH_STRSTR > extern char * strstr(const char *, const char *); > +char *__strstr(const char *, const char *); > #endif > #ifndef __HAVE_ARCH_STRNSTR > extern char * strnstr(const char *, const char *, size_t); > +char *__strnstr(const char *, const char *, size_t); > #endif > #ifndef __HAVE_ARCH_STRLEN > extern __kernel_size_t strlen(const char *); > +__kernel_size_t __strlen(const char *); > #endif > #ifndef __HAVE_ARCH_STRNLEN > extern __kernel_size_t strnlen(const char *,__kernel_size_t); > +__kernel_size_t __strnlen(const char *, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_STRPBRK > extern char * strpbrk(const char *,const char *); > +char *__strpbrk(const char *, const char *); > #endif > #ifndef __HAVE_ARCH_STRSEP > extern char * strsep(char **,const char *); > +char *__strsep(char **, const char *); > #endif > #ifndef __HAVE_ARCH_STRSPN > extern __kernel_size_t strspn(const char *,const char *); > +__kernel_size_t __strspn(const char *, const char *); > #endif > #ifndef __HAVE_ARCH_STRCSPN > extern __kernel_size_t strcspn(const char *,const char *); > +__kernel_size_t __strcspn(const char *, const char *); > #endif > > #ifndef __HAVE_ARCH_MEMSET > @@ -104,14 +175,17 @@ extern void * memset(void *,int,__kernel_size_t); > > #ifndef __HAVE_ARCH_MEMSET16 > extern void *memset16(uint16_t *, uint16_t, __kernel_size_t); > +void *__memset16(uint16_t *, uint16_t, __kernel_size_t); > #endif > > #ifndef __HAVE_ARCH_MEMSET32 > extern void *memset32(uint32_t *, uint32_t, __kernel_size_t); > +void *__memset32(uint32_t *, uint32_t, __kernel_size_t); > #endif > > #ifndef __HAVE_ARCH_MEMSET64 > extern void *memset64(uint64_t *, uint64_t, __kernel_size_t); > +void *__memset64(uint64_t *, uint64_t, __kernel_size_t); > #endif > > static inline void *memset_l(unsigned long *p, unsigned long v, > @@ -146,12 +220,15 @@ extern void * memmove(void *,const void *,__kernel_size_t); > #endif > #ifndef __HAVE_ARCH_MEMSCAN > extern void * memscan(void *,int,__kernel_size_t); > +void *__memscan(void *, int, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_MEMCMP > extern int memcmp(const void *,const void *,__kernel_size_t); > +int __memcmp(const void *, const void *, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_MEMCHR > extern void * memchr(const void *,int,__kernel_size_t); > +void *__memchr(const void *, int, __kernel_size_t); > #endif > #ifndef __HAVE_ARCH_MEMCPY_MCSAFE > static inline __must_check unsigned long memcpy_mcsafe(void *dst, > @@ -168,7 +245,9 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt) > } > #endif > void *memchr_inv(const void *s, int c, size_t n); > +void *__memchr_inv(const void *s, int c, size_t n); > char *strreplace(char *s, char old, char new); > +char *__strreplace(char *s, char old, char new); > > extern void kfree_const(const void *x); > > diff --git a/lib/Makefile b/lib/Makefile > index 30b9b0bfbba9..19d0237f9b9c 100644 > --- a/lib/Makefile > +++ b/lib/Makefile > @@ -18,6 +18,8 @@ KCOV_INSTRUMENT_list_debug.o := n > KCOV_INSTRUMENT_debugobjects.o := n > KCOV_INSTRUMENT_dynamic_debug.o := n > > +KASAN_SANITIZE_string.o := n > + > lib-y := ctype.o string.o string_sysfs.o vsprintf.o cmdline.o \ > rbtree.o radix-tree.o timerqueue.o xarray.o \ > idr.o int_sqrt.o extable.o \ > diff --git a/lib/string.c b/lib/string.c > index f3886c5175ac..31a253201bba 100644 > --- a/lib/string.c > +++ b/lib/string.c > @@ -85,7 +85,9 @@ EXPORT_SYMBOL(strcasecmp); > * @dest: Where to copy the string to > * @src: Where to copy the string from > */ > +#ifndef CONFIG_KASAN > #undef strcpy > +#endif > char *strcpy(char *dest, const char *src) > { > char *tmp = dest; > @@ -243,7 +245,9 @@ EXPORT_SYMBOL(strscpy); > * @dest: The string to be appended to > * @src: The string to append to it > */ > +#ifndef CONFIG_KASAN > #undef strcat > +#endif > char *strcat(char *dest, const char *src) > { > char *tmp = dest; > @@ -319,7 +323,9 @@ EXPORT_SYMBOL(strlcat); > * @cs: One string > * @ct: Another string > */ > +#ifndef CONFIG_KASAN > #undef strcmp > +#endif > int strcmp(const char *cs, const char *ct) > { > unsigned char c1, c2; > @@ -773,7 +779,9 @@ EXPORT_SYMBOL(memmove); > * @ct: Another area of memory > * @count: The size of the area. > */ > +#ifndef CONFIG_KASAN > #undef memcmp > +#endif > __visible int memcmp(const void *cs, const void *ct, size_t count) > { > const unsigned char *su1, *su2; > diff --git a/mm/kasan/string.c b/mm/kasan/string.c > index 083b967255a2..0db31bbbf643 100644 > --- a/mm/kasan/string.c > +++ b/mm/kasan/string.c > @@ -35,6 +35,42 @@ void *memset(void *addr, int c, size_t len) > return __memset(addr, c, len); > } > > +#undef memset16 > +void *memset16(uint16_t *s, uint16_t v, size_t count) > +{ > + check_memory_region((unsigned long)s, count << 1, true, _RET_IP_); > + > + return __memset16(s, v, count); > +} > +EXPORT_SYMBOL(memset16); > + > +#undef memset32 > +void *memset32(uint32_t *s, uint32_t v, size_t count) > +{ > + check_memory_region((unsigned long)s, count << 2, true, _RET_IP_); > + > + return __memset32(s, v, count); > +} > +EXPORT_SYMBOL(memset32); > + > +#undef memset64 > +void *memset64(uint64_t *s, uint64_t v, size_t count) > +{ > + check_memory_region((unsigned long)s, count << 3, true, _RET_IP_); > + > + return __memset64(s, v, count); > +} > +EXPORT_SYMBOL(memset64); > + > +#undef memzero_explicit > +void memzero_explicit(void *s, size_t count) > +{ > + check_memory_region((unsigned long)s, count, true, _RET_IP_); > + > + return __memzero_explicit(s, count); > +} > +EXPORT_SYMBOL(memzero_explicit); > + > #undef memmove > void *memmove(void *dest, const void *src, size_t len) > { > @@ -52,3 +88,361 @@ void *memcpy(void *dest, const void *src, size_t len) > > return __memcpy(dest, src, len); > } > + > +#undef strcpy > +char *strcpy(char *dest, const char *src) > +{ > + size_t len = __strlen(src) + 1; > + > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > + check_memory_region((unsigned long)dest, len, true, _RET_IP_); > + > + return __strcpy(dest, src); > +} > +EXPORT_SYMBOL(strcpy); > + > +#undef strncpy > +char *strncpy(char *dest, const char *src, size_t count) > +{ > + size_t len = min(__strlen(src) + 1, count); > + > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > + check_memory_region((unsigned long)dest, count, true, _RET_IP_); > + > + return __strncpy(dest, src, count); > +} > +EXPORT_SYMBOL(strncpy); > + > +#undef strlcpy > +size_t strlcpy(char *dest, const char *src, size_t size) > +{ > + size_t len = __strlen(src) + 1; > + > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > + check_memory_region((unsigned long)dest, min(len, size), true, _RET_IP_); > + > + return __strlcpy(dest, src, size); > +} > +EXPORT_SYMBOL(strlcpy); > + > +#undef strscpy > +ssize_t strscpy(char *dest, const char *src, size_t count) > +{ > + int len = min(__strlen(src) + 1, count); > + > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > + check_memory_region((unsigned long)dest, len, true, _RET_IP_); > + > + return __strscpy(dest, src, count); > +} > +EXPORT_SYMBOL(strscpy); > + > +#undef strcat > +char *strcat(char *dest, const char *src) > +{ > + size_t slen = __strlen(src) + 1; > + size_t dlen = __strlen(dest); > + > + check_memory_region((unsigned long)src, slen, false, _RET_IP_); > + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); > + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); > + > + return __strcat(dest, src); > +} > +EXPORT_SYMBOL(strcat); > + > +#undef strncat > +char *strncat(char *dest, const char *src, size_t count) > +{ > + size_t slen = min(__strlen(src) + 1, count); > + size_t dlen = __strlen(dest); > + > + check_memory_region((unsigned long)src, slen, false, _RET_IP_); > + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); > + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); > + > + return __strncat(dest, src, count); > +} > +EXPORT_SYMBOL(strncat); > + > +#undef strlcat > +size_t strlcat(char *dest, const char *src, size_t count) > +{ > + size_t slen = min(__strlen(src) + 1, count); > + size_t dlen = __strlen(dest); > + > + check_memory_region((unsigned long)src, slen, false, _RET_IP_); > + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); > + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); > + > + return __strlcat(dest, src, count); > +} > +EXPORT_SYMBOL(strlcat); > + > +#undef strcmp > +int strcmp(const char *cs, const char *ct) > +{ > + size_t len = min(__strlen(cs) + 1, __strlen(ct) + 1); > + > + check_memory_region((unsigned long)cs, len, false, _RET_IP_); > + check_memory_region((unsigned long)ct, len, false, _RET_IP_); > + > + return __strcmp(cs, ct); > +} > +EXPORT_SYMBOL(strcmp); > + > +#undef strncmp > +int strncmp(const char *cs, const char *ct, size_t count) > +{ > + size_t len = min3(__strlen(cs) + 1, __strlen(ct) + 1, count); > + > + check_memory_region((unsigned long)cs, len, false, _RET_IP_); > + check_memory_region((unsigned long)ct, len, false, _RET_IP_); > + > + return __strncmp(cs, ct, count); > +} > +EXPORT_SYMBOL(strncmp); > + > +#undef strcasecmp > +int strcasecmp(const char *s1, const char *s2) > +{ > + size_t len = min(__strlen(s1) + 1, __strlen(s2) + 1); > + > + check_memory_region((unsigned long)s1, len, false, _RET_IP_); > + check_memory_region((unsigned long)s2, len, false, _RET_IP_); > + > + return __strcasecmp(s1, s2); > +} > +EXPORT_SYMBOL(strcasecmp); > + > +#undef strncasecmp > +int strncasecmp(const char *s1, const char *s2, size_t len) > +{ > + size_t sz = min3(__strlen(s1) + 1, __strlen(s2) + 1, len); > + > + check_memory_region((unsigned long)s1, sz, false, _RET_IP_); > + check_memory_region((unsigned long)s2, sz, false, _RET_IP_); > + > + return __strncasecmp(s1, s2, len); > +} > +EXPORT_SYMBOL(strncasecmp); > + > +#undef strchr > +char *strchr(const char *s, int c) > +{ > + size_t len = __strlen(s) + 1; > + > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > + > + return __strchr(s, c); > +} > +EXPORT_SYMBOL(strchr); > + > +#undef strchrnul > +char *strchrnul(const char *s, int c) > +{ > + size_t len = __strlen(s) + 1; > + > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > + > + return __strchrnul(s, c); > +} > +EXPORT_SYMBOL(strchrnul); > + > +#undef strrchr > +char *strrchr(const char *s, int c) > +{ > + size_t len = __strlen(s) + 1; > + > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > + > + return __strrchr(s, c); > +} > +EXPORT_SYMBOL(strrchr); > + > +#undef strnchr > +char *strnchr(const char *s, size_t count, int c) > +{ > + size_t len = __strlen(s) + 1; > + > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > + > + return __strnchr(s, count, c); > +} > +EXPORT_SYMBOL(strnchr); > + > +#undef skip_spaces > +char *skip_spaces(const char *str) > +{ > + size_t len = __strlen(str) + 1; > + > + check_memory_region((unsigned long)str, len, false, _RET_IP_); > + > + return __skip_spaces(str); > +} > +EXPORT_SYMBOL(skip_spaces); > + > +#undef strim > +char *strim(char *s) > +{ > + size_t len = __strlen(s) + 1; > + > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > + > + return __strim(s); > +} > +EXPORT_SYMBOL(strim); > + > +#undef strstr > +char *strstr(const char *s1, const char *s2) > +{ > + size_t l1 = __strlen(s1) + 1; > + size_t l2 = __strlen(s2) + 1; > + > + check_memory_region((unsigned long)s1, l1, false, _RET_IP_); > + check_memory_region((unsigned long)s2, l2, false, _RET_IP_); > + > + return __strstr(s1, s2); > +} > +EXPORT_SYMBOL(strstr); > + > +#undef strnstr > +char *strnstr(const char *s1, const char *s2, size_t len) > +{ > + size_t l1 = min(__strlen(s1) + 1, len); > + size_t l2 = __strlen(s2) + 1; > + > + check_memory_region((unsigned long)s1, l1, false, _RET_IP_); > + check_memory_region((unsigned long)s2, l2, false, _RET_IP_); > + > + return __strnstr(s1, s2, len); > +} > +EXPORT_SYMBOL(strnstr); > + > +#undef strlen > +size_t strlen(const char *s) > +{ > + size_t len = __strlen(s); > + > + check_memory_region((unsigned long)s, len + 1, false, _RET_IP_); > + > + return len; > +} > +EXPORT_SYMBOL(strlen); > + > +#undef strnlen > +size_t strnlen(const char *s, size_t count) > +{ > + size_t len = __strnlen(s, count); > + > + check_memory_region((unsigned long)s, min(len + 1, count), false, _RET_IP_); > + > + return len; > +} > +EXPORT_SYMBOL(strnlen); > + > +#undef strpbrk > +char *strpbrk(const char *cs, const char *ct) > +{ > + size_t ls = __strlen(cs) + 1; > + size_t lt = __strlen(ct) + 1; > + > + check_memory_region((unsigned long)cs, ls, false, _RET_IP_); > + check_memory_region((unsigned long)ct, lt, false, _RET_IP_); > + > + return __strpbrk(cs, ct); > +} > +EXPORT_SYMBOL(strpbrk); > + > +#undef strsep > +char *strsep(char **s, const char *ct) > +{ > + char *cs = *s; > + > + check_memory_region((unsigned long)s, sizeof(*s), true, _RET_IP_); > + > + if (cs) { > + int ls = __strlen(cs) + 1; > + int lt = __strlen(ct) + 1; > + > + check_memory_region((unsigned long)cs, ls, false, _RET_IP_); > + check_memory_region((unsigned long)ct, lt, false, _RET_IP_); > + } > + > + return __strsep(s, ct); > +} > +EXPORT_SYMBOL(strsep); > + > +#undef strspn > +size_t strspn(const char *s, const char *accept) > +{ > + size_t ls = __strlen(s) + 1; > + size_t la = __strlen(accept) + 1; > + > + check_memory_region((unsigned long)s, ls, false, _RET_IP_); > + check_memory_region((unsigned long)accept, la, false, _RET_IP_); > + > + return __strspn(s, accept); > +} > +EXPORT_SYMBOL(strspn); > + > +#undef strcspn > +size_t strcspn(const char *s, const char *reject) > +{ > + size_t ls = __strlen(s) + 1; > + size_t lr = __strlen(reject) + 1; > + > + check_memory_region((unsigned long)s, ls, false, _RET_IP_); > + check_memory_region((unsigned long)reject, lr, false, _RET_IP_); > + > + return __strcspn(s, reject); > +} > +EXPORT_SYMBOL(strcspn); > + > +#undef memscan > +void *memscan(void *addr, int c, size_t size) > +{ > + check_memory_region((unsigned long)addr, size, false, _RET_IP_); > + > + return __memscan(addr, c, size); > +} > +EXPORT_SYMBOL(memscan); > + > +#undef memcmp > +int memcmp(const void *cs, const void *ct, size_t count) > +{ > + check_memory_region((unsigned long)cs, count, false, _RET_IP_); > + check_memory_region((unsigned long)ct, count, false, _RET_IP_); > + > + return __memcmp(cs, ct, count); > +} > +EXPORT_SYMBOL(memcmp); > + > +#undef memchr > +void *memchr(const void *s, int c, size_t n) > +{ > + check_memory_region((unsigned long)s, n, false, _RET_IP_); > + > + return __memchr(s, c, n); > +} > +EXPORT_SYMBOL(memchr); > + > +#undef memchr_inv > +void *memchr_inv(const void *start, int c, size_t bytes) > +{ > + check_memory_region((unsigned long)start, bytes, false, _RET_IP_); > + > + return __memchr_inv(start, c, bytes); > +} > +EXPORT_SYMBOL(memchr_inv); > + > +#undef strreplace > +char *strreplace(char *s, char old, char new) > +{ > + size_t len = __strlen(s) + 1; > + > + check_memory_region((unsigned long)s, len, true, _RET_IP_); > + > + return __strreplace(s, old, new); > +} > +EXPORT_SYMBOL(strreplace); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 3/3] kasan: add interceptors for all string functions 2019-04-02 9:43 ` [RFC PATCH v2 3/3] kasan: add interceptors for all string functions Christophe Leroy @ 2019-04-02 12:58 ` Dmitry Vyukov 2019-04-02 14:57 ` Christophe Leroy 2019-04-02 16:14 ` Andrey Ryabinin 1 sibling, 1 reply; 5+ messages in thread From: Dmitry Vyukov @ 2019-04-02 12:58 UTC (permalink / raw) To: Christophe Leroy Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin, Alexander Potapenko, Daniel Axtens, Linux-MM, linuxppc-dev, LKML, kasan-dev On Tue, Apr 2, 2019 at 11:43 AM Christophe Leroy <christophe.leroy@c-s.fr> wrote: > > Hi Dmitry, Andrey and others, > > Do you have any comments to this series ? > > I'd like to know if this approach is ok or if it is better to keep doing > as in https://patchwork.ozlabs.org/patch/1055788/ Hi Christophe, Forking every kernel function does not look like a scalable approach to me. There is not much special about str* functions. There is something a bit special about memset/memcpy as compiler emits them for struct set/copy. Could powerpc do the same as x86 and map some shadow early enough (before "prom")? Then we would not need anything of this? Sorry if we already discussed this, I am losing context quickly. > Thanks > Christophe > > Le 28/03/2019 à 16:00, Christophe Leroy a écrit : > > In the same spirit as commit 393f203f5fd5 ("x86_64: kasan: add > > interceptors for memset/memmove/memcpy functions"), this patch > > adds interceptors for string manipulation functions so that we > > can compile lib/string.o without kasan support hence allow the > > string functions to also be used from places where kasan has > > to be disabled. > > > > Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> > > --- > > v2: Fixed a few checkpatch stuff and added missing EXPORT_SYMBOL() and missing #undefs > > > > include/linux/string.h | 79 ++++++++++ > > lib/Makefile | 2 + > > lib/string.c | 8 + > > mm/kasan/string.c | 394 +++++++++++++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 483 insertions(+) > > > > diff --git a/include/linux/string.h b/include/linux/string.h > > index 7927b875f80c..3d2aff2ed402 100644 > > --- a/include/linux/string.h > > +++ b/include/linux/string.h > > @@ -19,54 +19,117 @@ extern void *memdup_user_nul(const void __user *, size_t); > > */ > > #include <asm/string.h> > > > > +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__) > > +/* > > + * For files that are not instrumented (e.g. mm/slub.c) we > > + * should use not instrumented version of mem* functions. > > + */ > > +#define memset16 __memset16 > > +#define memset32 __memset32 > > +#define memset64 __memset64 > > +#define memzero_explicit __memzero_explicit > > +#define strcpy __strcpy > > +#define strncpy __strncpy > > +#define strlcpy __strlcpy > > +#define strscpy __strscpy > > +#define strcat __strcat > > +#define strncat __strncat > > +#define strlcat __strlcat > > +#define strcmp __strcmp > > +#define strncmp __strncmp > > +#define strcasecmp __strcasecmp > > +#define strncasecmp __strncasecmp > > +#define strchr __strchr > > +#define strchrnul __strchrnul > > +#define strrchr __strrchr > > +#define strnchr __strnchr > > +#define skip_spaces __skip_spaces > > +#define strim __strim > > +#define strstr __strstr > > +#define strnstr __strnstr > > +#define strlen __strlen > > +#define strnlen __strnlen > > +#define strpbrk __strpbrk > > +#define strsep __strsep > > +#define strspn __strspn > > +#define strcspn __strcspn > > +#define memscan __memscan > > +#define memcmp __memcmp > > +#define memchr __memchr > > +#define memchr_inv __memchr_inv > > +#define strreplace __strreplace > > + > > +#ifndef __NO_FORTIFY > > +#define __NO_FORTIFY /* FORTIFY_SOURCE uses __builtin_memcpy, etc. */ > > +#endif > > + > > +#endif > > + > > #ifndef __HAVE_ARCH_STRCPY > > extern char * strcpy(char *,const char *); > > +char *__strcpy(char *, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRNCPY > > extern char * strncpy(char *,const char *, __kernel_size_t); > > +char *__strncpy(char *, const char *, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_STRLCPY > > size_t strlcpy(char *, const char *, size_t); > > +size_t __strlcpy(char *, const char *, size_t); > > #endif > > #ifndef __HAVE_ARCH_STRSCPY > > ssize_t strscpy(char *, const char *, size_t); > > +ssize_t __strscpy(char *, const char *, size_t); > > #endif > > #ifndef __HAVE_ARCH_STRCAT > > extern char * strcat(char *, const char *); > > +char *__strcat(char *, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRNCAT > > extern char * strncat(char *, const char *, __kernel_size_t); > > +char *__strncat(char *, const char *, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_STRLCAT > > extern size_t strlcat(char *, const char *, __kernel_size_t); > > +size_t __strlcat(char *, const char *, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_STRCMP > > extern int strcmp(const char *,const char *); > > +int __strcmp(const char *, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRNCMP > > extern int strncmp(const char *,const char *,__kernel_size_t); > > +int __strncmp(const char *, const char *, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_STRCASECMP > > extern int strcasecmp(const char *s1, const char *s2); > > +int __strcasecmp(const char *s1, const char *s2); > > #endif > > #ifndef __HAVE_ARCH_STRNCASECMP > > extern int strncasecmp(const char *s1, const char *s2, size_t n); > > +int __strncasecmp(const char *s1, const char *s2, size_t n); > > #endif > > #ifndef __HAVE_ARCH_STRCHR > > extern char * strchr(const char *,int); > > +char *__strchr(const char *, int); > > #endif > > #ifndef __HAVE_ARCH_STRCHRNUL > > extern char * strchrnul(const char *,int); > > +char *__strchrnul(const char *, int); > > #endif > > #ifndef __HAVE_ARCH_STRNCHR > > extern char * strnchr(const char *, size_t, int); > > +char *__strnchr(const char *, size_t, int); > > #endif > > #ifndef __HAVE_ARCH_STRRCHR > > extern char * strrchr(const char *,int); > > +char *__strrchr(const char *, int); > > #endif > > extern char * __must_check skip_spaces(const char *); > > +char * __must_check __skip_spaces(const char *); > > > > extern char *strim(char *); > > +char *__strim(char *); > > > > static inline __must_check char *strstrip(char *str) > > { > > @@ -75,27 +138,35 @@ static inline __must_check char *strstrip(char *str) > > > > #ifndef __HAVE_ARCH_STRSTR > > extern char * strstr(const char *, const char *); > > +char *__strstr(const char *, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRNSTR > > extern char * strnstr(const char *, const char *, size_t); > > +char *__strnstr(const char *, const char *, size_t); > > #endif > > #ifndef __HAVE_ARCH_STRLEN > > extern __kernel_size_t strlen(const char *); > > +__kernel_size_t __strlen(const char *); > > #endif > > #ifndef __HAVE_ARCH_STRNLEN > > extern __kernel_size_t strnlen(const char *,__kernel_size_t); > > +__kernel_size_t __strnlen(const char *, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_STRPBRK > > extern char * strpbrk(const char *,const char *); > > +char *__strpbrk(const char *, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRSEP > > extern char * strsep(char **,const char *); > > +char *__strsep(char **, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRSPN > > extern __kernel_size_t strspn(const char *,const char *); > > +__kernel_size_t __strspn(const char *, const char *); > > #endif > > #ifndef __HAVE_ARCH_STRCSPN > > extern __kernel_size_t strcspn(const char *,const char *); > > +__kernel_size_t __strcspn(const char *, const char *); > > #endif > > > > #ifndef __HAVE_ARCH_MEMSET > > @@ -104,14 +175,17 @@ extern void * memset(void *,int,__kernel_size_t); > > > > #ifndef __HAVE_ARCH_MEMSET16 > > extern void *memset16(uint16_t *, uint16_t, __kernel_size_t); > > +void *__memset16(uint16_t *, uint16_t, __kernel_size_t); > > #endif > > > > #ifndef __HAVE_ARCH_MEMSET32 > > extern void *memset32(uint32_t *, uint32_t, __kernel_size_t); > > +void *__memset32(uint32_t *, uint32_t, __kernel_size_t); > > #endif > > > > #ifndef __HAVE_ARCH_MEMSET64 > > extern void *memset64(uint64_t *, uint64_t, __kernel_size_t); > > +void *__memset64(uint64_t *, uint64_t, __kernel_size_t); > > #endif > > > > static inline void *memset_l(unsigned long *p, unsigned long v, > > @@ -146,12 +220,15 @@ extern void * memmove(void *,const void *,__kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_MEMSCAN > > extern void * memscan(void *,int,__kernel_size_t); > > +void *__memscan(void *, int, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_MEMCMP > > extern int memcmp(const void *,const void *,__kernel_size_t); > > +int __memcmp(const void *, const void *, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_MEMCHR > > extern void * memchr(const void *,int,__kernel_size_t); > > +void *__memchr(const void *, int, __kernel_size_t); > > #endif > > #ifndef __HAVE_ARCH_MEMCPY_MCSAFE > > static inline __must_check unsigned long memcpy_mcsafe(void *dst, > > @@ -168,7 +245,9 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt) > > } > > #endif > > void *memchr_inv(const void *s, int c, size_t n); > > +void *__memchr_inv(const void *s, int c, size_t n); > > char *strreplace(char *s, char old, char new); > > +char *__strreplace(char *s, char old, char new); > > > > extern void kfree_const(const void *x); > > > > diff --git a/lib/Makefile b/lib/Makefile > > index 30b9b0bfbba9..19d0237f9b9c 100644 > > --- a/lib/Makefile > > +++ b/lib/Makefile > > @@ -18,6 +18,8 @@ KCOV_INSTRUMENT_list_debug.o := n > > KCOV_INSTRUMENT_debugobjects.o := n > > KCOV_INSTRUMENT_dynamic_debug.o := n > > > > +KASAN_SANITIZE_string.o := n > > + > > lib-y := ctype.o string.o string_sysfs.o vsprintf.o cmdline.o \ > > rbtree.o radix-tree.o timerqueue.o xarray.o \ > > idr.o int_sqrt.o extable.o \ > > diff --git a/lib/string.c b/lib/string.c > > index f3886c5175ac..31a253201bba 100644 > > --- a/lib/string.c > > +++ b/lib/string.c > > @@ -85,7 +85,9 @@ EXPORT_SYMBOL(strcasecmp); > > * @dest: Where to copy the string to > > * @src: Where to copy the string from > > */ > > +#ifndef CONFIG_KASAN > > #undef strcpy > > +#endif > > char *strcpy(char *dest, const char *src) > > { > > char *tmp = dest; > > @@ -243,7 +245,9 @@ EXPORT_SYMBOL(strscpy); > > * @dest: The string to be appended to > > * @src: The string to append to it > > */ > > +#ifndef CONFIG_KASAN > > #undef strcat > > +#endif > > char *strcat(char *dest, const char *src) > > { > > char *tmp = dest; > > @@ -319,7 +323,9 @@ EXPORT_SYMBOL(strlcat); > > * @cs: One string > > * @ct: Another string > > */ > > +#ifndef CONFIG_KASAN > > #undef strcmp > > +#endif > > int strcmp(const char *cs, const char *ct) > > { > > unsigned char c1, c2; > > @@ -773,7 +779,9 @@ EXPORT_SYMBOL(memmove); > > * @ct: Another area of memory > > * @count: The size of the area. > > */ > > +#ifndef CONFIG_KASAN > > #undef memcmp > > +#endif > > __visible int memcmp(const void *cs, const void *ct, size_t count) > > { > > const unsigned char *su1, *su2; > > diff --git a/mm/kasan/string.c b/mm/kasan/string.c > > index 083b967255a2..0db31bbbf643 100644 > > --- a/mm/kasan/string.c > > +++ b/mm/kasan/string.c > > @@ -35,6 +35,42 @@ void *memset(void *addr, int c, size_t len) > > return __memset(addr, c, len); > > } > > > > +#undef memset16 > > +void *memset16(uint16_t *s, uint16_t v, size_t count) > > +{ > > + check_memory_region((unsigned long)s, count << 1, true, _RET_IP_); > > + > > + return __memset16(s, v, count); > > +} > > +EXPORT_SYMBOL(memset16); > > + > > +#undef memset32 > > +void *memset32(uint32_t *s, uint32_t v, size_t count) > > +{ > > + check_memory_region((unsigned long)s, count << 2, true, _RET_IP_); > > + > > + return __memset32(s, v, count); > > +} > > +EXPORT_SYMBOL(memset32); > > + > > +#undef memset64 > > +void *memset64(uint64_t *s, uint64_t v, size_t count) > > +{ > > + check_memory_region((unsigned long)s, count << 3, true, _RET_IP_); > > + > > + return __memset64(s, v, count); > > +} > > +EXPORT_SYMBOL(memset64); > > + > > +#undef memzero_explicit > > +void memzero_explicit(void *s, size_t count) > > +{ > > + check_memory_region((unsigned long)s, count, true, _RET_IP_); > > + > > + return __memzero_explicit(s, count); > > +} > > +EXPORT_SYMBOL(memzero_explicit); > > + > > #undef memmove > > void *memmove(void *dest, const void *src, size_t len) > > { > > @@ -52,3 +88,361 @@ void *memcpy(void *dest, const void *src, size_t len) > > > > return __memcpy(dest, src, len); > > } > > + > > +#undef strcpy > > +char *strcpy(char *dest, const char *src) > > +{ > > + size_t len = __strlen(src) + 1; > > + > > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, len, true, _RET_IP_); > > + > > + return __strcpy(dest, src); > > +} > > +EXPORT_SYMBOL(strcpy); > > + > > +#undef strncpy > > +char *strncpy(char *dest, const char *src, size_t count) > > +{ > > + size_t len = min(__strlen(src) + 1, count); > > + > > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, count, true, _RET_IP_); > > + > > + return __strncpy(dest, src, count); > > +} > > +EXPORT_SYMBOL(strncpy); > > + > > +#undef strlcpy > > +size_t strlcpy(char *dest, const char *src, size_t size) > > +{ > > + size_t len = __strlen(src) + 1; > > + > > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, min(len, size), true, _RET_IP_); > > + > > + return __strlcpy(dest, src, size); > > +} > > +EXPORT_SYMBOL(strlcpy); > > + > > +#undef strscpy > > +ssize_t strscpy(char *dest, const char *src, size_t count) > > +{ > > + int len = min(__strlen(src) + 1, count); > > + > > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, len, true, _RET_IP_); > > + > > + return __strscpy(dest, src, count); > > +} > > +EXPORT_SYMBOL(strscpy); > > + > > +#undef strcat > > +char *strcat(char *dest, const char *src) > > +{ > > + size_t slen = __strlen(src) + 1; > > + size_t dlen = __strlen(dest); > > + > > + check_memory_region((unsigned long)src, slen, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); > > + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); > > + > > + return __strcat(dest, src); > > +} > > +EXPORT_SYMBOL(strcat); > > + > > +#undef strncat > > +char *strncat(char *dest, const char *src, size_t count) > > +{ > > + size_t slen = min(__strlen(src) + 1, count); > > + size_t dlen = __strlen(dest); > > + > > + check_memory_region((unsigned long)src, slen, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); > > + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); > > + > > + return __strncat(dest, src, count); > > +} > > +EXPORT_SYMBOL(strncat); > > + > > +#undef strlcat > > +size_t strlcat(char *dest, const char *src, size_t count) > > +{ > > + size_t slen = min(__strlen(src) + 1, count); > > + size_t dlen = __strlen(dest); > > + > > + check_memory_region((unsigned long)src, slen, false, _RET_IP_); > > + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); > > + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); > > + > > + return __strlcat(dest, src, count); > > +} > > +EXPORT_SYMBOL(strlcat); > > + > > +#undef strcmp > > +int strcmp(const char *cs, const char *ct) > > +{ > > + size_t len = min(__strlen(cs) + 1, __strlen(ct) + 1); > > + > > + check_memory_region((unsigned long)cs, len, false, _RET_IP_); > > + check_memory_region((unsigned long)ct, len, false, _RET_IP_); > > + > > + return __strcmp(cs, ct); > > +} > > +EXPORT_SYMBOL(strcmp); > > + > > +#undef strncmp > > +int strncmp(const char *cs, const char *ct, size_t count) > > +{ > > + size_t len = min3(__strlen(cs) + 1, __strlen(ct) + 1, count); > > + > > + check_memory_region((unsigned long)cs, len, false, _RET_IP_); > > + check_memory_region((unsigned long)ct, len, false, _RET_IP_); > > + > > + return __strncmp(cs, ct, count); > > +} > > +EXPORT_SYMBOL(strncmp); > > + > > +#undef strcasecmp > > +int strcasecmp(const char *s1, const char *s2) > > +{ > > + size_t len = min(__strlen(s1) + 1, __strlen(s2) + 1); > > + > > + check_memory_region((unsigned long)s1, len, false, _RET_IP_); > > + check_memory_region((unsigned long)s2, len, false, _RET_IP_); > > + > > + return __strcasecmp(s1, s2); > > +} > > +EXPORT_SYMBOL(strcasecmp); > > + > > +#undef strncasecmp > > +int strncasecmp(const char *s1, const char *s2, size_t len) > > +{ > > + size_t sz = min3(__strlen(s1) + 1, __strlen(s2) + 1, len); > > + > > + check_memory_region((unsigned long)s1, sz, false, _RET_IP_); > > + check_memory_region((unsigned long)s2, sz, false, _RET_IP_); > > + > > + return __strncasecmp(s1, s2, len); > > +} > > +EXPORT_SYMBOL(strncasecmp); > > + > > +#undef strchr > > +char *strchr(const char *s, int c) > > +{ > > + size_t len = __strlen(s) + 1; > > + > > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > > + > > + return __strchr(s, c); > > +} > > +EXPORT_SYMBOL(strchr); > > + > > +#undef strchrnul > > +char *strchrnul(const char *s, int c) > > +{ > > + size_t len = __strlen(s) + 1; > > + > > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > > + > > + return __strchrnul(s, c); > > +} > > +EXPORT_SYMBOL(strchrnul); > > + > > +#undef strrchr > > +char *strrchr(const char *s, int c) > > +{ > > + size_t len = __strlen(s) + 1; > > + > > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > > + > > + return __strrchr(s, c); > > +} > > +EXPORT_SYMBOL(strrchr); > > + > > +#undef strnchr > > +char *strnchr(const char *s, size_t count, int c) > > +{ > > + size_t len = __strlen(s) + 1; > > + > > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > > + > > + return __strnchr(s, count, c); > > +} > > +EXPORT_SYMBOL(strnchr); > > + > > +#undef skip_spaces > > +char *skip_spaces(const char *str) > > +{ > > + size_t len = __strlen(str) + 1; > > + > > + check_memory_region((unsigned long)str, len, false, _RET_IP_); > > + > > + return __skip_spaces(str); > > +} > > +EXPORT_SYMBOL(skip_spaces); > > + > > +#undef strim > > +char *strim(char *s) > > +{ > > + size_t len = __strlen(s) + 1; > > + > > + check_memory_region((unsigned long)s, len, false, _RET_IP_); > > + > > + return __strim(s); > > +} > > +EXPORT_SYMBOL(strim); > > + > > +#undef strstr > > +char *strstr(const char *s1, const char *s2) > > +{ > > + size_t l1 = __strlen(s1) + 1; > > + size_t l2 = __strlen(s2) + 1; > > + > > + check_memory_region((unsigned long)s1, l1, false, _RET_IP_); > > + check_memory_region((unsigned long)s2, l2, false, _RET_IP_); > > + > > + return __strstr(s1, s2); > > +} > > +EXPORT_SYMBOL(strstr); > > + > > +#undef strnstr > > +char *strnstr(const char *s1, const char *s2, size_t len) > > +{ > > + size_t l1 = min(__strlen(s1) + 1, len); > > + size_t l2 = __strlen(s2) + 1; > > + > > + check_memory_region((unsigned long)s1, l1, false, _RET_IP_); > > + check_memory_region((unsigned long)s2, l2, false, _RET_IP_); > > + > > + return __strnstr(s1, s2, len); > > +} > > +EXPORT_SYMBOL(strnstr); > > + > > +#undef strlen > > +size_t strlen(const char *s) > > +{ > > + size_t len = __strlen(s); > > + > > + check_memory_region((unsigned long)s, len + 1, false, _RET_IP_); > > + > > + return len; > > +} > > +EXPORT_SYMBOL(strlen); > > + > > +#undef strnlen > > +size_t strnlen(const char *s, size_t count) > > +{ > > + size_t len = __strnlen(s, count); > > + > > + check_memory_region((unsigned long)s, min(len + 1, count), false, _RET_IP_); > > + > > + return len; > > +} > > +EXPORT_SYMBOL(strnlen); > > + > > +#undef strpbrk > > +char *strpbrk(const char *cs, const char *ct) > > +{ > > + size_t ls = __strlen(cs) + 1; > > + size_t lt = __strlen(ct) + 1; > > + > > + check_memory_region((unsigned long)cs, ls, false, _RET_IP_); > > + check_memory_region((unsigned long)ct, lt, false, _RET_IP_); > > + > > + return __strpbrk(cs, ct); > > +} > > +EXPORT_SYMBOL(strpbrk); > > + > > +#undef strsep > > +char *strsep(char **s, const char *ct) > > +{ > > + char *cs = *s; > > + > > + check_memory_region((unsigned long)s, sizeof(*s), true, _RET_IP_); > > + > > + if (cs) { > > + int ls = __strlen(cs) + 1; > > + int lt = __strlen(ct) + 1; > > + > > + check_memory_region((unsigned long)cs, ls, false, _RET_IP_); > > + check_memory_region((unsigned long)ct, lt, false, _RET_IP_); > > + } > > + > > + return __strsep(s, ct); > > +} > > +EXPORT_SYMBOL(strsep); > > + > > +#undef strspn > > +size_t strspn(const char *s, const char *accept) > > +{ > > + size_t ls = __strlen(s) + 1; > > + size_t la = __strlen(accept) + 1; > > + > > + check_memory_region((unsigned long)s, ls, false, _RET_IP_); > > + check_memory_region((unsigned long)accept, la, false, _RET_IP_); > > + > > + return __strspn(s, accept); > > +} > > +EXPORT_SYMBOL(strspn); > > + > > +#undef strcspn > > +size_t strcspn(const char *s, const char *reject) > > +{ > > + size_t ls = __strlen(s) + 1; > > + size_t lr = __strlen(reject) + 1; > > + > > + check_memory_region((unsigned long)s, ls, false, _RET_IP_); > > + check_memory_region((unsigned long)reject, lr, false, _RET_IP_); > > + > > + return __strcspn(s, reject); > > +} > > +EXPORT_SYMBOL(strcspn); > > + > > +#undef memscan > > +void *memscan(void *addr, int c, size_t size) > > +{ > > + check_memory_region((unsigned long)addr, size, false, _RET_IP_); > > + > > + return __memscan(addr, c, size); > > +} > > +EXPORT_SYMBOL(memscan); > > + > > +#undef memcmp > > +int memcmp(const void *cs, const void *ct, size_t count) > > +{ > > + check_memory_region((unsigned long)cs, count, false, _RET_IP_); > > + check_memory_region((unsigned long)ct, count, false, _RET_IP_); > > + > > + return __memcmp(cs, ct, count); > > +} > > +EXPORT_SYMBOL(memcmp); > > + > > +#undef memchr > > +void *memchr(const void *s, int c, size_t n) > > +{ > > + check_memory_region((unsigned long)s, n, false, _RET_IP_); > > + > > + return __memchr(s, c, n); > > +} > > +EXPORT_SYMBOL(memchr); > > + > > +#undef memchr_inv > > +void *memchr_inv(const void *start, int c, size_t bytes) > > +{ > > + check_memory_region((unsigned long)start, bytes, false, _RET_IP_); > > + > > + return __memchr_inv(start, c, bytes); > > +} > > +EXPORT_SYMBOL(memchr_inv); > > + > > +#undef strreplace > > +char *strreplace(char *s, char old, char new) > > +{ > > + size_t len = __strlen(s) + 1; > > + > > + check_memory_region((unsigned long)s, len, true, _RET_IP_); > > + > > + return __strreplace(s, old, new); > > +} > > +EXPORT_SYMBOL(strreplace); > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 3/3] kasan: add interceptors for all string functions 2019-04-02 12:58 ` Dmitry Vyukov @ 2019-04-02 14:57 ` Christophe Leroy 0 siblings, 0 replies; 5+ messages in thread From: Christophe Leroy @ 2019-04-02 14:57 UTC (permalink / raw) To: Dmitry Vyukov Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V, Andrey Ryabinin, Alexander Potapenko, Daniel Axtens, Linux-MM, linuxppc-dev, LKML, kasan-dev Le 02/04/2019 à 14:58, Dmitry Vyukov a écrit : > On Tue, Apr 2, 2019 at 11:43 AM Christophe Leroy > <christophe.leroy@c-s.fr> wrote: >> >> Hi Dmitry, Andrey and others, >> >> Do you have any comments to this series ? >> >> I'd like to know if this approach is ok or if it is better to keep doing >> as in https://patchwork.ozlabs.org/patch/1055788/ > > Hi Christophe, > > Forking every kernel function does not look like a scalable approach > to me. There is not much special about str* functions. There is > something a bit special about memset/memcpy as compiler emits them for > struct set/copy. > Could powerpc do the same as x86 and map some shadow early enough > (before "prom")? Then we would not need anything of this? Sorry if we > already discussed this, I am losing context quickly. Hi Dmitry, I'm afraid we can't map shadow ram that early. This code gets run by third party BIOS SW which manages the MMU and provides a 1:1 mapping, so there is no way we can map shadow memory. If you feel providing interceptors for the string functions is not a good idea, I'm ok with it, I'll keep the necessary string functions in prom_init.c I was proposing the interceptor's approach because behind the specific need for handling early prom_init code, I thought it was also a way to limit KASAN performance impact on string functions, and it was also a way to handle all the optimised string functions provided by architectures. In my series I have a patch that disables powerpc's optimised string functions (https://patchwork.ozlabs.org/patch/1055780/). The interceptor's approach was a way to avoid that. As far as I can see, at the time being the other arches don't disable their optimised string functions, meaning the KASAN checks are skipped. Thanks Christophe > > > > >> Thanks >> Christophe >> >> Le 28/03/2019 à 16:00, Christophe Leroy a écrit : >>> In the same spirit as commit 393f203f5fd5 ("x86_64: kasan: add >>> interceptors for memset/memmove/memcpy functions"), this patch >>> adds interceptors for string manipulation functions so that we >>> can compile lib/string.o without kasan support hence allow the >>> string functions to also be used from places where kasan has >>> to be disabled. >>> >>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr> >>> --- >>> v2: Fixed a few checkpatch stuff and added missing EXPORT_SYMBOL() and missing #undefs >>> >>> include/linux/string.h | 79 ++++++++++ >>> lib/Makefile | 2 + >>> lib/string.c | 8 + >>> mm/kasan/string.c | 394 +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 4 files changed, 483 insertions(+) >>> >>> diff --git a/include/linux/string.h b/include/linux/string.h >>> index 7927b875f80c..3d2aff2ed402 100644 >>> --- a/include/linux/string.h >>> +++ b/include/linux/string.h >>> @@ -19,54 +19,117 @@ extern void *memdup_user_nul(const void __user *, size_t); >>> */ >>> #include <asm/string.h> >>> >>> +#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__) >>> +/* >>> + * For files that are not instrumented (e.g. mm/slub.c) we >>> + * should use not instrumented version of mem* functions. >>> + */ >>> +#define memset16 __memset16 >>> +#define memset32 __memset32 >>> +#define memset64 __memset64 >>> +#define memzero_explicit __memzero_explicit >>> +#define strcpy __strcpy >>> +#define strncpy __strncpy >>> +#define strlcpy __strlcpy >>> +#define strscpy __strscpy >>> +#define strcat __strcat >>> +#define strncat __strncat >>> +#define strlcat __strlcat >>> +#define strcmp __strcmp >>> +#define strncmp __strncmp >>> +#define strcasecmp __strcasecmp >>> +#define strncasecmp __strncasecmp >>> +#define strchr __strchr >>> +#define strchrnul __strchrnul >>> +#define strrchr __strrchr >>> +#define strnchr __strnchr >>> +#define skip_spaces __skip_spaces >>> +#define strim __strim >>> +#define strstr __strstr >>> +#define strnstr __strnstr >>> +#define strlen __strlen >>> +#define strnlen __strnlen >>> +#define strpbrk __strpbrk >>> +#define strsep __strsep >>> +#define strspn __strspn >>> +#define strcspn __strcspn >>> +#define memscan __memscan >>> +#define memcmp __memcmp >>> +#define memchr __memchr >>> +#define memchr_inv __memchr_inv >>> +#define strreplace __strreplace >>> + >>> +#ifndef __NO_FORTIFY >>> +#define __NO_FORTIFY /* FORTIFY_SOURCE uses __builtin_memcpy, etc. */ >>> +#endif >>> + >>> +#endif >>> + >>> #ifndef __HAVE_ARCH_STRCPY >>> extern char * strcpy(char *,const char *); >>> +char *__strcpy(char *, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRNCPY >>> extern char * strncpy(char *,const char *, __kernel_size_t); >>> +char *__strncpy(char *, const char *, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRLCPY >>> size_t strlcpy(char *, const char *, size_t); >>> +size_t __strlcpy(char *, const char *, size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRSCPY >>> ssize_t strscpy(char *, const char *, size_t); >>> +ssize_t __strscpy(char *, const char *, size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRCAT >>> extern char * strcat(char *, const char *); >>> +char *__strcat(char *, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRNCAT >>> extern char * strncat(char *, const char *, __kernel_size_t); >>> +char *__strncat(char *, const char *, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRLCAT >>> extern size_t strlcat(char *, const char *, __kernel_size_t); >>> +size_t __strlcat(char *, const char *, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRCMP >>> extern int strcmp(const char *,const char *); >>> +int __strcmp(const char *, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRNCMP >>> extern int strncmp(const char *,const char *,__kernel_size_t); >>> +int __strncmp(const char *, const char *, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRCASECMP >>> extern int strcasecmp(const char *s1, const char *s2); >>> +int __strcasecmp(const char *s1, const char *s2); >>> #endif >>> #ifndef __HAVE_ARCH_STRNCASECMP >>> extern int strncasecmp(const char *s1, const char *s2, size_t n); >>> +int __strncasecmp(const char *s1, const char *s2, size_t n); >>> #endif >>> #ifndef __HAVE_ARCH_STRCHR >>> extern char * strchr(const char *,int); >>> +char *__strchr(const char *, int); >>> #endif >>> #ifndef __HAVE_ARCH_STRCHRNUL >>> extern char * strchrnul(const char *,int); >>> +char *__strchrnul(const char *, int); >>> #endif >>> #ifndef __HAVE_ARCH_STRNCHR >>> extern char * strnchr(const char *, size_t, int); >>> +char *__strnchr(const char *, size_t, int); >>> #endif >>> #ifndef __HAVE_ARCH_STRRCHR >>> extern char * strrchr(const char *,int); >>> +char *__strrchr(const char *, int); >>> #endif >>> extern char * __must_check skip_spaces(const char *); >>> +char * __must_check __skip_spaces(const char *); >>> >>> extern char *strim(char *); >>> +char *__strim(char *); >>> >>> static inline __must_check char *strstrip(char *str) >>> { >>> @@ -75,27 +138,35 @@ static inline __must_check char *strstrip(char *str) >>> >>> #ifndef __HAVE_ARCH_STRSTR >>> extern char * strstr(const char *, const char *); >>> +char *__strstr(const char *, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRNSTR >>> extern char * strnstr(const char *, const char *, size_t); >>> +char *__strnstr(const char *, const char *, size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRLEN >>> extern __kernel_size_t strlen(const char *); >>> +__kernel_size_t __strlen(const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRNLEN >>> extern __kernel_size_t strnlen(const char *,__kernel_size_t); >>> +__kernel_size_t __strnlen(const char *, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_STRPBRK >>> extern char * strpbrk(const char *,const char *); >>> +char *__strpbrk(const char *, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRSEP >>> extern char * strsep(char **,const char *); >>> +char *__strsep(char **, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRSPN >>> extern __kernel_size_t strspn(const char *,const char *); >>> +__kernel_size_t __strspn(const char *, const char *); >>> #endif >>> #ifndef __HAVE_ARCH_STRCSPN >>> extern __kernel_size_t strcspn(const char *,const char *); >>> +__kernel_size_t __strcspn(const char *, const char *); >>> #endif >>> >>> #ifndef __HAVE_ARCH_MEMSET >>> @@ -104,14 +175,17 @@ extern void * memset(void *,int,__kernel_size_t); >>> >>> #ifndef __HAVE_ARCH_MEMSET16 >>> extern void *memset16(uint16_t *, uint16_t, __kernel_size_t); >>> +void *__memset16(uint16_t *, uint16_t, __kernel_size_t); >>> #endif >>> >>> #ifndef __HAVE_ARCH_MEMSET32 >>> extern void *memset32(uint32_t *, uint32_t, __kernel_size_t); >>> +void *__memset32(uint32_t *, uint32_t, __kernel_size_t); >>> #endif >>> >>> #ifndef __HAVE_ARCH_MEMSET64 >>> extern void *memset64(uint64_t *, uint64_t, __kernel_size_t); >>> +void *__memset64(uint64_t *, uint64_t, __kernel_size_t); >>> #endif >>> >>> static inline void *memset_l(unsigned long *p, unsigned long v, >>> @@ -146,12 +220,15 @@ extern void * memmove(void *,const void *,__kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_MEMSCAN >>> extern void * memscan(void *,int,__kernel_size_t); >>> +void *__memscan(void *, int, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_MEMCMP >>> extern int memcmp(const void *,const void *,__kernel_size_t); >>> +int __memcmp(const void *, const void *, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_MEMCHR >>> extern void * memchr(const void *,int,__kernel_size_t); >>> +void *__memchr(const void *, int, __kernel_size_t); >>> #endif >>> #ifndef __HAVE_ARCH_MEMCPY_MCSAFE >>> static inline __must_check unsigned long memcpy_mcsafe(void *dst, >>> @@ -168,7 +245,9 @@ static inline void memcpy_flushcache(void *dst, const void *src, size_t cnt) >>> } >>> #endif >>> void *memchr_inv(const void *s, int c, size_t n); >>> +void *__memchr_inv(const void *s, int c, size_t n); >>> char *strreplace(char *s, char old, char new); >>> +char *__strreplace(char *s, char old, char new); >>> >>> extern void kfree_const(const void *x); >>> >>> diff --git a/lib/Makefile b/lib/Makefile >>> index 30b9b0bfbba9..19d0237f9b9c 100644 >>> --- a/lib/Makefile >>> +++ b/lib/Makefile >>> @@ -18,6 +18,8 @@ KCOV_INSTRUMENT_list_debug.o := n >>> KCOV_INSTRUMENT_debugobjects.o := n >>> KCOV_INSTRUMENT_dynamic_debug.o := n >>> >>> +KASAN_SANITIZE_string.o := n >>> + >>> lib-y := ctype.o string.o string_sysfs.o vsprintf.o cmdline.o \ >>> rbtree.o radix-tree.o timerqueue.o xarray.o \ >>> idr.o int_sqrt.o extable.o \ >>> diff --git a/lib/string.c b/lib/string.c >>> index f3886c5175ac..31a253201bba 100644 >>> --- a/lib/string.c >>> +++ b/lib/string.c >>> @@ -85,7 +85,9 @@ EXPORT_SYMBOL(strcasecmp); >>> * @dest: Where to copy the string to >>> * @src: Where to copy the string from >>> */ >>> +#ifndef CONFIG_KASAN >>> #undef strcpy >>> +#endif >>> char *strcpy(char *dest, const char *src) >>> { >>> char *tmp = dest; >>> @@ -243,7 +245,9 @@ EXPORT_SYMBOL(strscpy); >>> * @dest: The string to be appended to >>> * @src: The string to append to it >>> */ >>> +#ifndef CONFIG_KASAN >>> #undef strcat >>> +#endif >>> char *strcat(char *dest, const char *src) >>> { >>> char *tmp = dest; >>> @@ -319,7 +323,9 @@ EXPORT_SYMBOL(strlcat); >>> * @cs: One string >>> * @ct: Another string >>> */ >>> +#ifndef CONFIG_KASAN >>> #undef strcmp >>> +#endif >>> int strcmp(const char *cs, const char *ct) >>> { >>> unsigned char c1, c2; >>> @@ -773,7 +779,9 @@ EXPORT_SYMBOL(memmove); >>> * @ct: Another area of memory >>> * @count: The size of the area. >>> */ >>> +#ifndef CONFIG_KASAN >>> #undef memcmp >>> +#endif >>> __visible int memcmp(const void *cs, const void *ct, size_t count) >>> { >>> const unsigned char *su1, *su2; >>> diff --git a/mm/kasan/string.c b/mm/kasan/string.c >>> index 083b967255a2..0db31bbbf643 100644 >>> --- a/mm/kasan/string.c >>> +++ b/mm/kasan/string.c >>> @@ -35,6 +35,42 @@ void *memset(void *addr, int c, size_t len) >>> return __memset(addr, c, len); >>> } >>> >>> +#undef memset16 >>> +void *memset16(uint16_t *s, uint16_t v, size_t count) >>> +{ >>> + check_memory_region((unsigned long)s, count << 1, true, _RET_IP_); >>> + >>> + return __memset16(s, v, count); >>> +} >>> +EXPORT_SYMBOL(memset16); >>> + >>> +#undef memset32 >>> +void *memset32(uint32_t *s, uint32_t v, size_t count) >>> +{ >>> + check_memory_region((unsigned long)s, count << 2, true, _RET_IP_); >>> + >>> + return __memset32(s, v, count); >>> +} >>> +EXPORT_SYMBOL(memset32); >>> + >>> +#undef memset64 >>> +void *memset64(uint64_t *s, uint64_t v, size_t count) >>> +{ >>> + check_memory_region((unsigned long)s, count << 3, true, _RET_IP_); >>> + >>> + return __memset64(s, v, count); >>> +} >>> +EXPORT_SYMBOL(memset64); >>> + >>> +#undef memzero_explicit >>> +void memzero_explicit(void *s, size_t count) >>> +{ >>> + check_memory_region((unsigned long)s, count, true, _RET_IP_); >>> + >>> + return __memzero_explicit(s, count); >>> +} >>> +EXPORT_SYMBOL(memzero_explicit); >>> + >>> #undef memmove >>> void *memmove(void *dest, const void *src, size_t len) >>> { >>> @@ -52,3 +88,361 @@ void *memcpy(void *dest, const void *src, size_t len) >>> >>> return __memcpy(dest, src, len); >>> } >>> + >>> +#undef strcpy >>> +char *strcpy(char *dest, const char *src) >>> +{ >>> + size_t len = __strlen(src) + 1; >>> + >>> + check_memory_region((unsigned long)src, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, len, true, _RET_IP_); >>> + >>> + return __strcpy(dest, src); >>> +} >>> +EXPORT_SYMBOL(strcpy); >>> + >>> +#undef strncpy >>> +char *strncpy(char *dest, const char *src, size_t count) >>> +{ >>> + size_t len = min(__strlen(src) + 1, count); >>> + >>> + check_memory_region((unsigned long)src, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, count, true, _RET_IP_); >>> + >>> + return __strncpy(dest, src, count); >>> +} >>> +EXPORT_SYMBOL(strncpy); >>> + >>> +#undef strlcpy >>> +size_t strlcpy(char *dest, const char *src, size_t size) >>> +{ >>> + size_t len = __strlen(src) + 1; >>> + >>> + check_memory_region((unsigned long)src, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, min(len, size), true, _RET_IP_); >>> + >>> + return __strlcpy(dest, src, size); >>> +} >>> +EXPORT_SYMBOL(strlcpy); >>> + >>> +#undef strscpy >>> +ssize_t strscpy(char *dest, const char *src, size_t count) >>> +{ >>> + int len = min(__strlen(src) + 1, count); >>> + >>> + check_memory_region((unsigned long)src, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, len, true, _RET_IP_); >>> + >>> + return __strscpy(dest, src, count); >>> +} >>> +EXPORT_SYMBOL(strscpy); >>> + >>> +#undef strcat >>> +char *strcat(char *dest, const char *src) >>> +{ >>> + size_t slen = __strlen(src) + 1; >>> + size_t dlen = __strlen(dest); >>> + >>> + check_memory_region((unsigned long)src, slen, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); >>> + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); >>> + >>> + return __strcat(dest, src); >>> +} >>> +EXPORT_SYMBOL(strcat); >>> + >>> +#undef strncat >>> +char *strncat(char *dest, const char *src, size_t count) >>> +{ >>> + size_t slen = min(__strlen(src) + 1, count); >>> + size_t dlen = __strlen(dest); >>> + >>> + check_memory_region((unsigned long)src, slen, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); >>> + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); >>> + >>> + return __strncat(dest, src, count); >>> +} >>> +EXPORT_SYMBOL(strncat); >>> + >>> +#undef strlcat >>> +size_t strlcat(char *dest, const char *src, size_t count) >>> +{ >>> + size_t slen = min(__strlen(src) + 1, count); >>> + size_t dlen = __strlen(dest); >>> + >>> + check_memory_region((unsigned long)src, slen, false, _RET_IP_); >>> + check_memory_region((unsigned long)dest, dlen, false, _RET_IP_); >>> + check_memory_region((unsigned long)(dest + dlen), slen, true, _RET_IP_); >>> + >>> + return __strlcat(dest, src, count); >>> +} >>> +EXPORT_SYMBOL(strlcat); >>> + >>> +#undef strcmp >>> +int strcmp(const char *cs, const char *ct) >>> +{ >>> + size_t len = min(__strlen(cs) + 1, __strlen(ct) + 1); >>> + >>> + check_memory_region((unsigned long)cs, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)ct, len, false, _RET_IP_); >>> + >>> + return __strcmp(cs, ct); >>> +} >>> +EXPORT_SYMBOL(strcmp); >>> + >>> +#undef strncmp >>> +int strncmp(const char *cs, const char *ct, size_t count) >>> +{ >>> + size_t len = min3(__strlen(cs) + 1, __strlen(ct) + 1, count); >>> + >>> + check_memory_region((unsigned long)cs, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)ct, len, false, _RET_IP_); >>> + >>> + return __strncmp(cs, ct, count); >>> +} >>> +EXPORT_SYMBOL(strncmp); >>> + >>> +#undef strcasecmp >>> +int strcasecmp(const char *s1, const char *s2) >>> +{ >>> + size_t len = min(__strlen(s1) + 1, __strlen(s2) + 1); >>> + >>> + check_memory_region((unsigned long)s1, len, false, _RET_IP_); >>> + check_memory_region((unsigned long)s2, len, false, _RET_IP_); >>> + >>> + return __strcasecmp(s1, s2); >>> +} >>> +EXPORT_SYMBOL(strcasecmp); >>> + >>> +#undef strncasecmp >>> +int strncasecmp(const char *s1, const char *s2, size_t len) >>> +{ >>> + size_t sz = min3(__strlen(s1) + 1, __strlen(s2) + 1, len); >>> + >>> + check_memory_region((unsigned long)s1, sz, false, _RET_IP_); >>> + check_memory_region((unsigned long)s2, sz, false, _RET_IP_); >>> + >>> + return __strncasecmp(s1, s2, len); >>> +} >>> +EXPORT_SYMBOL(strncasecmp); >>> + >>> +#undef strchr >>> +char *strchr(const char *s, int c) >>> +{ >>> + size_t len = __strlen(s) + 1; >>> + >>> + check_memory_region((unsigned long)s, len, false, _RET_IP_); >>> + >>> + return __strchr(s, c); >>> +} >>> +EXPORT_SYMBOL(strchr); >>> + >>> +#undef strchrnul >>> +char *strchrnul(const char *s, int c) >>> +{ >>> + size_t len = __strlen(s) + 1; >>> + >>> + check_memory_region((unsigned long)s, len, false, _RET_IP_); >>> + >>> + return __strchrnul(s, c); >>> +} >>> +EXPORT_SYMBOL(strchrnul); >>> + >>> +#undef strrchr >>> +char *strrchr(const char *s, int c) >>> +{ >>> + size_t len = __strlen(s) + 1; >>> + >>> + check_memory_region((unsigned long)s, len, false, _RET_IP_); >>> + >>> + return __strrchr(s, c); >>> +} >>> +EXPORT_SYMBOL(strrchr); >>> + >>> +#undef strnchr >>> +char *strnchr(const char *s, size_t count, int c) >>> +{ >>> + size_t len = __strlen(s) + 1; >>> + >>> + check_memory_region((unsigned long)s, len, false, _RET_IP_); >>> + >>> + return __strnchr(s, count, c); >>> +} >>> +EXPORT_SYMBOL(strnchr); >>> + >>> +#undef skip_spaces >>> +char *skip_spaces(const char *str) >>> +{ >>> + size_t len = __strlen(str) + 1; >>> + >>> + check_memory_region((unsigned long)str, len, false, _RET_IP_); >>> + >>> + return __skip_spaces(str); >>> +} >>> +EXPORT_SYMBOL(skip_spaces); >>> + >>> +#undef strim >>> +char *strim(char *s) >>> +{ >>> + size_t len = __strlen(s) + 1; >>> + >>> + check_memory_region((unsigned long)s, len, false, _RET_IP_); >>> + >>> + return __strim(s); >>> +} >>> +EXPORT_SYMBOL(strim); >>> + >>> +#undef strstr >>> +char *strstr(const char *s1, const char *s2) >>> +{ >>> + size_t l1 = __strlen(s1) + 1; >>> + size_t l2 = __strlen(s2) + 1; >>> + >>> + check_memory_region((unsigned long)s1, l1, false, _RET_IP_); >>> + check_memory_region((unsigned long)s2, l2, false, _RET_IP_); >>> + >>> + return __strstr(s1, s2); >>> +} >>> +EXPORT_SYMBOL(strstr); >>> + >>> +#undef strnstr >>> +char *strnstr(const char *s1, const char *s2, size_t len) >>> +{ >>> + size_t l1 = min(__strlen(s1) + 1, len); >>> + size_t l2 = __strlen(s2) + 1; >>> + >>> + check_memory_region((unsigned long)s1, l1, false, _RET_IP_); >>> + check_memory_region((unsigned long)s2, l2, false, _RET_IP_); >>> + >>> + return __strnstr(s1, s2, len); >>> +} >>> +EXPORT_SYMBOL(strnstr); >>> + >>> +#undef strlen >>> +size_t strlen(const char *s) >>> +{ >>> + size_t len = __strlen(s); >>> + >>> + check_memory_region((unsigned long)s, len + 1, false, _RET_IP_); >>> + >>> + return len; >>> +} >>> +EXPORT_SYMBOL(strlen); >>> + >>> +#undef strnlen >>> +size_t strnlen(const char *s, size_t count) >>> +{ >>> + size_t len = __strnlen(s, count); >>> + >>> + check_memory_region((unsigned long)s, min(len + 1, count), false, _RET_IP_); >>> + >>> + return len; >>> +} >>> +EXPORT_SYMBOL(strnlen); >>> + >>> +#undef strpbrk >>> +char *strpbrk(const char *cs, const char *ct) >>> +{ >>> + size_t ls = __strlen(cs) + 1; >>> + size_t lt = __strlen(ct) + 1; >>> + >>> + check_memory_region((unsigned long)cs, ls, false, _RET_IP_); >>> + check_memory_region((unsigned long)ct, lt, false, _RET_IP_); >>> + >>> + return __strpbrk(cs, ct); >>> +} >>> +EXPORT_SYMBOL(strpbrk); >>> + >>> +#undef strsep >>> +char *strsep(char **s, const char *ct) >>> +{ >>> + char *cs = *s; >>> + >>> + check_memory_region((unsigned long)s, sizeof(*s), true, _RET_IP_); >>> + >>> + if (cs) { >>> + int ls = __strlen(cs) + 1; >>> + int lt = __strlen(ct) + 1; >>> + >>> + check_memory_region((unsigned long)cs, ls, false, _RET_IP_); >>> + check_memory_region((unsigned long)ct, lt, false, _RET_IP_); >>> + } >>> + >>> + return __strsep(s, ct); >>> +} >>> +EXPORT_SYMBOL(strsep); >>> + >>> +#undef strspn >>> +size_t strspn(const char *s, const char *accept) >>> +{ >>> + size_t ls = __strlen(s) + 1; >>> + size_t la = __strlen(accept) + 1; >>> + >>> + check_memory_region((unsigned long)s, ls, false, _RET_IP_); >>> + check_memory_region((unsigned long)accept, la, false, _RET_IP_); >>> + >>> + return __strspn(s, accept); >>> +} >>> +EXPORT_SYMBOL(strspn); >>> + >>> +#undef strcspn >>> +size_t strcspn(const char *s, const char *reject) >>> +{ >>> + size_t ls = __strlen(s) + 1; >>> + size_t lr = __strlen(reject) + 1; >>> + >>> + check_memory_region((unsigned long)s, ls, false, _RET_IP_); >>> + check_memory_region((unsigned long)reject, lr, false, _RET_IP_); >>> + >>> + return __strcspn(s, reject); >>> +} >>> +EXPORT_SYMBOL(strcspn); >>> + >>> +#undef memscan >>> +void *memscan(void *addr, int c, size_t size) >>> +{ >>> + check_memory_region((unsigned long)addr, size, false, _RET_IP_); >>> + >>> + return __memscan(addr, c, size); >>> +} >>> +EXPORT_SYMBOL(memscan); >>> + >>> +#undef memcmp >>> +int memcmp(const void *cs, const void *ct, size_t count) >>> +{ >>> + check_memory_region((unsigned long)cs, count, false, _RET_IP_); >>> + check_memory_region((unsigned long)ct, count, false, _RET_IP_); >>> + >>> + return __memcmp(cs, ct, count); >>> +} >>> +EXPORT_SYMBOL(memcmp); >>> + >>> +#undef memchr >>> +void *memchr(const void *s, int c, size_t n) >>> +{ >>> + check_memory_region((unsigned long)s, n, false, _RET_IP_); >>> + >>> + return __memchr(s, c, n); >>> +} >>> +EXPORT_SYMBOL(memchr); >>> + >>> +#undef memchr_inv >>> +void *memchr_inv(const void *start, int c, size_t bytes) >>> +{ >>> + check_memory_region((unsigned long)start, bytes, false, _RET_IP_); >>> + >>> + return __memchr_inv(start, c, bytes); >>> +} >>> +EXPORT_SYMBOL(memchr_inv); >>> + >>> +#undef strreplace >>> +char *strreplace(char *s, char old, char new) >>> +{ >>> + size_t len = __strlen(s) + 1; >>> + >>> + check_memory_region((unsigned long)s, len, true, _RET_IP_); >>> + >>> + return __strreplace(s, old, new); >>> +} >>> +EXPORT_SYMBOL(strreplace); >>> ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 3/3] kasan: add interceptors for all string functions 2019-04-02 9:43 ` [RFC PATCH v2 3/3] kasan: add interceptors for all string functions Christophe Leroy 2019-04-02 12:58 ` Dmitry Vyukov @ 2019-04-02 16:14 ` Andrey Ryabinin 2019-04-02 20:36 ` Christophe Leroy 1 sibling, 1 reply; 5+ messages in thread From: Andrey Ryabinin @ 2019-04-02 16:14 UTC (permalink / raw) To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V, Alexander Potapenko, Dmitry Vyukov, Daniel Axtens Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev On 4/2/19 12:43 PM, Christophe Leroy wrote: > Hi Dmitry, Andrey and others, > > Do you have any comments to this series ? > I don't see justification for adding all these non-instrumented functions. We need only some subset of these functions and only on powerpc so far. Arches that don't use str*() that early simply doesn't need not-instrumented __str*() variant. Also I don't think that auto-replace str* to __str* for all not instrumented files is a good idea, as this will reduce KASAN coverage. E.g. we don't instrument slub.c but there is no reason to use non-instrumented __str*() functions there. And finally, this series make bug reporting slightly worse. E.g. let's look at strcpy(): +char *strcpy(char *dest, const char *src) +{ + size_t len = __strlen(src) + 1; + + check_memory_region((unsigned long)src, len, false, _RET_IP_); + check_memory_region((unsigned long)dest, len, true, _RET_IP_); + + return __strcpy(dest, src); +} If src is not-null terminated string we might not see proper out-of-bounds report from KASAN only a crash in __strlen(). Which might make harder to identify where 'src' comes from, where it was allocated and what's the size of allocated area. > I'd like to know if this approach is ok or if it is better to keep doing as in https://patchwork.ozlabs.org/patch/1055788/ > I think the patch from link is a better solution to the problem. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [RFC PATCH v2 3/3] kasan: add interceptors for all string functions 2019-04-02 16:14 ` Andrey Ryabinin @ 2019-04-02 20:36 ` Christophe Leroy 0 siblings, 0 replies; 5+ messages in thread From: Christophe Leroy @ 2019-04-02 20:36 UTC (permalink / raw) To: Andrey Ryabinin, Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, Nicholas Piggin, Aneesh Kumar K.V, Alexander Potapenko, Dmitry Vyukov, Daniel Axtens Cc: linux-mm, linuxppc-dev, linux-kernel, kasan-dev Le 02/04/2019 à 18:14, Andrey Ryabinin a écrit : > > > On 4/2/19 12:43 PM, Christophe Leroy wrote: >> Hi Dmitry, Andrey and others, >> >> Do you have any comments to this series ? >> > > I don't see justification for adding all these non-instrumented functions. We need only some subset of these functions > and only on powerpc so far. Arches that don't use str*() that early simply doesn't need not-instrumented __str*() variant. > > Also I don't think that auto-replace str* to __str* for all not instrumented files is a good idea, as this will reduce KASAN coverage. > E.g. we don't instrument slub.c but there is no reason to use non-instrumented __str*() functions there. Ok, I didn't see it that way. In fact I was seeing the opposite and was considering it as an opportunity to increase KASAN coverage. E.g.: at the time being things like the above (from arch/xtensa/include/asm/string.h) are not covered at all I believe: #define __HAVE_ARCH_STRCPY static inline char *strcpy(char *__dest, const char *__src) { register char *__xdest = __dest; unsigned long __dummy; __asm__ __volatile__("1:\n\t" "l8ui %2, %1, 0\n\t" "s8i %2, %0, 0\n\t" "addi %1, %1, 1\n\t" "addi %0, %0, 1\n\t" "bnez %2, 1b\n\t" : "=r" (__dest), "=r" (__src), "=&r" (__dummy) : "0" (__dest), "1" (__src) : "memory"); return __xdest; } In my series, I have deactivated optimised string functions when KASAN is selected like arm64 do. See https://patchwork.ozlabs.org/patch/1055780/ But not every arch does that, meaning that some string functions remains not instrumented at all. Also, I was seeing it as a way to reduce impact on performance with KASAN. Because instrumenting each byte access of the non-optimised string functions is a performance genocide. > > And finally, this series make bug reporting slightly worse. E.g. let's look at strcpy(): > > +char *strcpy(char *dest, const char *src) > +{ > + size_t len = __strlen(src) + 1; > + > + check_memory_region((unsigned long)src, len, false, _RET_IP_); > + check_memory_region((unsigned long)dest, len, true, _RET_IP_); > + > + return __strcpy(dest, src); > +} > > If src is not-null terminated string we might not see proper out-of-bounds report from KASAN only a crash in __strlen(). > Which might make harder to identify where 'src' comes from, where it was allocated and what's the size of allocated area. > > >> I'd like to know if this approach is ok or if it is better to keep doing as in https://patchwork.ozlabs.org/patch/1055788/ >> > I think the patch from link is a better solution to the problem. > Ok, I'll stick with it then. Thanks for your feedback Christophe ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-02 20:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <f13944c4e99ec2cef6d93d762e6b526e0335877f.1553785019.git.christophe.leroy@c-s.fr>
[not found] ` <51a6d9d7185de310f37ccbd7e4ebfdd6c7e9791f.1553785020.git.christophe.leroy@c-s.fr>
2019-04-02 9:43 ` [RFC PATCH v2 3/3] kasan: add interceptors for all string functions Christophe Leroy
2019-04-02 12:58 ` Dmitry Vyukov
2019-04-02 14:57 ` Christophe Leroy
2019-04-02 16:14 ` Andrey Ryabinin
2019-04-02 20:36 ` Christophe Leroy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).