Re: [PATCH RESEND v3 2/3] kasan: migrate copy_user_test to kunit

[Sabyrzhan Tasbolatov]

On Tue, Oct 15, 2024 at 6:18 AM Andrey Konovalov <andreyknvl@xxxxxxxxx> wrote:
>
> On Tue, Oct 15, 2024 at 1:10 AM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> >
> > On Mon, 14 Oct 2024 07:57:00 +0500 Sabyrzhan Tasbolatov <snovitoll@xxxxxxxxx> wrote:
> >
> > > Migrate the copy_user_test to the KUnit framework to verify out-of-bound
> > > detection via KASAN reports in copy_from_user(), copy_to_user() and
> > > their static functions.
> > >
> > > This is the last migrated test in kasan_test_module.c, therefore delete
> > > the file.
> > >
> >
> > x86_64 allmodconfig produces:
> >
> > vmlinux.o: warning: objtool: strncpy_from_user+0x8a: call to __check_object_size() with UACCESS enabled

I've missed this warning during x86_64 build, sorry.

>
> Too bad. I guess we have to duplicate both kasan_check_write and
> check_object_size before both do_strncpy_from_user calls in
> strncpy_from_user.

Shall we do it once in strncpy_from_user() as I did in v1?
Please let me know as I've tested in x86_64 and arm64 -
there is no warning during kernel build with the diff below.

These checks are for kernel pointer *dst only and size:
   kasan_check_write(dst, count);
   check_object_size(dst, count, false);

And there are 2 calls of do_strncpy_from_user,
which are implemented in x86 atm per commit 2865baf54077,
and they are relevant to __user *src address, AFAIU.

long strncpy_from_user()
   if (can_do_masked_user_access()) {
      src = masked_user_access_begin(src);
      retval = do_strncpy_from_user(dst, src, count, count);
      user_read_access_end();
   }

   if (likely(src_addr < max_addr)) {
      if (user_read_access_begin(src, max)) {
         retval = do_strncpy_from_user(dst, src, count, max);
         user_read_access_end();

---
diff --git a/lib/strncpy_from_user.c b/lib/strncpy_from_user.c
index 989a12a6787..6dc234913dd 100644
--- a/lib/strncpy_from_user.c
+++ b/lib/strncpy_from_user.c
@@ -120,6 +120,9 @@ long strncpy_from_user(char *dst, const char
__user *src, long count)
        if (unlikely(count <= 0))
                return 0;

+       kasan_check_write(dst, count);
+       check_object_size(dst, count, false);
+
        if (can_do_masked_user_access()) {
                long retval;

@@ -142,8 +145,6 @@ long strncpy_from_user(char *dst, const char
__user *src, long count)
                if (max > count)
                        max = count;

-               kasan_check_write(dst, count);
-               check_object_size(dst, count, false);
                if (user_read_access_begin(src, max)) {
                        retval = do_strncpy_from_user(dst, src, count, max);
                        user_read_access_end();