From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751867AbdB0JnK (ORCPT ); Mon, 27 Feb 2017 04:43:10 -0500 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:38547 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751597AbdB0JnG (ORCPT ); Mon, 27 Feb 2017 04:43:06 -0500 Date: Mon, 27 Feb 2017 07:21:30 +0100 From: Martin Schwidefsky To: Linus Torvalds Cc: Linux Kernel Mailing List , Alexander Viro , Carsten Otte Subject: Re: Using TASK_SIZE for kernel threads In-Reply-To: References: <20170224161529.80054-1-schwidefsky@de.ibm.com> X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Content-Scanned: Fidelis XPS MAILER x-cbid: 17022706-0008-0000-0000-000003F03A5F X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17022706-0009-0000-0000-00001C9912DB Message-Id: <20170227072130.76a0b6e2@mschwideX1> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-02-27_03:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1612050000 definitions=main-1702270065 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 25 Feb 2017 10:19:04 -0800 Linus Torvalds wrote: > On Fri, Feb 24, 2017 at 8:15 AM, Martin Schwidefsky > 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. I see, set_fs(KERNEL_DS) sets a different address for user_addr_max to return. That would work but requires that all architectures have the define. > 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. The background is that TASK_SIZE on s390 is not a constant, it depends on the layout of the mm. There are three, 2GB for 31-bit with a 2-level page table, 4TB for a standard 64-bit process with a 3-level page table and 8PB with 4 levels for a process that did a really large mmap. The upgrade from 4TB to 8PB is at runtime, that is why the size of the mm is stored in mm->context. It is an attribute of the mm, if one thread changes it, it changes for all threads. > 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.. I guess that won't happen anytime soon. I will use the proposed fix within the arch code. Thanks. -- blue skies, Martin. "Reality continues to ruin my life." - Calvin.