From patchwork Mon Aug 5 12:52:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jann Horn X-Patchwork-Id: 13753580 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 3C1EEC3DA4A for ; Mon, 5 Aug 2024 12:52:17 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id C579A6B008A; Mon, 5 Aug 2024 08:52:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C071D6B008C; Mon, 5 Aug 2024 08:52:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id AA7166B0092; Mon, 5 Aug 2024 08:52:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 8BBF56B008A for ; Mon, 5 Aug 2024 08:52:16 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 35729A029C for ; Mon, 5 Aug 2024 12:52:16 +0000 (UTC) X-FDA: 82418179872.24.2120686 Received: from mail-ed1-f51.google.com (mail-ed1-f51.google.com [209.85.208.51]) by imf13.hostedemail.com (Postfix) with ESMTP id 37AC120010 for ; Mon, 5 Aug 2024 12:52:13 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UE2JPnvi; spf=pass (imf13.hostedemail.com: domain of jannh@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1722862272; 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: references:dkim-signature; bh=iFF3dav9yqLjo0TF+kMn3kLYM5TWdwyUTbXMx4bI3W4=; b=TPwef2YW33In4qt8jN5QRHtOmHZDoPHdtl1Pc4zMo78crgiqo9V11htokpAzDU+zS3TCtq u4e+zWMJwZJ+3fqaE5VD2g7fVZo7lQlzkRWPsx5YTZK908f2eHnBij1uG+0tW4HYeY0Knq FWUNyQ/IeAogf0LNFB18cyTt+s/4UT8= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1722862272; a=rsa-sha256; cv=none; b=A+YIqrwLi09y932/TzMVcAA7KqYRjTr3ummd8wV0Zmy8/BanBCTEEK+WrN8tbCFA97H8/w jOIfXuHhGwWBitDs5cNv1XIo0XZLUWehGvz5++UAsZaYYisdsVsk2vecKwbeLsHoeBHVf5 +ocs7/nMaCd7xHqbNDhQaA4/Fi8zRPE= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=UE2JPnvi; spf=pass (imf13.hostedemail.com: domain of jannh@google.com designates 209.85.208.51 as permitted sender) smtp.mailfrom=jannh@google.com; dmarc=pass (policy=reject) header.from=google.com Received: by mail-ed1-f51.google.com with SMTP id 4fb4d7f45d1cf-5b9fe5ea355so12255a12.0 for ; Mon, 05 Aug 2024 05:52:13 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1722862332; x=1723467132; darn=kvack.org; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:from:to:cc:subject:date:message-id:reply-to; bh=iFF3dav9yqLjo0TF+kMn3kLYM5TWdwyUTbXMx4bI3W4=; b=UE2JPnvi246ALTqd4I7EC9DLpxJT0CiNFFz4bhUeIZclNUF6Pv9qw2hhUGeKZ3yurq TfzsRo1JRxBpSwunDwJcxhbTlRDM6OXDxqRi+GSP8v2LnONuIBHH/WXK3/E6wznfN7Md z5U/FSz0RGQs0ZErcj7sT+zPGowKQqLZ4jHP+RduLdk+cfe7V4QdgyAK/wWrExaB1M/g evX4QqC7d8YAx8obBrLXHyiEcp1AnQ8wVXgfdT9Gqj/2ytMFkuTjG+AgIQx5vE1B7++D 2KS4KDPXRwMUgt2hwByAnQ1YviNnDQmm5P9zbejyLWdJcbMKytVI5YOYUteLegjEQD+y HEQw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1722862332; x=1723467132; h=cc:to:message-id:content-transfer-encoding:mime-version:subject :date:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=iFF3dav9yqLjo0TF+kMn3kLYM5TWdwyUTbXMx4bI3W4=; b=Ll4AO4hmE2XNAYXcTUBTCGyrWSzYCxgWgd67kSgMXW0pc/AOVdb9dteCPJT0InnEh7 KcY1MCd6CFN7huCKBVa8J2DYruX9anGfLWePLWSZKDWVZanEubKOf3WiohMWtKYTItzo 0bc2Xlo7HHqkyGDc2SAbn6aSRLK1ATJAcA4FcO9ltYFAh/rLnY6WY8Of1NT8Au2EmIPq lWbx400dEttyNP39+gpsPOXzMKJxFQD7x3dB1dlMMw97pk4MpC1EmBbLAtfItns/42dr jK9Ps18h8O40C0Qkdrw5b/YOKK9hdz+6mzG5L7OwiJPzNG/r0TyXeV4yj3YqymyC2ZSb 3hKg== X-Forwarded-Encrypted: i=1; AJvYcCVM7yDvguLbFexYuDuBqd9F0XzusQfdow5ignV+3v29S98Um5ljND5AOdwCOLA0Bhu1/WkfZk2z0b0b4So0xMZ7t4o= X-Gm-Message-State: AOJu0YyIrLXLfAwE8J0zTPA2KijQHkOjPf9074SQPzjRNwSz477zkymJ hOfKTBP84ZVmfxNr02+2VrNjGwGgIy3/5w/x2JwhVezuwuf9ZI7sMhXFZDDt/Q== X-Google-Smtp-Source: AGHT+IG4/26WzDopprFPE7meuJ2PeY5a6Pyu8HGXYYV2gD7rPHmYXMwsFQ95W3PHgu9F8FoWIi0JoA== X-Received: by 2002:a05:6402:2684:b0:5aa:19b1:ffc7 with SMTP id 4fb4d7f45d1cf-5b9c4a18619mr248206a12.2.1722862331691; Mon, 05 Aug 2024 05:52:11 -0700 (PDT) Received: from localhost ([2a00:79e0:9d:4:ca74:8a49:a6f6:b872]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-36bbd05b92esm9932069f8f.85.2024.08.05.05.52.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Aug 2024 05:52:11 -0700 (PDT) From: Jann Horn Date: Mon, 05 Aug 2024 14:52:03 +0200 Subject: [PATCH] mm: fix (harmless) type confusion in lock_vma_under_rcu() MIME-Version: 1.0 Message-Id: <20240805-fix-vma-lock-type-confusion-v1-1-9f25443a9a71@google.com> X-B4-Tracking: v=1; b=H4sIAPPKsGYC/x2MWw5AMBAAryL7bZN6BlcRH0272KCVFiHi7srnJ DNzgyfH5KGJbnB0sGdrAiRxBGqUZiBkHRhSkeaiEgX2fOKxSJytmnC7VkJlTb9/HQpZF6WudZZ IgnBYHQX9v7fd87wlDPD0bQAAAA== To: Andrew Morton , Suren Baghdasaryan Cc: Matthew Wilcox , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Jann Horn X-Mailer: b4 0.15-dev X-Developer-Signature: v=1; a=ed25519-sha256; t=1722862327; l=5226; i=jannh@google.com; s=20240730; h=from:subject:message-id; bh=MWjl5CHOtPJ+y+F3Ajk9ABVkQ1dUm6I4TJr96LZ3VDg=; b=TWd/79YsLdDZZKgn1Jz6osa0+ZPcWmB2/fiJKUY5M99IObxmRNEi05oVj7U30iEKYVBXTtGQ4 8VjaRetHUt8BF5g0WHqWgJqczrOPR7vixIhO+L4L2ybGirAof1h1xNw X-Developer-Key: i=jannh@google.com; a=ed25519; pk=AljNtGOzXeF6khBXDJVVvwSEkVDGnnZZYqfWhP1V+C8= X-Stat-Signature: jxfzpcdruxkpus9d6xojr6gik8ctfmfq X-Rspamd-Queue-Id: 37AC120010 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1722862333-160864 X-HE-Meta: U2FsdGVkX1+V/NP/IWVCppr38es+zv4SZpISXKFV6mz9R9sGJV7P00vCNjr0/w8TSzxO+W9YR/tytCECiTTZ7hqh/ytJmC3NFSPnOUJ2V5KMcqDK439mgreFaa8smEDBQSRR7YagOgZXlJw9pZd/Yui5Bbblm3u+/Irns7tDXB2SOGyA7zxY0pafyrgou/92GYiUNfnBDWTDR8G0kghaPS2KcQfVFldIpdNWtuNnTT2UhdxSIdDVhvPYX6dmUnLZ1iozVO0ApAf67T2TlYBxNkJYFcsNpYOHjXYhV3VsiPcUr5jqn4pZFiqERmQNlVbKEt8GlrTWeti6kSizaG+IgGNuprxI74PcyLx4kaO9wG/+n0jN2kIHiPl6ZxjLy2FlRRlffd/bopX6tJlPnX+3ufoaET7J0vyVaKXcJyzF4hHIJkx4HneTjD6/jN9y8F0RhUaOlRapvcuq9v+fCy40t6UBRY18TAJsrXGK3pCFFoO1M0eh+yYqV5t3MwxVeW2/7qwK59BHUjdy8oM7jNYwmuP6x7EGycf1xD2T9DENhAozRKvT09CQ+Vmw7tCNngueU9FN1/V99oCJvgjuQxkzBznVo5rY+mDpnUfkXfhVjqFQDlMDluX9rAS+87T0WsElLXAcXPVN3CFAJutSyt7bQQCVE0S7kOZ7rOBOe6OxPtO9PCDseYAq4x53CiaWQrSNi5kBsiOP0qsew2PsqIoSFUu54w/DJzhE9dlm9OtCESn8BokA0nayQxusXLh8Zn5pmyKKAFBhEa6/LMe0FT5gyiX4cw+YLuKIeMeohU4rbIZuOIovFLqrDd545BzxCrwBIZ1p5R8g8M65VgLJwy4wR/J3ZxI33UvHimaSssRu6a1VPMWImWB+lv00OIVg+Q5Ut/W4zRQfwEKrq+i82IVgF+e36a6qXJwZueFzZLwumUvIAFpZ3iwWkvbzlWLk/R/MaNlAuS6MLWiCE/JsIgM imaEOJIJ LbwsVnv9eIISjyZ6gvYgUy6RxGr2bqrgZiJJcPHMEGJTTX6yX+fXz2D/t9+xSjCJd/7dryuvArvSW8ZcC0cQWo/Nnt1swF2t980K8cpVnQDamqQEEgHZW8zzz/0pT6q0WBA39d5wvGJjPGHR9Dgr3BEIwMAn//+PlLLkN4SvTC4/whkQwi3IRDVaCjF6fQ2AuVY1M7lSfNY4ZuiICP7y34bbNUtSMCNEW/WqCIvQ22sh2U4WPK81zqiD7Co8Ek5benIH3Rhl1i3Ah/ZLv9niYaCSoWIzwrs/q/TC5slwN93HX8iHXrJcq5xZPyru8luQ9D3pePlpmE1GwXfY5dmCPp5VK9A== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000018, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: There is a (harmless) type confusion in lock_vma_under_rcu(): After vma_start_read(), we have taken the VMA lock but don't know yet whether the VMA has already been detached and scheduled for RCU freeing. At this point, ->vm_start and ->vm_end are accessed. vm_area_struct contains a union such that ->vm_rcu uses the same memory as ->vm_start and ->vm_end; so accessing ->vm_start and ->vm_end of a detached VMA is illegal and leads to type confusion between union members. Fix it by reordering the vma->detached check above the address checks, and document the rules for RCU readers accessing VMAs. This will probably change the number of observed VMA_LOCK_MISS events (since previously, trying to access a detached VMA whose ->vm_rcu has been scheduled would bail out when checking the fault address against the rcu_head members reinterpreted as VMA bounds). Fixes: 50ee32537206 ("mm: introduce lock_vma_under_rcu to be used from arch-specific code") Signed-off-by: Jann Horn Acked-by: Suren Baghdasaryan --- sidenote: I'm not entirely sure why we handle detached VMAs and moved VMAs differently here (detached VMAs retry, moved VMAs bail out), but that's kinda out of scope of this patch. --- include/linux/mm_types.h | 15 +++++++++++++-- mm/memory.c | 14 ++++++++++---- 2 files changed, 23 insertions(+), 6 deletions(-) --- base-commit: de9c2c66ad8e787abec7c9d7eff4f8c3cdd28aed change-id: 20240805-fix-vma-lock-type-confusion-0a956d9d31ae diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 485424979254..498cdf3121d0 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -657,58 +657,69 @@ struct vma_numab_state { /* * This struct describes a virtual memory area. There is one of these * per VM-area/task. A VM area is any part of the process virtual memory * space that has a special rule for the page-fault handlers (ie a shared * library, the executable area etc). + * + * Only explicitly marked struct members may be accessed by RCU readers before + * getting a stable reference. */ struct vm_area_struct { /* The first cache line has the info for VMA tree walking. */ union { struct { /* VMA covers [vm_start; vm_end) addresses within mm */ unsigned long vm_start; unsigned long vm_end; }; #ifdef CONFIG_PER_VMA_LOCK struct rcu_head vm_rcu; /* Used for deferred freeing. */ #endif }; - struct mm_struct *vm_mm; /* The address space we belong to. */ + /* + * The address space we belong to. + * Unstable RCU readers are allowed to read this. + */ + struct mm_struct *vm_mm; pgprot_t vm_page_prot; /* Access permissions of this VMA. */ /* * Flags, see mm.h. * To modify use vm_flags_{init|reset|set|clear|mod} functions. */ union { const vm_flags_t vm_flags; vm_flags_t __private __vm_flags; }; #ifdef CONFIG_PER_VMA_LOCK - /* Flag to indicate areas detached from the mm->mm_mt tree */ + /* + * Flag to indicate areas detached from the mm->mm_mt tree. + * Unstable RCU readers are allowed to read this. + */ bool detached; /* * Can only be written (using WRITE_ONCE()) while holding both: * - mmap_lock (in write mode) * - vm_lock->lock (in write mode) * Can be read reliably while holding one of: * - mmap_lock (in read or write mode) * - vm_lock->lock (in read or write mode) * Can be read unreliably (using READ_ONCE()) for pessimistic bailout * while holding nothing (except RCU to keep the VMA struct allocated). * * This sequence counter is explicitly allowed to overflow; sequence * counter reuse can only lead to occasional unnecessary use of the * slowpath. */ int vm_lock_seq; + /* Unstable RCU readers are allowed to read this. */ struct vma_lock *vm_lock; #endif /* * For areas with an address space and backing store, * linkage into the address_space->i_mmap interval tree. diff --git a/mm/memory.c b/mm/memory.c index 34f8402d2046..3f4232b985a1 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -5996,23 +5996,29 @@ struct vm_area_struct *lock_vma_under_rcu(struct mm_struct *mm, if (!vma) goto inval; if (!vma_start_read(vma)) goto inval; - /* Check since vm_start/vm_end might change before we lock the VMA */ - if (unlikely(address < vma->vm_start || address >= vma->vm_end)) - goto inval_end_read; - /* Check if the VMA got isolated after we found it */ if (vma->detached) { vma_end_read(vma); count_vm_vma_lock_event(VMA_LOCK_MISS); /* The area was replaced with another one */ goto retry; } + /* + * At this point, we have a stable reference to a VMA: The VMA is + * locked and we know it hasn't already been isolated. + * From here on, we can access the VMA without worrying about which + * fields are accessible for RCU readers. + */ + + /* Check since vm_start/vm_end might change before we lock the VMA */ + if (unlikely(address < vma->vm_start || address >= vma->vm_end)) + goto inval_end_read; rcu_read_unlock(); return vma; inval_end_read: vma_end_read(vma);