From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.5 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6382FC4332D for ; Thu, 19 Mar 2020 18:33:07 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3487020787 for ; Thu, 19 Mar 2020 18:33:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="pPyPgdkv"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="sEmiPLH8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3487020787 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/MIpa5e1JW0vIIO/vT5a5UVgI6vbZSvWqTnk7kolddQ=; b=pPyPgdkvnY/RwW pfyQuB0+dGKafoLR4GEJU7NAqgbLDdSz00YtRQepnPf+/Zj+3x8ELlqkct/t0kqCVbBbYmoohjIbP A47lWDGvQY4opGDFhGtQOgsu+wUO556euZWyV5QT7apVuTroFkUnqwSFUGvy02J4yhADcZYi8qPtx 7A7CnxBZ9hEZzbInCapKq7/WLAMR4AS7WrthAfqFzgb2AwHhEiPIctQtUnYLX0WkjLcHazD3hrgAQ sTwStGIX8wo/AD33nCSMgvtmBX9WCtqT56/0F4URxmVnToCodGmlOPb8d2Z1wQoW/K8jLAyukbqil NVJDB+ovy9qRaLuLExfQ==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jEzyY-0004Ip-Ns; Thu, 19 Mar 2020 18:33:02 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jEzyV-0004IV-Q2 for linux-arm-kernel@lists.infradead.org; Thu, 19 Mar 2020 18:33:01 +0000 Received: from willie-the-truck (236.31.169.217.in-addr.arpa [217.169.31.236]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 8D66020787; Thu, 19 Mar 2020 18:32:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584642779; bh=vWC53xuIwbcvlZtUjvLtXkAFlLvKIaP8Wi2Wi740fjk=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=sEmiPLH8nJ/EiqLkDJJok7EXoTzk4Hbjcn4KHPM4rD/IjInM0z0WlYNhLDgYF8a0l gM3dC3mrKg9Z8S/KQ4+wB0n5zHHA0TB70S3WmQMS+bHF+SntJgBQxlIse07gKfzYDi nMTNWWhWpYOF1oZG7xFkI7Bi//YMYz15t7NPGEPA= Date: Thu, 19 Mar 2020 18:32:52 +0000 From: Will Deacon To: Vincenzo Frascino Subject: Re: [PATCH v4 18/26] arm64: vdso32: Replace TASK_SIZE_32 check in vgettimeofday Message-ID: <20200319183251.GA27141@willie-the-truck> References: <20200317122220.30393-1-vincenzo.frascino@arm.com> <20200317122220.30393-19-vincenzo.frascino@arm.com> <20200317143834.GC632169@arrakis.emea.arm.com> <78109f4e-c9c7-57b6-079b-c911b6090aa0@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <78109f4e-c9c7-57b6-079b-c911b6090aa0@arm.com> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200319_113259_885490_7150977A X-CRM114-Status: GOOD ( 23.39 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Catalin Marinas , Dmitry Safonov <0x7f454c46@gmail.com>, "open list:MIPS" , linux-arch , Marc Zyngier , X86 ML , Russell King , clang-built-linux , Ingo Molnar , Kees Cook , Arnd Bergmann , Will Deacon , Borislav Petkov , Andy Lutomirski , Thomas Gleixner , Peter Collingbourne , linux-arm-kernel , Andrei Vagin , Stephen Boyd , Nick Desaulniers , LKML , Mark Salyzyn , Paul Burton Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Mar 19, 2020 at 04:58:00PM +0000, Vincenzo Frascino wrote: > On 3/19/20 3:49 PM, Andy Lutomirski wrote: > > On Tue, Mar 17, 2020 at 7:38 AM Catalin Marinas wrote: > >> > >> On Tue, Mar 17, 2020 at 12:22:12PM +0000, Vincenzo Frascino wrote: > >>> diff --git a/arch/arm64/kernel/vdso32/vgettimeofday.c b/arch/arm64/kernel/vdso32/vgettimeofday.c > >>> index 54fc1c2ce93f..91138077b073 100644 > >>> --- a/arch/arm64/kernel/vdso32/vgettimeofday.c > >>> +++ b/arch/arm64/kernel/vdso32/vgettimeofday.c > >>> @@ -8,11 +8,14 @@ > >>> #include > >>> #include > >>> > >>> +#define VALID_CLOCK_ID(x) \ > >>> + ((x >= 0) && (x < VDSO_BASES)) > >>> + > >>> int __vdso_clock_gettime(clockid_t clock, > >>> struct old_timespec32 *ts) > >>> { > >>> /* The checks below are required for ABI consistency with arm */ > >>> - if ((u32)ts >= TASK_SIZE_32) > >>> + if ((u32)ts > UINTPTR_MAX - sizeof(*ts) + 1) > >>> return -EFAULT; > >>> > >>> return __cvdso_clock_gettime32(clock, ts); > >> > >> I probably miss something but I can't find the TASK_SIZE check in the > >> arch/arm/vdso/vgettimeofday.c code. Is this done elsewhere? > >> > > > > Can you not just remove the TASK_SIZE_32 check entirely? If you pass > > a garbage address to the vDSO, you are quite likely to get SIGSEGV. > > Why does this particular type of error need special handling? > > > > In this particular case the system call and the vDSO library as it stands > returns -EFAULT on all the architectures that support the vdso library except on > arm64 compat. The reason why it does not return the correct error code, as I was > discussing with Catalin, it is because arm64 uses USER_DS (addr_limit set > happens in arch/arm64/kernel/entry.S), which is defined as (1 << VA_BITS), as > access_ok() validation even on compat tasks and since arm64 supports up to 52bit > VA, this does not detect the end of the user address space for a 32 bit task. > Hence when we fall back on the system call we get the wrong error code out of it. > > According to me to have ABI consistency we need this check, but if we say that > we can make an ABI exception in this case, I am fine with that either if it has > enough consensus. > > Please let me know your thoughts. I don't agree with your reasoning -- letting the thing SEGV is perfectly fine and we don't need to perform additional checking in userspace here. If you treat the vDSO more as being part of libc then part of the kernel then I think it makes perfect sense. There are other system calls that will SEGV in libc if they are passed dodgy pointers before the kernel has a chance to return -EFAULT. Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel