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.2 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, 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 06B29C4332B for ; Thu, 19 Mar 2020 16:57:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id D59272071C for ; Thu, 19 Mar 2020 16:57:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728338AbgCSQ5h (ORCPT ); Thu, 19 Mar 2020 12:57:37 -0400 Received: from foss.arm.com ([217.140.110.172]:39026 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727382AbgCSQ5g (ORCPT ); Thu, 19 Mar 2020 12:57:36 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 052B930E; Thu, 19 Mar 2020 09:57:36 -0700 (PDT) Received: from [10.37.12.142] (unknown [10.37.12.142]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3018C3F305; Thu, 19 Mar 2020 09:57:31 -0700 (PDT) Subject: Re: [PATCH v4 18/26] arm64: vdso32: Replace TASK_SIZE_32 check in vgettimeofday To: Andy Lutomirski , Catalin Marinas Cc: linux-arch , linux-arm-kernel , LKML , clang-built-linux , "open list:MIPS" , X86 ML , Will Deacon , Arnd Bergmann , Russell King , Paul Burton , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Stephen Boyd , Mark Salyzyn , Kees Cook , Peter Collingbourne , Dmitry Safonov <0x7f454c46@gmail.com>, Andrei Vagin , Nick Desaulniers , Marc Zyngier , Mark Rutland , Will Deacon References: <20200317122220.30393-1-vincenzo.frascino@arm.com> <20200317122220.30393-19-vincenzo.frascino@arm.com> <20200317143834.GC632169@arrakis.emea.arm.com> From: Vincenzo Frascino Message-ID: <78109f4e-c9c7-57b6-079b-c911b6090aa0@arm.com> Date: Thu, 19 Mar 2020 16:58:00 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Andy, 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. -- Regards, Vincenzo From mboxrd@z Thu Jan 1 00:00:00 1970 From: Vincenzo Frascino Subject: Re: [PATCH v4 18/26] arm64: vdso32: Replace TASK_SIZE_32 check in vgettimeofday Date: Thu, 19 Mar 2020 16:58:00 +0000 Message-ID: <78109f4e-c9c7-57b6-079b-c911b6090aa0@arm.com> References: <20200317122220.30393-1-vincenzo.frascino@arm.com> <20200317122220.30393-19-vincenzo.frascino@arm.com> <20200317143834.GC632169@arrakis.emea.arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Return-path: Received: from foss.arm.com ([217.140.110.172]:39026 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727382AbgCSQ5g (ORCPT ); Thu, 19 Mar 2020 12:57:36 -0400 In-Reply-To: Content-Language: en-US Sender: linux-arch-owner@vger.kernel.org List-ID: To: Andy Lutomirski , Catalin Marinas Cc: linux-arch , linux-arm-kernel , LKML , clang-built-linux , "open list:MIPS" , X86 ML , Will Deacon , Arnd Bergmann , Russell King , Paul Burton , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Stephen Boyd , Mark Salyzyn , Kees Cook , Peter Collingbourne , Dmitry Safonov <0x7f454c46@gmail.com>, Andrei Vagin , Nick Desaulniers Hi Andy, 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. -- Regards, Vincenzo 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.3 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,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 D6426C4332D for ; Thu, 19 Mar 2020 16:57:44 +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 AA5D620836 for ; Thu, 19 Mar 2020 16:57:44 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="UvPF4XlH" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org AA5D620836 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com 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:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=FvFSnnYGtUIKzNHOYKg914Ld+X7yL0OuUJ/Dn4A4Uas=; b=UvPF4XlHS29sVk I6GVDHUx0L/OmMlPYBtbRVP/3PyLl0HFniCJOV/FW658rbEz4jdqgB7XkhQFnc+Nj03n21gWsCxiX TnD36vv8b1cgLtFgg3yNn3bHY25YvZwaLBskGO/BniptZLE9cjkwz7Ldql/lhQQUWQTlrktY9WnJL wYx2AQXADXlfpS1+GP8BSbSfTgJI+t95+YxZktlQ7Jdkqoh+GGIMCMI7QXHMJ5Aum54T3/NIzF/Rb AsnYQ6ai9LqkoUCLkisFIGmLXaDPEJqLftRwDg9TbkDmPv+49KQQ7gmLvMoVHByOehMfSX3y7R8C4 9NzPKhbxW9TgUJ5sE/yQ==; 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 1jEyUH-0004sV-4J; Thu, 19 Mar 2020 16:57:41 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jEyUE-0004rX-CA for linux-arm-kernel@lists.infradead.org; Thu, 19 Mar 2020 16:57:39 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 052B930E; Thu, 19 Mar 2020 09:57:36 -0700 (PDT) Received: from [10.37.12.142] (unknown [10.37.12.142]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 3018C3F305; Thu, 19 Mar 2020 09:57:31 -0700 (PDT) Subject: Re: [PATCH v4 18/26] arm64: vdso32: Replace TASK_SIZE_32 check in vgettimeofday To: Andy Lutomirski , Catalin Marinas References: <20200317122220.30393-1-vincenzo.frascino@arm.com> <20200317122220.30393-19-vincenzo.frascino@arm.com> <20200317143834.GC632169@arrakis.emea.arm.com> From: Vincenzo Frascino Message-ID: <78109f4e-c9c7-57b6-079b-c911b6090aa0@arm.com> Date: Thu, 19 Mar 2020 16:58:00 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200319_095738_508159_C2803729 X-CRM114-Status: GOOD ( 18.81 ) 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 , Dmitry Safonov <0x7f454c46@gmail.com>, "open list:MIPS" , Will Deacon , linux-arch , Marc Zyngier , X86 ML , Russell King , clang-built-linux , Ingo Molnar , Kees Cook , Arnd Bergmann , Will Deacon , Borislav Petkov , 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 Hi Andy, 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. -- Regards, Vincenzo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel