On Fri, Feb 24, 2017 at 8:15 AM, Martin Schwidefsky
<schwidef...@de.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
