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 585E1C77B7D for ; Tue, 18 Apr 2023 10:11:07 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DCFD16B0072; Tue, 18 Apr 2023 06:11:06 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D7FC08E0002; Tue, 18 Apr 2023 06:11:06 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C475B8E0001; Tue, 18 Apr 2023 06:11:06 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id B62EA6B0072 for ; Tue, 18 Apr 2023 06:11:06 -0400 (EDT) Received: from smtpin07.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 4AF21ABD9A for ; Tue, 18 Apr 2023 10:11:06 +0000 (UTC) X-FDA: 80694093732.07.2BA62C6 Received: from mail-il1-f179.google.com (mail-il1-f179.google.com [209.85.166.179]) by imf28.hostedemail.com (Postfix) with ESMTP id 91081C000E for ; Tue, 18 Apr 2023 10:11:04 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=MmwB5ib1; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf28.hostedemail.com: domain of elver@google.com designates 209.85.166.179 as permitted sender) smtp.mailfrom=elver@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1681812664; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=qkfA+RN8+NxglUFBtFLBAQbfb++ZPOPFuIp98xxhha0=; b=h0nXvGUGyd7eo0GRMboewvMOXKgv49aXrM6n5QB8z5rG01o4Amvai5nPd74SZY0geAd1Qo NBPlaQm3B6VAfpigs8r/kq5dug37m3qfFN8SxoQq1ME9urz1NayCpk36wPAuAp3rMh3sQD SzQTweyL+MNnqYy4UhbiyB/N2mlJSuU= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=MmwB5ib1; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf28.hostedemail.com: domain of elver@google.com designates 209.85.166.179 as permitted sender) smtp.mailfrom=elver@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1681812664; a=rsa-sha256; cv=none; b=TUa4/F5BT2Byq8jJVIvryK9BxxpmtSJaSqmYDdnpE1LVZ0sm0ALyXlKUAkNyQ7qLx/DhKc TPZuYZBsYJUPtNhzwDfZO0prGqELfptZ08te2l2Ji4M3a9a5juim3GeafcX8r6zKR9JSXX KuEfcflUjVdFRaWObUw++vGVSP+Sr7Q= Received: by mail-il1-f179.google.com with SMTP id e9e14a558f8ab-32aabbe7e77so3291505ab.2 for ; Tue, 18 Apr 2023 03:11:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1681812663; x=1684404663; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=qkfA+RN8+NxglUFBtFLBAQbfb++ZPOPFuIp98xxhha0=; b=MmwB5ib1jiB1vveishoJODEjHtjcRFokuTQ+zoReKj4y6MHzILRvRxPxQikoUZ3gpg OzDLNgHEdo17gJTBx4c6NcHJYTqHtZkfFeJbNgY9nvxcxjDA1+D99cfiApUuBfECaNUW yn2RbQ1mGm/WgRlOmRkfOr0afQMeRiP80XJAPzYWduLXDm0+qOuS0RugqlO3FQbbKLMb NZPzmmHxLyRPXXJ3ixqfD9axn9QJlYi45/FUj9a6EqSBtCAth5OUUwqqStXgbx012Qb4 yCEmf8bMROeA78mVQh/64uLXqnxilN/Kguv4LTDZOHuEQ/RfCax5irsd+jHMI/Rq0Olc L+uA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681812663; x=1684404663; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=qkfA+RN8+NxglUFBtFLBAQbfb++ZPOPFuIp98xxhha0=; b=YettygJbS8HXFuDdERISV2fno1mmrC6p6nj49nxT7OtKOg//g7OskEIYwwTleoCw0A 2RsLHv8ECMfyBWoIJTUFckSa5Pl9CFT/tRtN4GjAgCeVrtSHBgKDnfxMUb5GPgRHHLK/ CagwELsrXtnmBQgnsvhDpJA3pbSlHW8tyKCDkax9N9OdlN8VDCejpgox29i9n9bwwnnG YmUCzoAo+wdSGGobevGOa9GknIVgknfdY4q8EVOrSiMR1XgSGBkbFqxYMFmQJMN0e5HZ ajnxwWMcYqPevEz+UFisz07EECVcyJr5akWcs9aIZeQN/i5z8IwRLVbyJyRXQUle+LLB IJZA== X-Gm-Message-State: AAQBX9fNtmh1IucOjJcZNKidnKrJ9l7IVQ9sIb2CGhQmhQZ4i6WfNn+Z SJQJ8s1ZKHdztK4VHUWXX+vE+WmCVnPE/zS2vTKvrQ== X-Google-Smtp-Source: AKy350aerQraOJZ/IlNOLQy+h6vs4wtRqD4ScpxSOOhcGpktvkaRORLvGcdmd2MNK01I92oGFDmBNJrDwPeRokzyzGs= X-Received: by 2002:a92:1301:0:b0:326:68bc:b423 with SMTP id 1-20020a921301000000b0032668bcb423mr12482145ilt.20.1681812663485; Tue, 18 Apr 2023 03:11:03 -0700 (PDT) MIME-Version: 1.0 References: <20230413131223.4135168-1-glider@google.com> <20230413131223.4135168-2-glider@google.com> In-Reply-To: <20230413131223.4135168-2-glider@google.com> From: Marco Elver Date: Tue, 18 Apr 2023 12:10:27 +0200 Message-ID: Subject: Re: [PATCH v2 2/4] mm: kmsan: handle alloc failures in kmsan_ioremap_page_range() To: Alexander Potapenko Cc: urezki@gmail.com, hch@infradead.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, dvyukov@google.com, kasan-dev@googlegroups.com, Dipanjan Das Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 91081C000E X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: bi3rs1unpfhsuzjhsocssfwyx7thshxy X-HE-Tag: 1681812664-464013 X-HE-Meta: U2FsdGVkX1/b2J2Mlkwku/cba5jqV2+uCNVIKlg9+b7ZVRR1GLixCd/bbTc0Y2ECeoxj+JPNKYWW6vgVAWyLIYIf3vL/UtoG/apBGmrw/AfPY6xx3EalbfCLYOlrKkg+uMrz7xOIdX/E4Tyy5TTnaZh0y1Bw0vkhadt/Tn0c7fbJ5T4CTzyqFrwaVsvxr8QEmLTod0skP/cxdoqJMWJ4ZePLiIc+qshDsCu2w0R/pkJNPAnphX0DHoNjdWIK/dHgvMT+5YiCZuMkuqStA8VzN1h9c4A5xPe0KTkfk5IWFD3AgkriEji5i4gxny8OAcIiUKwksUBVdhHPm0byxA0d2XR9Os6saLwMxK/Oiq+YTXfK1s7NpAo1fupBpprGurk+68rSmJkx779w5rSZs2g7k69i9n60kNXWpZNMbeLhFFVwGRfP6Vj39u5O5oSwVk44z8yrkGXJgqoJUvCHFLEH/YfO0Z8YxtusuWCJKU0GTpymvCRXUCr/QWclELZntUgHbK58cKFvZwp2U9apIY1Oheoluofzjg/ZYVL5/PBK8mjPkMg6pNz4oSmkVFqn7uxNJStKdFexCnkRVNAob/MY7eRHCx5EDuxG8oqpeyrripbGu0JM/cMFpbTs37471vuGAkHYIyWItN7uFw9B4UWBdjD/JQWlKI25RnQyLLnvCtbmpzVM5cNzXpGAYt2AuTeIE7i5CJQgFUFz1NtfjEZYLx8raeNJWqv+FO30dB6NgJECfhqBMoGvHoUP7WMP7wZlLRWavwyyjgV7EYW/G9RI+CvPYoIuJqodLHOOG3z4GlBkmycMq3A/iedcxnweAn/9yfXVdtu5OZZm4ZeT2lf/w6hjWZxOVzytKlXobD0wRojF5ygIKs8ysmvEROz+MDtq/HerIEcwnC/r8ulRvN8mcO5fgmJMFX6/m+0fmcv4sw0qSQjKWj1qtOTSJ8b1nhjXnS5SH7JgnfwQDy69Gj+ AtnqcV+g xo3x7KvkRCwUXfkxIEONGedFSU0bb0Uq85CXJ0nlYVg3U/YtZMLlwJ7Qfi0ocT7ENhRhGh06tjKwaSeudCz5k9paLe3RuNgclWB3VAJVZYC7aLMpldoLqtQ2pkOuSKRSJtcDX052JKcLp8Dot5FB7SDAkhHCeIa9B/SQ6Qpa6IJYHufDZAC1Wd9yuDNeLEj/yU1STHXCEXsTzR7hVlChRE4IMoSq3Y264o0Zhq3hwG4EnIxrpCj7p3HlYkf0WTDARpQLktGKYH3yTaU88l1GU6omjvqqlnTl1rizUgR2k3wDkD1ZcLPKp3uhbaak/WXsBpw1t6SCZxyTzl/6zUomMjNCJQc8LGocUyNt+PTt18lhdXHooYtEgy53MOSHKQLA8Oy/dG1rnV8JI6SyMkDogi5p77q7KdmmN6wUgV4zUXtBlfDBKt0NNPPJFx9bmh7iRG69WXci6Bryc5+5v2niFW/+eKCI+hqy0lcU0gtgIFN+ydX0= 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 Thu, 13 Apr 2023 at 15:12, Alexander Potapenko wrote: > > Similarly to kmsan_vmap_pages_range_noflush(), > kmsan_ioremap_page_range() must also properly handle allocation/mapping > failures. In the case of such, it must clean up the already created > metadata mappings and return an error code, so that the error can be > propagated to ioremap_page_range(). Without doing so, KMSAN may silently > fail to bring the metadata for the page range into a consistent state, > which will result in user-visible crashes when trying to access them. > > Reported-by: Dipanjan Das > Link: https://lore.kernel.org/linux-mm/CANX2M5ZRrRA64k0hOif02TjmY9kbbO2aCBPyq79es34RXZ=cAw@mail.gmail.com/ > Fixes: b073d7f8aee4 ("mm: kmsan: maintain KMSAN metadata for page operations") > Signed-off-by: Alexander Potapenko Reviewed-by: Marco Elver > --- > v2: > -- updated patch description as requested by Andrew Morton > -- check the return value of __vmap_pages_range_noflush(), as suggested by Dipanjan Das > -- return 0 from the inline version of kmsan_ioremap_page_range() > (spotted by kernel test robot ) > --- > include/linux/kmsan.h | 19 ++++++++------- > mm/kmsan/hooks.c | 55 ++++++++++++++++++++++++++++++++++++------- > mm/vmalloc.c | 4 ++-- > 3 files changed, 59 insertions(+), 19 deletions(-) > > diff --git a/include/linux/kmsan.h b/include/linux/kmsan.h > index c7ff3aefc5a13..30b17647ce3c7 100644 > --- a/include/linux/kmsan.h > +++ b/include/linux/kmsan.h > @@ -160,11 +160,12 @@ void kmsan_vunmap_range_noflush(unsigned long start, unsigned long end); > * @page_shift: page_shift argument passed to vmap_range_noflush(). > * > * KMSAN creates new metadata pages for the physical pages mapped into the > - * virtual memory. > + * virtual memory. Returns 0 on success, callers must check for non-zero return > + * value. > */ > -void kmsan_ioremap_page_range(unsigned long addr, unsigned long end, > - phys_addr_t phys_addr, pgprot_t prot, > - unsigned int page_shift); > +int kmsan_ioremap_page_range(unsigned long addr, unsigned long end, > + phys_addr_t phys_addr, pgprot_t prot, > + unsigned int page_shift); > > /** > * kmsan_iounmap_page_range() - Notify KMSAN about a iounmap_page_range() call. > @@ -296,12 +297,12 @@ static inline void kmsan_vunmap_range_noflush(unsigned long start, > { > } > > -static inline void kmsan_ioremap_page_range(unsigned long start, > - unsigned long end, > - phys_addr_t phys_addr, > - pgprot_t prot, > - unsigned int page_shift) > +static inline int kmsan_ioremap_page_range(unsigned long start, > + unsigned long end, > + phys_addr_t phys_addr, pgprot_t prot, > + unsigned int page_shift) > { > + return 0; > } > > static inline void kmsan_iounmap_page_range(unsigned long start, > diff --git a/mm/kmsan/hooks.c b/mm/kmsan/hooks.c > index 3807502766a3e..ec0da72e65aa0 100644 > --- a/mm/kmsan/hooks.c > +++ b/mm/kmsan/hooks.c > @@ -148,35 +148,74 @@ void kmsan_vunmap_range_noflush(unsigned long start, unsigned long end) > * into the virtual memory. If those physical pages already had shadow/origin, > * those are ignored. > */ > -void kmsan_ioremap_page_range(unsigned long start, unsigned long end, > - phys_addr_t phys_addr, pgprot_t prot, > - unsigned int page_shift) > +int kmsan_ioremap_page_range(unsigned long start, unsigned long end, > + phys_addr_t phys_addr, pgprot_t prot, > + unsigned int page_shift) > { > gfp_t gfp_mask = GFP_KERNEL | __GFP_ZERO; > struct page *shadow, *origin; > unsigned long off = 0; > - int nr; > + int nr, err = 0, clean = 0, mapped; > > if (!kmsan_enabled || kmsan_in_runtime()) > - return; > + return 0; > > nr = (end - start) / PAGE_SIZE; > kmsan_enter_runtime(); > - for (int i = 0; i < nr; i++, off += PAGE_SIZE) { > + for (int i = 0; i < nr; i++, off += PAGE_SIZE, clean = i) { > shadow = alloc_pages(gfp_mask, 1); > origin = alloc_pages(gfp_mask, 1); > - __vmap_pages_range_noflush( > + if (!shadow || !origin) { > + err = -ENOMEM; > + goto ret; > + } > + mapped = __vmap_pages_range_noflush( > vmalloc_shadow(start + off), > vmalloc_shadow(start + off + PAGE_SIZE), prot, &shadow, > PAGE_SHIFT); > - __vmap_pages_range_noflush( > + if (mapped) { > + err = mapped; > + goto ret; > + } > + shadow = NULL; > + mapped = __vmap_pages_range_noflush( > vmalloc_origin(start + off), > vmalloc_origin(start + off + PAGE_SIZE), prot, &origin, > PAGE_SHIFT); > + if (mapped) { > + __vunmap_range_noflush( > + vmalloc_shadow(start + off), > + vmalloc_shadow(start + off + PAGE_SIZE)); > + err = mapped; > + goto ret; > + } > + origin = NULL; > + } > + /* Page mapping loop finished normally, nothing to clean up. */ > + clean = 0; > + > +ret: > + if (clean > 0) { > + /* > + * Something went wrong. Clean up shadow/origin pages allocated > + * on the last loop iteration, then delete mappings created > + * during the previous iterations. > + */ > + if (shadow) > + __free_pages(shadow, 1); > + if (origin) > + __free_pages(origin, 1); > + __vunmap_range_noflush( > + vmalloc_shadow(start), > + vmalloc_shadow(start + clean * PAGE_SIZE)); > + __vunmap_range_noflush( > + vmalloc_origin(start), > + vmalloc_origin(start + clean * PAGE_SIZE)); > } > flush_cache_vmap(vmalloc_shadow(start), vmalloc_shadow(end)); > flush_cache_vmap(vmalloc_origin(start), vmalloc_origin(end)); > kmsan_leave_runtime(); > + return err; > } > > void kmsan_iounmap_page_range(unsigned long start, unsigned long end) > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 1355d95cce1ca..31ff782d368b0 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -313,8 +313,8 @@ int ioremap_page_range(unsigned long addr, unsigned long end, > ioremap_max_page_shift); > flush_cache_vmap(addr, end); > if (!err) > - kmsan_ioremap_page_range(addr, end, phys_addr, prot, > - ioremap_max_page_shift); > + err = kmsan_ioremap_page_range(addr, end, phys_addr, prot, > + ioremap_max_page_shift); > return err; > } > > -- > 2.40.0.577.gac1e443424-goog >