| Message ID | 20200423154503.5103-4-dja@axtens.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix some incompatibilites between KASAN and FORTIFY_SOURCE | expand |
On Thu, Apr 23, 2020 at 5:45 PM Daniel Axtens <dja@axtens.net> wrote: > > memcmp may bail out before accessing all the memory if the buffers > contain differing bytes. kasan_memcmp calls memcmp with a stack array. > Stack variables are not necessarily initialised (in the absence of a > compiler plugin, at least). Sometimes this causes the memcpy to bail > early thus fail to trigger kasan. > > Make sure the array initialised to zero in the code. > > No other test is dependent on the contents of an array on the stack. > > Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> > Cc: Alexander Potapenko <glider@google.com> > Cc: Dmitry Vyukov <dvyukov@google.com> > Signed-off-by: Daniel Axtens <dja@axtens.net> > Reviewed-by: Dmitry Vyukov <dvyukov@google.com> > --- > lib/test_kasan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/lib/test_kasan.c b/lib/test_kasan.c > index 939f395a5392..7700097842c8 100644 > --- a/lib/test_kasan.c > +++ b/lib/test_kasan.c > @@ -638,7 +638,7 @@ static noinline void __init kasan_memcmp(void) > { > char *ptr; > size_t size = 24; > - int arr[9]; > + int arr[9] = {}; > > pr_info("out-of-bounds in memcmp\n"); > ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); My version of this function contains the following below: memset(arr, 0, sizeof(arr)); What am I missing?
Dmitry Vyukov <dvyukov@google.com> writes: > On Thu, Apr 23, 2020 at 5:45 PM Daniel Axtens <dja@axtens.net> wrote: >> >> memcmp may bail out before accessing all the memory if the buffers >> contain differing bytes. kasan_memcmp calls memcmp with a stack array. >> Stack variables are not necessarily initialised (in the absence of a >> compiler plugin, at least). Sometimes this causes the memcpy to bail >> early thus fail to trigger kasan. >> >> Make sure the array initialised to zero in the code. >> >> No other test is dependent on the contents of an array on the stack. >> >> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> >> Cc: Alexander Potapenko <glider@google.com> >> Cc: Dmitry Vyukov <dvyukov@google.com> >> Signed-off-by: Daniel Axtens <dja@axtens.net> >> Reviewed-by: Dmitry Vyukov <dvyukov@google.com> >> --- >> lib/test_kasan.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/lib/test_kasan.c b/lib/test_kasan.c >> index 939f395a5392..7700097842c8 100644 >> --- a/lib/test_kasan.c >> +++ b/lib/test_kasan.c >> @@ -638,7 +638,7 @@ static noinline void __init kasan_memcmp(void) >> { >> char *ptr; >> size_t size = 24; >> - int arr[9]; >> + int arr[9] = {}; >> >> pr_info("out-of-bounds in memcmp\n"); >> ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO); > > My version of this function contains the following below: > > memset(arr, 0, sizeof(arr)); > > What am I missing? Ah! It turns out I accidentally removed the memset in patch 1. No idea why I did that. I'll fix up patch 1 to not remove the memset and drop this patch. Daniel
diff --git a/lib/test_kasan.c b/lib/test_kasan.c index 939f395a5392..7700097842c8 100644 --- a/lib/test_kasan.c +++ b/lib/test_kasan.c @@ -638,7 +638,7 @@ static noinline void __init kasan_memcmp(void) { char *ptr; size_t size = 24; - int arr[9]; + int arr[9] = {}; pr_info("out-of-bounds in memcmp\n"); ptr = kmalloc(size, GFP_KERNEL | __GFP_ZERO);