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!
