linux-kernel - Re: Using TASK_SIZE for kernel threads

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+55aFxuv=a4o9bxbfBmq9XHgyju82xEnwjXY6NFLj2Ko2kKuQ@mail.gmail.com>
Date:   Sat, 25 Feb 2017 10:19:04 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Martin Schwidefsky <schwidefsky@...ibm.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Alexander Viro <viro@...iv.linux.org.uk>,
        Carsten Otte <cotte@...ibm.com>
Subject: Re: Using TASK_SIZE for kernel threads

On Fri, Feb 24, 2017 at 8:15 AM, Martin Schwidefsky
<schwidefsky@...ibm.com> wrote:
>
> Now I fixed this in the s390 code, the patch is queued and will be
> included in next weeks please-pull. But I am wondering about the use
> of TASK_SIZE in kernel threads. For x86 copy_mount_options works
> because the size calculation will give a negative result for 'data'
> pointing to kernel space. Which is corrected by the size limit:
>
>         if (size > PAGE_SIZE)
>                 size = PAGE_SIZE;
>
> Wouldn't it be cleaner to test "get_fs()==KERNEL_DS" and just use
> size=4096 in this case? The detour via TASK_SIZE does not make much
> sense to me.
>
> To find out how big the problem is, I have added a warning to TASK_SIZE
> to create a console messsage if it is called for a task without an mm.
> The only hit has been copy_mount_options.

So copy_mount_options() is a horrible hack. It doesn't have a size
limit, and it can copy binary data, so our good auto-limiting code in
strncpy_from_user() isn't usable either.

It probably *should* use the same user_addr_max() logic that
strncpy_from_user() uses, but that wouldn't actually have helped s390,
because s390 doesn't use the generic strncpy_from_user(), and doesn't
have that user_addr_max() thing.

So from everything I see, I think this is actually a s390 bug in every
way. Your TASK_SIZE_OF() implementation is simply bogus and broken,
and that's the core problem.

For example, you could have just had

   #define user_addr_max()   (current_thread_info()->addr_limit.seg)

like some other architectures, and it would have been all good.

If somebody is willing to add user_addr_max() to all architectures and
make copy_mount_options() use the same logic as
lib/strncpy_from_user.c, then that would certainly be acceptable to
me. As it is, I think it uses TASK_SIZE in ways that are not pretty,
but are what they are..

                Linus