From patchwork Fri Oct 27 02:00:04 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?b?THZxaWFuZyBIdWFuZyAo6buE5ZCV5by6KQ==?= X-Patchwork-Id: 10028969 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 3F95B60375 for ; Fri, 27 Oct 2017 02:01:25 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3018C28E4F for ; Fri, 27 Oct 2017 02:01:25 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 24F2A28E6B; Fri, 27 Oct 2017 02:01:25 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 289E728E4F for ; Fri, 27 Oct 2017 02:01:23 +0000 (UTC) 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:MIME-Version:In-Reply-To:References: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=/D24PO6k0GqNQ3C8h0JBl2+ouWxgScPeymxdEe065OI=; b=mtNCG4EjEEUgxt wz5LDUDbGn4YKocH/nl3sc8WPrrbpUHqY61v3vw39MKKVm38ytBSF3YyVN22urcaI9w+BecWGBZHi TfI/NWw+PxWyrFHeoRSeEmABEvCsOBNpVIBdEI7sGIJlY3V5Fx3Abjmq4URSfd46uKdjO5Hml3e7b S21JHQtMkwtLyPXdq5WKxQsRiD/rzS6Z446Qo9Q15UdetA+foPKd7o4S40YZifCAJbrkbD0xHAFRO FpMh/sEbF1MJslEMgXVUyuXNgSK2oDBV4WgAGgRtGfnZAIfrhCLZpSYb5Elmyi1FI62BPwm21/pSU P2RJkNpyKaw2/8WzT7Vg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1e7txa-0000jE-7L; Fri, 27 Oct 2017 02:01:22 +0000 Received: from [222.66.158.135] (helo=SHSQR01.spreadtrum.com) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1e7txU-0000Z7-9F for linux-arm-kernel@lists.infradead.org; Fri, 27 Oct 2017 02:01:20 +0000 Received: from ig2.spreadtrum.com (shmbx01.spreadtrum.com [10.0.1.203]) by SHSQR01.spreadtrum.com with ESMTP id v9R202WV048591 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 27 Oct 2017 10:00:02 +0800 (CST) (envelope-from lvqiang.huang@spreadtrum.com) Received: from SHMBX04.spreadtrum.com (10.0.1.214) by SHMBX01.spreadtrum.com (10.0.1.203) with Microsoft SMTP Server (TLS) id 15.0.847.32; Fri, 27 Oct 2017 10:00:04 +0800 Received: from SHMBX04.spreadtrum.com ([fe80::5cf6:e5f3:7a55:e6ca]) by SHMBX04.spreadtrum.com ([fe80::5cf6:e5f3:7a55:e6ca%13]) with mapi id 15.00.0847.030; Fri, 27 Oct 2017 10:00:04 +0800 From: =?gb2312?B?THZxaWFuZyBIdWFuZyAou8bCwMe/KQ==?= To: Russell King Subject: Re: [PATCH] ARM: Fix csum_partial_copy_from_user() stack mismatch Thread-Topic: [PATCH] ARM: Fix csum_partial_copy_from_user() stack mismatch Thread-Index: AQHTTJrs9zTD+UgfjkKn+Ar/LyezUqL27RCw Date: Fri, 27 Oct 2017 02:00:04 +0000 Message-ID: References: <20171024073225.17374-1-chunyan.zhang@spreadtrum.com> In-Reply-To: <20171024073225.17374-1-chunyan.zhang@spreadtrum.com> Accept-Language: en-US Content-Language: zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.0.64.19] MIME-Version: 1.0 X-MAIL: SHSQR01.spreadtrum.com v9R202WV048591 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20171026_190116_900432_A219031C X-CRM114-Status: GOOD ( 16.72 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chunyan Zhang , =?gb2312?B?T3Jzb24gWmhhaSAotdS+qSk=?= , =?gb2312?B?Q2h1bnlhbiBaaGFuZyAo1cW0utHeKQ==?= , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Russell, The bug was introduced by the commit a5e090acbf545c0a3b04080f8a488b17ec41fe02 Author: Russell King Date: Wed Aug 19 20:40:41 2015 +0100 ARM: software-based priviledged-no-access support Provide a software-based implementation of the priviledged no access support found in ARMv8.1. Userspace pages are mapped using a different domain number from the kernel and IO mappings. If we switch the user domain to "no access" when we enter the kernel, we can prevent the kernel from touching userspace. However, the kernel needs to be able to access userspace via the various user accessor functions. With the wrapping in the previous patch, we can temporarily enable access when the kernel needs user access, and re-disable it afterwards. This allows us to trap non-intended accesses to userspace, eg, caused by an inadvertent dereference of the LIST_POISON* values, which, with appropriate user mappings setup, can be made to succeed. This in turn can allow use-after-free bugs to be further exploited than would otherwise be possible. Signed-off-by: Russell King ...... -- 1.7.9.5 diff --git a/arch/arm/lib/csumpartialcopyuser.S b/arch/arm/lib/csumpartialcopyuser.S index 1d0957e..1712f13 100644 --- a/arch/arm/lib/csumpartialcopyuser.S +++ b/arch/arm/lib/csumpartialcopyuser.S @@ -17,6 +17,19 @@ .text +#ifdef CONFIG_CPU_SW_DOMAIN_PAN + .macro save_regs + mrc p15, 0, ip, c3, c0, 0 + stmfd sp!, {r1, r2, r4 - r8, ip, lr} + uaccess_enable ip + .endm + + .macro load_regs + ldmfd sp!, {r1, r2, r4 - r8, ip, lr} + mcr p15, 0, ip, c3, c0, 0 + ret lr + .endm +#else .macro save_regs stmfd sp!, {r1, r2, r4 - r8, lr} .endm @@ -24,6 +37,7 @@ .macro load_regs ldmfd sp!, {r1, r2, r4 - r8, pc} .endm +#endif ...... The following is based on CONFIG_CPU_SW_DOMAIN_PAN defined. the commit modified the save_reg marco. + stmfd sp!, {r1, r2, r4 - r8, ip, lr} So, additional ip will push to the stack. The csum_partial_copy_from_user() use the save_reg marco. /* * unsigned int * csum_partial_copy_from_user(const char *src, char *dst, int len, int sum, int *err_ptr) * r0 = src, r1 = dst, r2 = len, r3 = sum, [sp] = *err_ptr * Returns : r0 = checksum, [[sp, #0], #0] = 0 or -EFAULT */ The src r0 is from user space. and if user buffer become not mapped for some reasons, the ldr from r0 will cause an abort, the abort handler will return the PC to .fixup entry 9001: 9001: mov r4, #-EFAULT ldr r5, [sp, #8*4] @ *err_ptr str r4, [r5] ldmia sp, {r1, r2} @ retrieve dst, len add r2, r2, r1 mov r0, #0 @ zero the buffer the r5 will pointer to the lr pushed. ldr r5, [sp, #8*4] @ *err_ptr then str to lr is the bug. str r4, [r5] we hit the bug many times recently. Here is a example, [ 259.378437] c0 Unable to handle kernel paging request at virtual address c0494578 [ 259.378460] c0 pgd = dc888000 [ 259.378469] c0 [c0494578] *pgd=8041940e(bad) [ 259.378490] c0 Internal error: Oops: 80d [#1] PREEMPT SMP ARM [ 259.384159] c0 Modules linked in: sprdwl_ng(O) mtty marlin2_fm mali_kbase(O) [ 259.384191] c0 CPU: 0 PID: 7068 Comm: AsyncTask #3 Tainted: G W O 4.4.83-00113-g5c60505 #1 [ 259.384200] c0 Hardware name: sc9850k [ 259.384211] c0 task: e00eb480 task.stack: e0080000 [ 259.384228] c0 PC is at csum_partial_copy_from_user+0x3bc/0x3e4 [ 259.384242] c0 LR is at csum_and_copy_from_iter+0x340/0x4f4 [ 259.384254] c0 pc : [] lr : [] psr: 000d0013 sp : e0081d8c ip : 00000170 fp : e0081e04 [ 259.384267] c0 r10: e0081edc r9 : e0081ecc r8 : 00000174 [ 259.384277] c0 r7 : 00000000 r6 : dd9bda84 r5 : c0494578 r4 : fffffff2 [ 259.384287] c0 r3 : 00000000 r2 : 00000174 r1 : dd9bd910 r0 : 8a1779d8 [ 259.384298] c0 Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user [ 259.384309] c0 Control: 10c5387d Table: 9c88806a DAC: 00000055 The r5 = LR = c0494578, at csum_and_copy_from_iter+0x340/0x4f4 The str to LR cause an kernel crash. 0xc04809d0 : mvn r4, #13 0xc04809d4 : ldr r5, [sp, #32] 0xc04809d8 : str r4, [r5] We can see the r0 is not mapped at that time. crash> vtop 8a1779d8 VIRTUAL PHYSICAL 8a1779d8 (not mapped) The backtrace get from the stack data is csum_partial_copy_from_user csum_and_copy_from_iter+832 tcp_sendmsg+1008 inet_sendmsg+156. sock_sendmsg+68 sys_sendto+204 the patch for the bug 9001: mov r4, #-EFAULT +#ifdef CONFIG_CPU_SW_DOMAIN_PAN + ldr r5, [sp, #9*4] @ *err_ptr +#else ldr r5, [sp, #8*4] @ *err_ptr +#endif str r4, [r5] I'm sure you can provide a better solution. looking forward to your reply, thanks. Best Wishes, Lvqiang Huang -----邮件原件----- 发件人: Chunyan Zhang (张春艳) 发送时间: 2017年10月24日 15:32 收件人: Russell King 抄送: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Orson Zhai (翟京); Chunyan Zhang; Lvqiang Huang (黄吕强) 主题: [PATCH] ARM: Fix csum_partial_copy_from_user() stack mismatch From: Lvqiang Huang An additional 'ip' will be pushed to the stack, for restoring the DACR later, if CONFIG_CPU_SW_DOMAIN_PAN defined. However, the fixup still get the err_ptr by add #8*4 to sp, which results in the fact that the code area pointed by the LR will be overwritten, or the kernel will crash if CONFIG_DEBUG_RODATA is enabled. This patch fixes the stack mismatch. Signed-off-by: Lvqiang Huang Signed-off-by: Chunyan Zhang --- arch/arm/lib/csumpartialcopyuser.S | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm/lib/csumpartialcopyuser.S b/arch/arm/lib/csumpartialcopyuser.S index 1712f13..b83fdc0 100644 --- a/arch/arm/lib/csumpartialcopyuser.S +++ b/arch/arm/lib/csumpartialcopyuser.S @@ -85,7 +85,11 @@ .pushsection .text.fixup,"ax" .align 4 9001: mov r4, #-EFAULT +#ifdef CONFIG_CPU_SW_DOMAIN_PAN + ldr r5, [sp, #9*4] @ *err_ptr +#else ldr r5, [sp, #8*4] @ *err_ptr +#endif str r4, [r5] ldmia sp, {r1, r2} @ retrieve dst, len add r2, r2, r1