From: Arnd Bergmann <[email protected]>
Calling memcmp() and memchr() with an intentional buffer overflow
is now caught at compile time:
In function 'memcmp',
inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
263 | __read_overflow();
| ^~~~~~~~~~~~~~~~~
In function 'memchr',
inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
277 | __read_overflow();
| ^~~~~~~~~~~~~~~~~
Change the kasan tests to wrap those inside of a noinline function
to prevent the compiler from noticing the bug and let kasan find
it at runtime.
Signed-off-by: Arnd Bergmann <[email protected]>
---
lib/test_kasan.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/lib/test_kasan.c b/lib/test_kasan.c
index 67ed689a0b1b..903215e944f1 100644
--- a/lib/test_kasan.c
+++ b/lib/test_kasan.c
@@ -852,6 +852,21 @@ static void kmem_cache_invalid_free(struct kunit *test)
kmem_cache_destroy(cache);
}
+/*
+ * noinline wrappers to prevent the compiler from noticing the overflow
+ * at compile time rather than having kasan catch it.
+ * */
+static noinline void *__kasan_memchr(const void *s, int c, size_t n)
+{
+ return memchr(s, c, n);
+}
+
+static noinline int __kasan_memcmp(const void *s1, const void *s2, size_t n)
+{
+ return memcmp(s1, s2, n);
+}
+
+
static void kasan_memchr(struct kunit *test)
{
char *ptr;
@@ -870,7 +885,7 @@ static void kasan_memchr(struct kunit *test)
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
KUNIT_EXPECT_KASAN_FAIL(test,
- kasan_ptr_result = memchr(ptr, '1', size + 1));
+ kasan_ptr_result = __kasan_memchr(ptr, '1', size + 1));
kfree(ptr);
}
@@ -895,7 +910,7 @@ static void kasan_memcmp(struct kunit *test)
memset(arr, 0, sizeof(arr));
KUNIT_EXPECT_KASAN_FAIL(test,
- kasan_int_result = memcmp(ptr, arr, size+1));
+ kasan_int_result = __kasan_memcmp(ptr, arr, size+1));
kfree(ptr);
}
--
2.29.2
From: Arnd Bergmann <[email protected]>
GCC has separate macros for -fsanitize=kernel-address and
-fsanitize=kernel-hwaddress, and the check in the arm64 string.h
gets this wrong, which leads to string functions not getting
fortified with gcc. The newly added tests find this:
warning: unsafe memchr() usage lacked '__read_overflow' warning in /git/arm-soc/lib/test_fortify/read_overflow-memchr.c
warning: unsafe memchr_inv() usage lacked '__read_overflow' symbol in /git/arm-soc/lib/test_fortify/read_overflow-memchr_inv.c
warning: unsafe memcmp() usage lacked '__read_overflow' warning in /git/arm-soc/lib/test_fortify/read_overflow-memcmp.c
warning: unsafe memscan() usage lacked '__read_overflow' symbol in /git/arm-soc/lib/test_fortify/read_overflow-memscan.c
warning: unsafe memcmp() usage lacked '__read_overflow2' warning in /git/arm-soc/lib/test_fortify/read_overflow2-memcmp.c
warning: unsafe memcpy() usage lacked '__read_overflow2' symbol in /git/arm-soc/lib/test_fortify/read_overflow2-memcpy.c
warning: unsafe memmove() usage lacked '__read_overflow2' symbol in /git/arm-soc/lib/test_fortify/read_overflow2-memmove.c
warning: unsafe memcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memcpy.c
warning: unsafe memmove() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memmove.c
warning: unsafe memset() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memset.c
warning: unsafe strcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strcpy-lit.c
warning: unsafe strcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strcpy.c
warning: unsafe strlcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strlcpy-src.c
warning: unsafe strlcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strlcpy.c
warning: unsafe strncpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strncpy-src.c
warning: unsafe strncpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strncpy.c
warning: unsafe strscpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strscpy.c
Add a workaround to include/linux/compiler_types.h so we always
define __SANITIZE_ADDRESS__ for either mode, as we already do
for clang.
Signed-off-by: Arnd Bergmann <[email protected]>
---
include/linux/compiler_types.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index aad6f6408bfa..2f2776fffefe 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -178,6 +178,13 @@ struct ftrace_likely_data {
*/
#define noinline_for_stack noinline
+/*
+ * Treat __SANITIZE_HWADDRESS__ the same as __SANITIZE_ADDRESS__ in the kernel
+ */
+#ifdef __SANITIZE_HWADDRESS__
+#define __SANITIZE_ADDRESS__
+#endif
+
/*
* Sanitizer helper attributes: Because using __always_inline and
* __no_sanitize_* conflict, provide helper attributes that will either expand
--
2.29.2
On 10/13/21 5:00 PM, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> Calling memcmp() and memchr() with an intentional buffer overflow
> is now caught at compile time:
>
> In function 'memcmp',
> inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
> include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> 263 | __read_overflow();
> | ^~~~~~~~~~~~~~~~~
> In function 'memchr',
> inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
> include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> 277 | __read_overflow();
> | ^~~~~~~~~~~~~~~~~
>
> Change the kasan tests to wrap those inside of a noinline function
> to prevent the compiler from noticing the bug and let kasan find
> it at runtime.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
Reviewed-by: Vincenzo Frascino <[email protected]>
> ---
> lib/test_kasan.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 67ed689a0b1b..903215e944f1 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -852,6 +852,21 @@ static void kmem_cache_invalid_free(struct kunit *test)
> kmem_cache_destroy(cache);
> }
>
> +/*
> + * noinline wrappers to prevent the compiler from noticing the overflow
> + * at compile time rather than having kasan catch it.
> + * */
> +static noinline void *__kasan_memchr(const void *s, int c, size_t n)
> +{
> + return memchr(s, c, n);
> +}
> +
> +static noinline int __kasan_memcmp(const void *s1, const void *s2, size_t n)
> +{
> + return memcmp(s1, s2, n);
> +}
> +
> +
> static void kasan_memchr(struct kunit *test)
> {
> char *ptr;
> @@ -870,7 +885,7 @@ static void kasan_memchr(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> KUNIT_EXPECT_KASAN_FAIL(test,
> - kasan_ptr_result = memchr(ptr, '1', size + 1));
> + kasan_ptr_result = __kasan_memchr(ptr, '1', size + 1));
>
> kfree(ptr);
> }
> @@ -895,7 +910,7 @@ static void kasan_memcmp(struct kunit *test)
> memset(arr, 0, sizeof(arr));
>
> KUNIT_EXPECT_KASAN_FAIL(test,
> - kasan_int_result = memcmp(ptr, arr, size+1));
> + kasan_int_result = __kasan_memcmp(ptr, arr, size+1));
> kfree(ptr);
> }
>
>
--
Regards,
Vincenzo
On October 14, 2021 1:12:54 AM PDT, Vincenzo Frascino <[email protected]> wrote:
>
>
>On 10/13/21 5:00 PM, Arnd Bergmann wrote:
>> From: Arnd Bergmann <[email protected]>
>>
>> Calling memcmp() and memchr() with an intentional buffer overflow
>> is now caught at compile time:
>>
>> In function 'memcmp',
>> inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
>> include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
>> 263 | __read_overflow();
>> | ^~~~~~~~~~~~~~~~~
>> In function 'memchr',
>> inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
>> include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
>> 277 | __read_overflow();
>> | ^~~~~~~~~~~~~~~~~
>>
>> Change the kasan tests to wrap those inside of a noinline function
>> to prevent the compiler from noticing the bug and let kasan find
>> it at runtime.
>>
>> Signed-off-by: Arnd Bergmann <[email protected]>
>
>Reviewed-by: Vincenzo Frascino <[email protected]>
How about just explicitly making the size invisible to the compiler?
I did this for similar issues in the same source:
https://lore.kernel.org/linux-hardening/[email protected]/T/#u
-Kees
>
>> ---
>> lib/test_kasan.c | 19 +++++++++++++++++--
>> 1 file changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
>> index 67ed689a0b1b..903215e944f1 100644
>> --- a/lib/test_kasan.c
>> +++ b/lib/test_kasan.c
>> @@ -852,6 +852,21 @@ static void kmem_cache_invalid_free(struct kunit *test)
>> kmem_cache_destroy(cache);
>> }
>>
>> +/*
>> + * noinline wrappers to prevent the compiler from noticing the overflow
>> + * at compile time rather than having kasan catch it.
>> + * */
>> +static noinline void *__kasan_memchr(const void *s, int c, size_t n)
>> +{
>> + return memchr(s, c, n);
>> +}
>> +
>> +static noinline int __kasan_memcmp(const void *s1, const void *s2, size_t n)
>> +{
>> + return memcmp(s1, s2, n);
>> +}
>> +
>> +
>> static void kasan_memchr(struct kunit *test)
>> {
>> char *ptr;
>> @@ -870,7 +885,7 @@ static void kasan_memchr(struct kunit *test)
>> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>>
>> KUNIT_EXPECT_KASAN_FAIL(test,
>> - kasan_ptr_result = memchr(ptr, '1', size + 1));
>> + kasan_ptr_result = __kasan_memchr(ptr, '1', size + 1));
>>
>> kfree(ptr);
>> }
>> @@ -895,7 +910,7 @@ static void kasan_memcmp(struct kunit *test)
>> memset(arr, 0, sizeof(arr));
>>
>> KUNIT_EXPECT_KASAN_FAIL(test,
>> - kasan_int_result = memcmp(ptr, arr, size+1));
>> + kasan_int_result = __kasan_memcmp(ptr, arr, size+1));
>> kfree(ptr);
>> }
>>
>>
>
--
Kees Cook
On Wed, Oct 13, 2021 at 05:00:05PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> Calling memcmp() and memchr() with an intentional buffer overflow
> is now caught at compile time:
>
> In function 'memcmp',
> inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
> include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> 263 | __read_overflow();
> | ^~~~~~~~~~~~~~~~~
> In function 'memchr',
> inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
> include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> 277 | __read_overflow();
> | ^~~~~~~~~~~~~~~~~
>
> Change the kasan tests to wrap those inside of a noinline function
> to prevent the compiler from noticing the bug and let kasan find
> it at runtime.
Is this with W=1 ? I had explicitly disabled the read overflows for
"phase 1" of the overflow restriction tightening...
(And what do you think of using OPTIMIZER_HIDE_VAR() instead[1]?
-Kees
[1] https://lore.kernel.org/linux-hardening/[email protected]/T/#u
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> lib/test_kasan.c | 19 +++++++++++++++++--
> 1 file changed, 17 insertions(+), 2 deletions(-)
>
> diff --git a/lib/test_kasan.c b/lib/test_kasan.c
> index 67ed689a0b1b..903215e944f1 100644
> --- a/lib/test_kasan.c
> +++ b/lib/test_kasan.c
> @@ -852,6 +852,21 @@ static void kmem_cache_invalid_free(struct kunit *test)
> kmem_cache_destroy(cache);
> }
>
> +/*
> + * noinline wrappers to prevent the compiler from noticing the overflow
> + * at compile time rather than having kasan catch it.
> + * */
> +static noinline void *__kasan_memchr(const void *s, int c, size_t n)
> +{
> + return memchr(s, c, n);
> +}
> +
> +static noinline int __kasan_memcmp(const void *s1, const void *s2, size_t n)
> +{
> + return memcmp(s1, s2, n);
> +}
> +
> +
> static void kasan_memchr(struct kunit *test)
> {
> char *ptr;
> @@ -870,7 +885,7 @@ static void kasan_memchr(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ptr);
>
> KUNIT_EXPECT_KASAN_FAIL(test,
> - kasan_ptr_result = memchr(ptr, '1', size + 1));
> + kasan_ptr_result = __kasan_memchr(ptr, '1', size + 1));
>
> kfree(ptr);
> }
> @@ -895,7 +910,7 @@ static void kasan_memcmp(struct kunit *test)
> memset(arr, 0, sizeof(arr));
>
> KUNIT_EXPECT_KASAN_FAIL(test,
> - kasan_int_result = memcmp(ptr, arr, size+1));
> + kasan_int_result = __kasan_memcmp(ptr, arr, size+1));
> kfree(ptr);
> }
>
> --
> 2.29.2
>
--
Kees Cook
On Mon, Oct 18, 2021 at 9:47 PM Kees Cook <[email protected]> wrote:
> On Wed, Oct 13, 2021 at 05:00:05PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > Calling memcmp() and memchr() with an intentional buffer overflow
> > is now caught at compile time:
> >
> > In function 'memcmp',
> > inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
> > include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> > 263 | __read_overflow();
> > | ^~~~~~~~~~~~~~~~~
> > In function 'memchr',
> > inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
> > include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> > 277 | __read_overflow();
> > | ^~~~~~~~~~~~~~~~~
> >
> > Change the kasan tests to wrap those inside of a noinline function
> > to prevent the compiler from noticing the bug and let kasan find
> > it at runtime.
>
> Is this with W=1 ? I had explicitly disabled the read overflows for
> "phase 1" of the overflow restriction tightening...
I have a somewhat modified source tree that builds cleanly with W=1 after
disabling all the noisy ones, so this is probably one that I would not have
seen without it.
> (And what do you think of using OPTIMIZER_HIDE_VAR() instead[1]?
>
> [1] https://lore.kernel.org/linux-hardening/[email protected]/T/#u
Yes, that is probably better. I can try updating the patch tomorrow,
unless you do it first.
Arnd
On Wed, Oct 13, 2021 at 05:00:06PM +0200, Arnd Bergmann wrote:
> From: Arnd Bergmann <[email protected]>
>
> GCC has separate macros for -fsanitize=kernel-address and
> -fsanitize=kernel-hwaddress, and the check in the arm64 string.h
> gets this wrong, which leads to string functions not getting
> fortified with gcc. The newly added tests find this:
>
> warning: unsafe memchr() usage lacked '__read_overflow' warning in /git/arm-soc/lib/test_fortify/read_overflow-memchr.c
> warning: unsafe memchr_inv() usage lacked '__read_overflow' symbol in /git/arm-soc/lib/test_fortify/read_overflow-memchr_inv.c
> warning: unsafe memcmp() usage lacked '__read_overflow' warning in /git/arm-soc/lib/test_fortify/read_overflow-memcmp.c
> warning: unsafe memscan() usage lacked '__read_overflow' symbol in /git/arm-soc/lib/test_fortify/read_overflow-memscan.c
> warning: unsafe memcmp() usage lacked '__read_overflow2' warning in /git/arm-soc/lib/test_fortify/read_overflow2-memcmp.c
> warning: unsafe memcpy() usage lacked '__read_overflow2' symbol in /git/arm-soc/lib/test_fortify/read_overflow2-memcpy.c
> warning: unsafe memmove() usage lacked '__read_overflow2' symbol in /git/arm-soc/lib/test_fortify/read_overflow2-memmove.c
> warning: unsafe memcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memcpy.c
> warning: unsafe memmove() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memmove.c
> warning: unsafe memset() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memset.c
> warning: unsafe strcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strcpy-lit.c
> warning: unsafe strcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strcpy.c
> warning: unsafe strlcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strlcpy-src.c
> warning: unsafe strlcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strlcpy.c
> warning: unsafe strncpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strncpy-src.c
> warning: unsafe strncpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strncpy.c
> warning: unsafe strscpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strscpy.c
>
What is the build config that trips these warnings?
In trying to understand this, I see in arch/arm64/include/asm/string.h:
#if (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)) && \
!defined(__SANITIZE_ADDRESS__)
other architectures (like arm32) do:
#if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
so it's okay because it's not getting touched by the hwaddress sanitizer?
e.g. I see:
config CC_HAS_KASAN_GENERIC
def_bool $(cc-option, -fsanitize=kernel-address)
config CC_HAS_KASAN_SW_TAGS
def_bool $(cc-option, -fsanitize=kernel-hwaddress)
> Add a workaround to include/linux/compiler_types.h so we always
> define __SANITIZE_ADDRESS__ for either mode, as we already do
> for clang.
Where is the clang work-around? (Or is this a statement that clang,
under -fsanitize=kernel-hwaddress, already sets __SANITIZE_ADDRESS__ by
default?
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> include/linux/compiler_types.h | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index aad6f6408bfa..2f2776fffefe 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -178,6 +178,13 @@ struct ftrace_likely_data {
> */
> #define noinline_for_stack noinline
>
> +/*
> + * Treat __SANITIZE_HWADDRESS__ the same as __SANITIZE_ADDRESS__ in the kernel
> + */
> +#ifdef __SANITIZE_HWADDRESS__
> +#define __SANITIZE_ADDRESS__
> +#endif
Should this go into compiler-gcc.h instead?
> +
> /*
> * Sanitizer helper attributes: Because using __always_inline and
> * __no_sanitize_* conflict, provide helper attributes that will either expand
> --
> 2.29.2
>
--
Kees Cook
On Mon, Oct 18, 2021 at 9:57 PM Kees Cook <[email protected]> wrote:
>
> On Wed, Oct 13, 2021 at 05:00:06PM +0200, Arnd Bergmann wrote:
> > From: Arnd Bergmann <[email protected]>
> >
> > GCC has separate macros for -fsanitize=kernel-address and
> > -fsanitize=kernel-hwaddress, and the check in the arm64 string.h
> > gets this wrong, which leads to string functions not getting
> > fortified with gcc. The newly added tests find this:
> >
> > warning: unsafe memchr() usage lacked '__read_overflow' warning in /git/arm-soc/lib/test_fortify/read_overflow-memchr.c
> > warning: unsafe memchr_inv() usage lacked '__read_overflow' symbol in /git/arm-soc/lib/test_fortify/read_overflow-memchr_inv.c
> > warning: unsafe memcmp() usage lacked '__read_overflow' warning in /git/arm-soc/lib/test_fortify/read_overflow-memcmp.c
> > warning: unsafe memscan() usage lacked '__read_overflow' symbol in /git/arm-soc/lib/test_fortify/read_overflow-memscan.c
> > warning: unsafe memcmp() usage lacked '__read_overflow2' warning in /git/arm-soc/lib/test_fortify/read_overflow2-memcmp.c
> > warning: unsafe memcpy() usage lacked '__read_overflow2' symbol in /git/arm-soc/lib/test_fortify/read_overflow2-memcpy.c
> > warning: unsafe memmove() usage lacked '__read_overflow2' symbol in /git/arm-soc/lib/test_fortify/read_overflow2-memmove.c
> > warning: unsafe memcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memcpy.c
> > warning: unsafe memmove() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memmove.c
> > warning: unsafe memset() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-memset.c
> > warning: unsafe strcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strcpy-lit.c
> > warning: unsafe strcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strcpy.c
> > warning: unsafe strlcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strlcpy-src.c
> > warning: unsafe strlcpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strlcpy.c
> > warning: unsafe strncpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strncpy-src.c
> > warning: unsafe strncpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strncpy.c
> > warning: unsafe strscpy() usage lacked '__write_overflow' symbol in /git/arm-soc/lib/test_fortify/write_overflow-strscpy.c
> >
>
> What is the build config that trips these warnings?
It's a randconfig build, I've uploaded one .config to
https://pastebin.com/raw/4TKB9mhs,
but I have other ones if you can't reproduce with that one.
> In trying to understand this, I see in arch/arm64/include/asm/string.h:
>
> #if (defined(CONFIG_KASAN_GENERIC) || defined(CONFIG_KASAN_SW_TAGS)) && \
> !defined(__SANITIZE_ADDRESS__)
>
> other architectures (like arm32) do:
>
> #if defined(CONFIG_KASAN) && !defined(__SANITIZE_ADDRESS__)
Yes, that is exactly the thing that goes wrong. With clang, __SANITIZE_ADDRESS__
gets set here, but gcc sets __SANITIZE_HWADDRESS__ instead
for CONFIG_KASAN_SW_TAGS, so the condition is always true.
> > Add a workaround to include/linux/compiler_types.h so we always
> > define __SANITIZE_ADDRESS__ for either mode, as we already do
> > for clang.
>
> Where is the clang work-around? (Or is this a statement that clang,
> under -fsanitize=kernel-hwaddress, already sets __SANITIZE_ADDRESS__ by
> default?
I mean this snippet:
#if __has_feature(address_sanitizer) || __has_feature(hwaddress_sanitizer)
/* Emulate GCC's __SANITIZE_ADDRESS__ flag */
#define __SANITIZE_ADDRESS__
#endif
Without that, clang sets neither __SANITIZE_ADDRESS__ nor
__SANITIZE_HWADDRESS__
> > diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> > index aad6f6408bfa..2f2776fffefe 100644
> > --- a/include/linux/compiler_types.h
> > +++ b/include/linux/compiler_types.h
> > @@ -178,6 +178,13 @@ struct ftrace_likely_data {
> > */
> > #define noinline_for_stack noinline
> >
> > +/*
> > + * Treat __SANITIZE_HWADDRESS__ the same as __SANITIZE_ADDRESS__ in the kernel
> > + */
> > +#ifdef __SANITIZE_HWADDRESS__
> > +#define __SANITIZE_ADDRESS__
> > +#endif
>
> Should this go into compiler-gcc.h instead?
Yes, that might be clearer, but the effect is the same, as no other
compiler defines
those macros.
Arnd
On Thu, 14 Oct 2021 19:40:45 -0700 Kees Cook <[email protected]> wrote:
>
>
> On October 14, 2021 1:12:54 AM PDT, Vincenzo Frascino <[email protected]> wrote:
> >
> >
> >On 10/13/21 5:00 PM, Arnd Bergmann wrote:
> >> From: Arnd Bergmann <[email protected]>
> >>
> >> Calling memcmp() and memchr() with an intentional buffer overflow
> >> is now caught at compile time:
> >>
> >> In function 'memcmp',
> >> inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
> >> include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> >> 263 | __read_overflow();
> >> | ^~~~~~~~~~~~~~~~~
> >> In function 'memchr',
> >> inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
> >> include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> >> 277 | __read_overflow();
> >> | ^~~~~~~~~~~~~~~~~
> >>
> >> Change the kasan tests to wrap those inside of a noinline function
> >> to prevent the compiler from noticing the bug and let kasan find
> >> it at runtime.
> >>
> >> Signed-off-by: Arnd Bergmann <[email protected]>
> >
> >Reviewed-by: Vincenzo Frascino <[email protected]>
>
> How about just explicitly making the size invisible to the compiler?
>
> I did this for similar issues in the same source:
>
> https://lore.kernel.org/linux-hardening/[email protected]/T/#u
>
Arnd?
On Thu, Oct 28, 2021 at 01:15:26PM -0700, Andrew Morton wrote:
> On Thu, 14 Oct 2021 19:40:45 -0700 Kees Cook <[email protected]> wrote:
>
> >
> >
> > On October 14, 2021 1:12:54 AM PDT, Vincenzo Frascino <[email protected]> wrote:
> > >
> > >
> > >On 10/13/21 5:00 PM, Arnd Bergmann wrote:
> > >> From: Arnd Bergmann <[email protected]>
> > >>
> > >> Calling memcmp() and memchr() with an intentional buffer overflow
> > >> is now caught at compile time:
> > >>
> > >> In function 'memcmp',
> > >> inlined from 'kasan_memcmp' at lib/test_kasan.c:897:2:
> > >> include/linux/fortify-string.h:263:25: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> > >> 263 | __read_overflow();
> > >> | ^~~~~~~~~~~~~~~~~
> > >> In function 'memchr',
> > >> inlined from 'kasan_memchr' at lib/test_kasan.c:872:2:
> > >> include/linux/fortify-string.h:277:17: error: call to '__read_overflow' declared with attribute error: detected read beyond size of object (1st parameter)
> > >> 277 | __read_overflow();
> > >> | ^~~~~~~~~~~~~~~~~
> > >>
> > >> Change the kasan tests to wrap those inside of a noinline function
> > >> to prevent the compiler from noticing the bug and let kasan find
> > >> it at runtime.
> > >>
> > >> Signed-off-by: Arnd Bergmann <[email protected]>
> > >
> > >Reviewed-by: Vincenzo Frascino <[email protected]>
> >
> > How about just explicitly making the size invisible to the compiler?
> >
> > I did this for similar issues in the same source:
> >
> > https://lore.kernel.org/linux-hardening/[email protected]/T/#u
This is already fixed in your tree with:
"kasan: test: consolidate workarounds for unwanted __alloc_size() protection"
which was based on this original patch (and my comments).
--
Kees Cook