linux-kernel - Re: [PATCH v4] mm: x86: instrument __get/__put_kernel

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: <CA+fCnZfXp3-NOq-LB13q8V6tv=H976AeNQXU8p-VGK-m9Qdh1g@mail.gmail.com>
Date: Sun, 22 Sep 2024 14:06:34 +0200
From: Andrey Konovalov <andreyknvl@...il.com>
To: Sabyrzhan Tasbolatov <snovitoll@...il.com>, glider@...gle.com
Cc: akpm@...ux-foundation.org, bp@...en8.de, brauner@...nel.org, 
	dave.hansen@...ux.intel.com, dhowells@...hat.com, dvyukov@...gle.com, 
	hpa@...or.com, kasan-dev@...glegroups.com, linux-kernel@...r.kernel.org, 
	linux-mm@...ck.org, mingo@...hat.com, ryabinin.a.a@...il.com, 
	tglx@...utronix.de, vincenzo.frascino@....com, x86@...nel.org
Subject: Re: [PATCH v4] mm: x86: instrument __get/__put_kernel_nofault

On Sat, Sep 21, 2024 at 9:09 AM Sabyrzhan Tasbolatov
<snovitoll@...il.com> wrote:
>
> Instrument copy_from_kernel_nofault(), copy_to_kernel_nofault(),
> strncpy_from_kernel_nofault() where __put_kernel_nofault,
> __get_kernel_nofault macros are used.
>
> __get_kernel_nofault needs instrument_memcpy_before() which handles
> KASAN, KCSAN checks for src, dst address, whereas for __put_kernel_nofault
> macro, instrument_write() check should be enough as it's validated via
> kmsan_copy_to_user() in instrument_put_user().
>
> __get_user_size was appended with instrument_get_user() for KMSAN check in
> commit 888f84a6da4d("x86: asm: instrument usercopy in get_user() and
> put_user()") but only for CONFIG_CC_HAS_ASM_GOTO_OUTPUT.
>
> copy_from_to_kernel_nofault_oob() kunit test triggers 4 KASAN OOB
> bug reports as expected, one for each copy_from/to_kernel_nofault call.
>
> Reported-by: Andrey Konovalov <andreyknvl@...il.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=210505
> Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@...il.com>
> ---
> v3: changed kunit test from UAF to OOB case and git commit message.
> v4: updated a grammar in git commit message.
> ---
>  arch/x86/include/asm/uaccess.h |  4 ++++
>  mm/kasan/kasan_test.c          | 21 +++++++++++++++++++++
>  2 files changed, 25 insertions(+)
>
> diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
> index 3a7755c1a441..87fb59071e8c 100644
> --- a/arch/x86/include/asm/uaccess.h
> +++ b/arch/x86/include/asm/uaccess.h
> @@ -353,6 +353,7 @@ do {                                                                        \
>         default:                                                        \
>                 (x) = __get_user_bad();                                 \
>         }                                                               \
> +       instrument_get_user(x);                                         \
>  } while (0)

instrument_get_user is KMSAN-related, so I don't think this change
belongs as a part of this patch.

Perhaps Alexander can comment on whether we need to add
instrument_get_user here for KMSAN.