Hello Andrey,

Thanks for your review.

On 2018-09-03 ?? 6:40, Andrey Ryabinin wrote:

On 08/23/2018 11:56 AM, Kyeongdon Kim wrote:

quoted

diff --git a/mm/kasan/kasan.c b/mm/kasan/kasan.c
index c3bd520..61ad7f1 100644
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c

@@ -304,6 +304,29 @@ void *memcpy(void *dest, const void *src, 

size_t len)

quoted

return __memcpy(dest, src, len);
}
+#ifdef CONFIG_ARM64
+/*
+ * Arch arm64 use assembly variant for strcmp/strncmp,
+ * xtensa use inline asm operations and x86_64 use c one,
+ * so now this interceptors only for arm64 kasan.
+ */
+#undef strcmp
+int strcmp(const char *cs, const char *ct)
+{
+ check_memory_region((unsigned long)cs, 1, false, _RET_IP_);
+ check_memory_region((unsigned long)ct, 1, false, _RET_IP_);
+

Well this is definitely wrong. strcmp() often accesses far more than 
one byte.

quoted

+ return __strcmp(cs, ct);
+}
+#undef strncmp
+int strncmp(const char *cs, const char *ct, size_t len)
+{
+ check_memory_region((unsigned long)cs, len, false, _RET_IP_);
+ check_memory_region((unsigned long)ct, len, false, _RET_IP_);

This will cause false positives. Both 'cs', and 'ct' could be less 
than len bytes.

There is no need in these interceptors, just use the C implementations 
from lib/string.c
like you did in your first patch.
The only thing that was wrong in the first patch is that assembly 
implementations
were compiled out instead of being declared week.

Well, at first I thought so..
I would remove diff code in /mm/kasan/kasan.c then use C implementations 
in lib/string.c
w/ assem implementations as weak :

diff --git a/lib/string.c b/lib/string.c
index 2c0900a..a18b18f 100644
--- a/lib/string.c
+++ b/lib/string.c

@@ -312,7 +312,7 @@ size_t strlcat(char *dest, const char *src, size_t 

count)
 ?EXPORT_SYMBOL(strlcat);
 ?#endif

-#ifndef __HAVE_ARCH_STRCMP
+#if (defined(CONFIG_ARM64) && defined(CONFIG_KASAN)) || 
!defined(__HAVE_ARCH_STRCMP)
 ?/**
 ? * strcmp - Compare two strings
 ? * @cs: One string

@@ -336,7 +336,7 @@ int strcmp(const char *cs, const char *ct)
 ?EXPORT_SYMBOL(strcmp);
 ?#endif

-#ifndef __HAVE_ARCH_STRNCMP
+#if (defined(CONFIG_ARM64) && defined(CONFIG_KASAN)) || 

!defined(__HAVE_ARCH_STRNCMP)
 ?/**
 ? * strncmp - Compare two length-limited strings

Can I get your opinion wrt this ?

Thanks,