linux-kernel - Re: [PATCH RESEND v3 2/3] kasan: migrate copy_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACzwLxgJaOL9RXkhAZEosmFDzp-D4=gGfhSh3d5scBRBaq76pw@mail.gmail.com>
Date: Tue, 15 Oct 2024 15:53:35 +0500
From: Sabyrzhan Tasbolatov <snovitoll@...il.com>
To: Andrey Konovalov <andreyknvl@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, 2023002089@...k.tyut.edu.cn, 
	alexs@...nel.org, corbet@....net, dvyukov@...gle.com, elver@...gle.com, 
	glider@...gle.com, kasan-dev@...glegroups.com, linux-doc@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-mm@...ck.org, ryabinin.a.a@...il.com, 
	siyanteng@...ngson.cn, vincenzo.frascino@....com, workflows@...r.kernel.org
Subject: Re: [PATCH RESEND v3 2/3] kasan: migrate copy_user_test to kunit

On Tue, Oct 15, 2024 at 6:18 AM Andrey Konovalov <andreyknvl@...il.com> wrote:
>
> On Tue, Oct 15, 2024 at 1:10 AM Andrew Morton <akpm@...ux-foundation.org> wrote:
> >
> > On Mon, 14 Oct 2024 07:57:00 +0500 Sabyrzhan Tasbolatov <snovitoll@...il.com> 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();