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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id E40C9C4332F for ; Mon, 7 Nov 2022 14:51:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 17DB16B0071; Mon, 7 Nov 2022 09:51:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 12E218E0001; Mon, 7 Nov 2022 09:51:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F41A96B0073; Mon, 7 Nov 2022 09:51:07 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id E4D956B0071 for ; Mon, 7 Nov 2022 09:51:07 -0500 (EST) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id C061CC027C for ; Mon, 7 Nov 2022 14:51:07 +0000 (UTC) X-FDA: 80106933774.10.9388383 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf02.hostedemail.com (Postfix) with ESMTP id 08C4C8000A for ; Mon, 7 Nov 2022 14:51:06 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 05DF5B81215; Mon, 7 Nov 2022 14:51:05 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id D361EC433D7; Mon, 7 Nov 2022 14:51:02 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1667832663; bh=RQF6DcpueeAn/U7ivkg2nCuDs++5yALpTWm0b1zzxGc=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=YgzYQ13JxuI90RZ1M7xc3VnCQqQN/crcFmQjiJeYd1Fw8obFllxKSq7FZh8RqXZ52 ygxmgve4PxQGecUVubnOQGZAhe/ei6FiT6j0ibRtvFy5DjxVF8gbv3cVjj0b9y9Rvd LLCpuGlDY8LIs74sHC44ywd9vzfdmdIB4dQz7t86Cnbq7rI7HvewqeTzFYiNb2mKAj JLelAQYvs29pe3Bi75zxDZqR5DDcDoFYX8gWYs2M8258mVgVkRvhQ4gpGR6vAIDpN9 l1UwFUgYeS60XMuVO5gFHpcROcKYUV8fVWKCB7T5Xmk8zcfCSkGVpUSYn45cJqJF0f 1D4+t202ieTvw== Message-ID: Date: Mon, 7 Nov 2022 06:50:51 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCHv11 05/16] x86/uaccess: Provide untagged_addr() and remove tags before address check Content-Language: en-US To: "Kirill A. Shutemov" , Dave Hansen , Peter Zijlstra Cc: x86@kernel.org, Kostya Serebryany , Andrey Ryabinin , Andrey Konovalov , Alexander Potapenko , Taras Madan , Dmitry Vyukov , "H . J . Lu" , Andi Kleen , Rick Edgecombe , Bharata B Rao , Jacob Pan , Ashok Raj , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20221025001722.17466-1-kirill.shutemov@linux.intel.com> <20221025001722.17466-6-kirill.shutemov@linux.intel.com> From: Andy Lutomirski In-Reply-To: <20221025001722.17466-6-kirill.shutemov@linux.intel.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1667832667; a=rsa-sha256; cv=none; b=BTKHgI7DwcNmmu71QYb3BPlOw9UzyJ1GLEl2qmCEk2mKv24B2cfFKHGcjhbUUvft37/E4j OT0vjQrUtWOvQ2+rLS3UFrPdslmnGqn0ce+llF8npp4anI3Si9tAIpcD0yPvWWcCJWty8B u2f/gEfouIwH/vk6uLaEjmUy36JKTX4= ARC-Authentication-Results: i=1; imf02.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=YgzYQ13J; spf=pass (imf02.hostedemail.com: domain of luto@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=luto@kernel.org; dmarc=pass (policy=none) header.from=kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1667832667; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=dF7JPGaNeMuPH6wJXQ7t82Ybiq8JSa1ne5G/OzypJsk=; b=Erh+Yig52zUpL1yvHfrGcdHG9m7QwaXfAV+j31n20TLczTiXPQyXoc8Td0mUXxc910VYqL jkWKgmAxy9LXNEBS5eAI2JPrGhCUdszDBJv1dGa9kER6OXFqyzsv2H+L4RQte1R9kEiVDd fDaRuxN+85jKXf2d4WpNGIJqiozRn7c= X-Stat-Signature: frjb5dk3xjta6k4w7asssf57px9z9jqb X-Rspam-User: Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=YgzYQ13J; spf=pass (imf02.hostedemail.com: domain of luto@kernel.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=luto@kernel.org; dmarc=pass (policy=none) header.from=kernel.org X-Rspamd-Queue-Id: 08C4C8000A X-Rspamd-Server: rspam09 X-HE-Tag: 1667832666-86453 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On 10/24/22 17:17, Kirill A. Shutemov wrote: > untagged_addr() is a helper used by the core-mm to strip tag bits and > get the address to the canonical shape. In only handles userspace > addresses. The untagging mask is stored in mmu_context and will be set > on enabling LAM for the process. > > The tags must not be included into check whether it's okay to access the > userspace address. > > Strip tags in access_ok(). > > get_user() and put_user() don't use access_ok(), but check access > against TASK_SIZE directly in assembly. Strip tags, before calling into > the assembly helper. > > Signed-off-by: Kirill A. Shutemov > Tested-by: Alexander Potapenko > Acked-by: Peter Zijlstra (Intel) > --- > arch/x86/include/asm/mmu.h | 3 +++ > arch/x86/include/asm/mmu_context.h | 11 ++++++++ > arch/x86/include/asm/uaccess.h | 42 +++++++++++++++++++++++++++--- > arch/x86/kernel/process.c | 3 +++ > 4 files changed, 56 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/include/asm/mmu.h b/arch/x86/include/asm/mmu.h > index 002889ca8978..2fdb390040b5 100644 > --- a/arch/x86/include/asm/mmu.h > +++ b/arch/x86/include/asm/mmu.h > @@ -43,6 +43,9 @@ typedef struct { > > /* Active LAM mode: X86_CR3_LAM_U48 or X86_CR3_LAM_U57 or 0 (disabled) */ > unsigned long lam_cr3_mask; > + > + /* Significant bits of the virtual address. Excludes tag bits. */ > + u64 untag_mask; > #endif > > struct mutex lock; > diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h > index 69c943b2ae90..5bd3d46685dc 100644 > --- a/arch/x86/include/asm/mmu_context.h > +++ b/arch/x86/include/asm/mmu_context.h > @@ -100,6 +100,12 @@ static inline unsigned long mm_lam_cr3_mask(struct mm_struct *mm) > static inline void dup_lam(struct mm_struct *oldmm, struct mm_struct *mm) > { > mm->context.lam_cr3_mask = oldmm->context.lam_cr3_mask; > + mm->context.untag_mask = oldmm->context.untag_mask; > +} > + > +static inline void mm_reset_untag_mask(struct mm_struct *mm) > +{ > + mm->context.untag_mask = -1UL; > } > > #else > @@ -112,6 +118,10 @@ static inline unsigned long mm_lam_cr3_mask(struct mm_struct *mm) > static inline void dup_lam(struct mm_struct *oldmm, struct mm_struct *mm) > { > } > + > +static inline void mm_reset_untag_mask(struct mm_struct *mm) > +{ > +} > #endif > > #define enter_lazy_tlb enter_lazy_tlb > @@ -138,6 +148,7 @@ static inline int init_new_context(struct task_struct *tsk, > mm->context.execute_only_pkey = -1; > } > #endif > + mm_reset_untag_mask(mm); > init_new_context_ldt(mm); > return 0; > } > diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h > index 8bc614cfe21b..c6062c07ccd2 100644 > --- a/arch/x86/include/asm/uaccess.h > +++ b/arch/x86/include/asm/uaccess.h > @@ -7,6 +7,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -21,6 +22,30 @@ static inline bool pagefault_disabled(void); > # define WARN_ON_IN_IRQ() > #endif > > +#ifdef CONFIG_X86_64 > +/* > + * Mask out tag bits from the address. > + * > + * Magic with the 'sign' allows to untag userspace pointer without any branches > + * while leaving kernel addresses intact. > + */ > +#define untagged_addr(mm, addr) ({ \ > + u64 __addr = (__force u64)(addr); \ > + s64 sign = (s64)__addr >> 63; \ > + __addr &= (mm)->context.untag_mask | sign; \ > + (__force __typeof__(addr))__addr; \ > +}) > + I think this implementation is correct, but I'm wondering if there are any callers of untagged_addr that actually need to preserve kernel addresses. Are there? (There certainly *were* back when we had set_fs().) I'm also mildly uneasy about a potential edge case. Naively, one would expect: untagged_addr(current->mm, addr) + size == untagged_addr(current->mm, addr + size) at least for an address that is valid enough to be potentially dereferenced. This isn't true any more for size that overflows into the tag bit range. I *think* we're okay though -- __access_ok requires that addr <= limit - size, so any range that overflows into tag bits will be rejected even if the entire range consists of valid (tagged) user addresses. So: Acked-by: Andy Lutomirski