From patchwork Mon Aug 14 21:37:30 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thomas Garnier X-Patchwork-Id: 9900183 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 537F9602D9 for ; Mon, 14 Aug 2017 21:38:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 46462283FF for ; Mon, 14 Aug 2017 21:38:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 3ABFB2873A; Mon, 14 Aug 2017 21:38:29 +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.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id DD8E3283FF for ; Mon, 14 Aug 2017 21:38:27 +0000 (UTC) Received: (qmail 23987 invoked by uid 550); 14 Aug 2017 21:38:19 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 23802 invoked from network); 14 Aug 2017 21:38:17 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=0qAd4Yaw7+6Lk/NhZaNSI7QgWGbolA3fH+YjYeEX02k=; b=KM/vqEVBxCMhYIaht/WA4KmuPPEf5PyEUrbU60zTFDOcVpaqpm0fACSCjSiNFSsTt0 uzo71Se/qWOJN1TqBzKJuhLsCmv8UTxQVgpFV3LlANJJZzBuo9rw6CrAAuEyN7sOEe3L ivdSadcYbAqJElAeJQ5dlfpnKKPxePHWWgy+qqabcu6vu+PMCenG/+a1d4vl/ZeEMeT4 itIT+lqBX6lE5XQF3ze2TNxnp+oIsdNbFdVPdIS4XofGkXvMr4gCSeO+Wte4Pi2escfW dvQ7qt7qIJZOC8ygG81wtpAk5twciUtzw46QUGPEkJMN3lPn+il+mO42zuOlcz0w6qfA twHA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=0qAd4Yaw7+6Lk/NhZaNSI7QgWGbolA3fH+YjYeEX02k=; b=nl9UBnXLZtzuaIl8eXiXdnUashfbtaV75iVww09jc2UCfma3Kr1Z2NLoWa7H8HvZLX SHOwrx/Dcv5tM0bqFh/6nCuP3eiW6eztlwDkJdeQURg4cVcI+XCikoK7JT/rYnZF9PbB w7jG1m+DrmyyrTfXOVL4Fy33RuOMlWbgkaKyu4bFzUuTCahy+nr+hb5RxDOFNI+Nfdeq xmnqpqHwS6n4O8iOY71B2oFO6xdnGigGJhnAU8qawpJPUkzXiqgdNh9A+KuzL6Y8Wqfy 6hZON/q05kNptFEW7srvHgAvNjRxFmB92Jr2CHB4+mew5QKTu2g6mFZg7LUe7UfDElud W1eQ== X-Gm-Message-State: AHYfb5jxOTuBUTFmtRQ02mzAkfniXSkwI1Uwq1F3nR4yJG9zutIpDAXK 3c3zzLWiK7vIwhdl X-Received: by 10.98.33.148 with SMTP id o20mr25968934pfj.89.1502746685394; Mon, 14 Aug 2017 14:38:05 -0700 (PDT) From: Thomas Garnier To: Al Viro , Dave Hansen , Arnd Bergmann , Thomas Gleixner , Thomas Garnier , Yonghong Song , David Howells , Russell King , Kees Cook , Andy Lutomirski , Will Drewry , Dave Martin , Catalin Marinas , Will Deacon Cc: linux-api@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kernel-hardening@lists.openwall.com Date: Mon, 14 Aug 2017 14:37:30 -0700 Message-Id: <20170814213732.104301-2-thgarnie@google.com> X-Mailer: git-send-email 2.14.1.480.gb18f417b89-goog In-Reply-To: <20170814213732.104301-1-thgarnie@google.com> References: <20170814213732.104301-1-thgarnie@google.com> Subject: [kernel-hardening] [PATCH v3 2/4] Revert "arm/syscalls: Check address limit on user-mode return" X-Virus-Scanned: ClamAV using ClamSMTP This reverts commit 73ac5d6a2b6ac3ae8d1e1818f3e9946f97489bc9. The work pending loop can call set_fs after addr_limit_user_check removed the _TIF_FSCHECK flag. This may happen at anytime based on how ARM handles alignment exceptions. It leads to an infinite loop condition. After discussion, it has been agreed that the generic approach is not tailored to the ARM architecture and any fix might not be complete. This patch will be replaced by an architecture specific implementation. The work flag approach will be kept for other architectures. Reported-by: Leonard Crestez Signed-off-by: Thomas Garnier --- arch/arm/include/asm/thread_info.h | 15 ++++++--------- arch/arm/include/asm/uaccess.h | 2 -- arch/arm/kernel/entry-common.S | 9 ++------- arch/arm/kernel/signal.c | 5 ----- 4 files changed, 8 insertions(+), 23 deletions(-) diff --git a/arch/arm/include/asm/thread_info.h b/arch/arm/include/asm/thread_info.h index 1d468b527b7b..776757d1604a 100644 --- a/arch/arm/include/asm/thread_info.h +++ b/arch/arm/include/asm/thread_info.h @@ -139,11 +139,10 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, #define TIF_NEED_RESCHED 1 /* rescheduling necessary */ #define TIF_NOTIFY_RESUME 2 /* callback before returning to user */ #define TIF_UPROBE 3 /* breakpointed or singlestepping */ -#define TIF_FSCHECK 4 /* Check FS is USER_DS on return */ -#define TIF_SYSCALL_TRACE 5 /* syscall trace active */ -#define TIF_SYSCALL_AUDIT 6 /* syscall auditing active */ -#define TIF_SYSCALL_TRACEPOINT 7 /* syscall tracepoint instrumentation */ -#define TIF_SECCOMP 8 /* seccomp syscall filtering active */ +#define TIF_SYSCALL_TRACE 4 /* syscall trace active */ +#define TIF_SYSCALL_AUDIT 5 /* syscall auditing active */ +#define TIF_SYSCALL_TRACEPOINT 6 /* syscall tracepoint instrumentation */ +#define TIF_SECCOMP 7 /* seccomp syscall filtering active */ #define TIF_NOHZ 12 /* in adaptive nohz mode */ #define TIF_USING_IWMMXT 17 @@ -154,7 +153,6 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, #define _TIF_NEED_RESCHED (1 << TIF_NEED_RESCHED) #define _TIF_NOTIFY_RESUME (1 << TIF_NOTIFY_RESUME) #define _TIF_UPROBE (1 << TIF_UPROBE) -#define _TIF_FSCHECK (1 << TIF_FSCHECK) #define _TIF_SYSCALL_TRACE (1 << TIF_SYSCALL_TRACE) #define _TIF_SYSCALL_AUDIT (1 << TIF_SYSCALL_AUDIT) #define _TIF_SYSCALL_TRACEPOINT (1 << TIF_SYSCALL_TRACEPOINT) @@ -168,9 +166,8 @@ extern int vfp_restore_user_hwstate(struct user_vfp __user *, /* * Change these and you break ASM code in entry-common.S */ -#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \ - _TIF_NOTIFY_RESUME | _TIF_UPROBE | \ - _TIF_FSCHECK) +#define _TIF_WORK_MASK (_TIF_NEED_RESCHED | _TIF_SIGPENDING | \ + _TIF_NOTIFY_RESUME | _TIF_UPROBE) #endif /* __KERNEL__ */ #endif /* __ASM_ARM_THREAD_INFO_H */ diff --git a/arch/arm/include/asm/uaccess.h b/arch/arm/include/asm/uaccess.h index 87936dd5d151..0bf2347495f1 100644 --- a/arch/arm/include/asm/uaccess.h +++ b/arch/arm/include/asm/uaccess.h @@ -70,8 +70,6 @@ static inline void set_fs(mm_segment_t fs) { current_thread_info()->addr_limit = fs; modify_domain(DOMAIN_KERNEL, fs ? DOMAIN_CLIENT : DOMAIN_MANAGER); - /* On user-mode return, check fs is correct */ - set_thread_flag(TIF_FSCHECK); } #define segment_eq(a, b) ((a) == (b)) diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index ca3614dc6938..0b60adf4a5d9 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -49,9 +49,7 @@ ret_fast_syscall: UNWIND(.cantunwind ) disable_irq_notrace @ disable interrupts ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing - tst r1, #_TIF_SYSCALL_WORK - bne fast_work_pending - tst r1, #_TIF_WORK_MASK + tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK bne fast_work_pending /* perform architecture specific actions before user return */ @@ -77,15 +75,12 @@ ret_fast_syscall: str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 disable_irq_notrace @ disable interrupts ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing - tst r1, #_TIF_SYSCALL_WORK - bne fast_work_pending - tst r1, #_TIF_WORK_MASK + tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK beq no_work_pending UNWIND(.fnend ) ENDPROC(ret_fast_syscall) /* Slower path - fall through to work_pending */ -fast_work_pending: #endif tst r1, #_TIF_SYSCALL_WORK diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c index e2de50bf8742..5814298ef0b7 100644 --- a/arch/arm/kernel/signal.c +++ b/arch/arm/kernel/signal.c @@ -14,7 +14,6 @@ #include #include #include -#include #include #include @@ -614,10 +613,6 @@ do_work_pending(struct pt_regs *regs, unsigned int thread_flags, int syscall) * Update the trace code with the current status. */ trace_hardirqs_off(); - - /* Check valid user FS if needed */ - addr_limit_user_check(); - do { if (likely(thread_flags & _TIF_NEED_RESCHED)) { schedule();