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

[Andrey Konovalov]

On Tue, Oct 15, 2024 at 12:52 PM Sabyrzhan Tasbolatov
<snovitoll@xxxxxxxxx> wrote:
>
> > 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();

Ok, let's do this. (What looked concerning to me with this approach
was doing the KASAN/userscopy checks outside of the src_addr <
max_addr, but I suppose that should be fine.)

Thank you!