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 7DF24C3DA7F for ; Mon, 12 Aug 2024 23:34:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 02A376B008C; Mon, 12 Aug 2024 19:34:29 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F1C886B0098; Mon, 12 Aug 2024 19:34:28 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id DE3686B009A; Mon, 12 Aug 2024 19:34:28 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id C06786B008C for ; Mon, 12 Aug 2024 19:34:28 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 628E1404B7 for ; Mon, 12 Aug 2024 23:34:28 +0000 (UTC) X-FDA: 82445199816.04.45346DF Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf15.hostedemail.com (Postfix) with ESMTP id A38BCA001A for ; Mon, 12 Aug 2024 23:34:25 +0000 (UTC) Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ArMMrm6K; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf15.hostedemail.com: domain of bhe@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bhe@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1723505614; a=rsa-sha256; cv=none; b=Z4bg2yCfMiRr6fZIBYhehPb6vilvtwvU7XKYEjLJ2AojmL88ywvMPh4JcOw6pwod0QPQPq o5sLnxgP044Hmu/p3h4474Tmaw/Qh4AZrSo66eLdsPTfP5BwEG7mUeV4BR5ERrl7fxpid4 oLoqQL5JYRoP4korN7miK3Ldeb6eFWk= ARC-Authentication-Results: i=1; imf15.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=ArMMrm6K; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf15.hostedemail.com: domain of bhe@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bhe@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1723505614; 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=xKDbGaTX8EZpcenHgPa8veNn1wKEPpwdIKATGSiYtC4=; b=1O5iirDQaWhQ73mJ3OgCQxgK12vochqedT+Ttr6kHXbsl9VI/wHR4IhdOR0wDmBh6/b9T9 vvLC28MeIy1KrwTp2zoS71jEaRAoss3yVLpuhJYaX/6D47FFmmTwbSDk+LqgdR4XoXpIVL WdcxZA99j2vfRm1MhVODA9Zi1kXNFDg= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1723505665; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=xKDbGaTX8EZpcenHgPa8veNn1wKEPpwdIKATGSiYtC4=; b=ArMMrm6K94B4o61dKvWqdOtZ8jrhdHVOzbtNOUaecV+Ei7nhvRVz4DaxEYob6n112W5Iyz 0BA8RWJmVk3Hv6soQCTFFR047g7Byjq7E3QMB7iFuO/P6+kG/HDGQh98ZJrP8gJrybR88h fQH3dYqVg6fyEQH0WPMJZdz+Oqxh0Gc= Received: from mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-367-9hhukabLNNaaRmDZhHsE1w-1; Mon, 12 Aug 2024 19:34:18 -0400 X-MC-Unique: 9hhukabLNNaaRmDZhHsE1w-1 Received: from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 5E4DC1955F45; Mon, 12 Aug 2024 23:34:16 +0000 (UTC) Received: from localhost (unknown [10.72.112.25]) by mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 8B6CB30001A1; Mon, 12 Aug 2024 23:34:14 +0000 (UTC) Date: Tue, 13 Aug 2024 07:34:10 +0800 From: Baoquan He To: Will Deacon Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org, Zhaoyang Huang , "Hailong . Liu" , Uladzislau Rezki , Christoph Hellwig , Lorenzo Stoakes , Thomas Gleixner , Andrew Morton , stable@vger.kernel.org Subject: Re: [PATCH] mm: vmalloc: Ensure vmap_block is initialised before adding to queue Message-ID: References: <20240812171606.17486-1-will@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240812171606.17486-1-will@kernel.org> X-Scanned-By: MIMEDefang 3.4.1 on 10.30.177.4 X-Rspamd-Queue-Id: A38BCA001A X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: srwqqz6ip6nfoe83rzdtztozhtswf9b8 X-HE-Tag: 1723505665-213027 X-HE-Meta: U2FsdGVkX18IM+I8Y3xJs1VS1RYisjRLiRXQE144mHB/3SPQbIXMsp7P1jvpcfeL0LBmIH6o4Uwrz1A+iAQOz2RVQEuQWZQMtar4jy8/yGgRjIfWVenziyvcWUDK4kFy7ot0miORccIAJZzfMiYF6t2NElssN8Mzj7QMY9U8rN7jQPe35y+2w06k5PPrC+/ynIJThXcZzHYI6XXvUgauKZyfa0Wzwi40qEJIEIUDj3AqhXH+sTB3kF19ESiGv0vl8o7o9BkwIbAynrDEwps5kpbjXi2bWjk3LToti1jYjneoUy021i03RNL6NCZ035b2jxqMzi0IurtQMDf6HRObaDYEE9Zu8EYvkYwN9Dc5DxosaL/XOMOEe5zyPKlEKoZsHxQnyJvtXvMSqotzG/lxADGRqpfYd7d5+udK4ArV505rPXqaAlnK5FF+fkuz/DWdNHd/GWYqdBb+688xsLSUcDQtybyUzdZM/nTS1WZpCGazZuv+1nH0Th+N2ftekrfxDrASHSU+VkYdECAAoPc8nsUoBskp1s0SmopC88kP0zsK/vsXGKfM25DP699TO4Rnn1L57bPPdF2dv2fXLnFOmdrGv9XTV2KFiuDMwohOMxIUtGZa+23IAa2mDlaOENHsVe32R/GecbgICpskv5OUaAcglqhbu1bKGL0d2JJaOpjfZSO+bHWcWXjH++TLwL2zu8YZ2dYbMYtVf0rybpW5cHq4tXl76WJi4vDQzarMscd0yAfO39Hf5exB73d+afQujruoLO2ljF9RK42h4gjECwY8TasJ2HFVU3hT5rC0Wm9aSul6f9BnRtZqlEXD9UBfjXW5kuEz8SCSr8QRGjl84t0WthFA46a06d62t2kImOhH8cOuk5TcsZZcFGVOKZddGt+z5kl2e/QrGDG/2Cr4x+zAkV4mARk1BwaQtZWicFVqrR16MBxJaEQ5HZweDOUI9s3NFzbIfH+LsVSigmv EIqBtsWm GA+tt6hLqD7zq7OeW+g9NGDjSXNbM+/Y3Gp8q270BnBzoZl7FeQPI/oVIrcICxJZEIGMseFLzsOvxqFmraT50BATg/bkRTZUAszlwaDF4zP5jL4cNMd9IxS8jkjIU07ucnl5O3nPg08Ib/f+bACCGXGt3BsPY7qw1dlL8Unkoso/LBlUSg6JwKlW0+KF5LG0X7SeKKSv/rZt4OQIR8aXQ0mc95DM7gpMjZGOAwBC7Z5FOTCS5d+hdCCUNwWzlNSVDQF/8R/0AJhYWDwQhS0e11evngM3QrdZntkCXFS2RAiWnay2zG8zdX3O7bOe0P35/l9m+b8rIGYr8oDl/AHnN2BD+BcqSooDi3J/KI4wXpdMIjy2Og/cmpetleu0XlE8s2/RBlIvC5jjviSgSJnGTW4uwWwNLThujdBUTJUhFgPb7J5aKwO9g/rLdAJhLjwjJ4YgN3witl2DNVKc9t2RiNr/JNchIdQqYEYVo4Lx4Ex/uTdTLLlfKLXeYndWcv42HJEbc+iQGqwNDbRpnc0+/huoAKoidnXb+pl/cf7sqUpEPxQjDynqZvhXQu7dwNQzdL7w0mXRQ27eC+RRlkaH3WPf4zAI2Lxg/68H65cUHOu74j5JUwvqDE0f/Kgqa8xxyvCPDLlQc1EHu8Kci8r4lQeU28eLHYMr/9K0ABrzDoHN5ZQIrNAFYMZKdMcvc4yj9w2pSbUdsbaouJApohfoI44VAeg== 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: List-Subscribe: List-Unsubscribe: On 08/12/24 at 06:16pm, Will Deacon wrote: > Commit 8c61291fd850 ("mm: fix incorrect vbq reference in > purge_fragmented_block") extended the 'vmap_block' structure to contain > a 'cpu' field which is set at allocation time to the id of the > initialising CPU. > > When a new 'vmap_block' is being instantiated by new_vmap_block(), the > partially initialised structure is added to the local 'vmap_block_queue' > xarray before the 'cpu' field has been initialised. If another CPU is > concurrently walking the xarray (e.g. via vm_unmap_aliases()), then it > may perform an out-of-bounds access to the remote queue thanks to an > uninitialised index. > > This has been observed as UBSAN errors in Android: > > | Internal error: UBSAN: array index out of bounds: 00000000f2005512 [#1] PREEMPT SMP > | > | Call trace: > | purge_fragmented_block+0x204/0x21c > | _vm_unmap_aliases+0x170/0x378 > | vm_unmap_aliases+0x1c/0x28 > | change_memory_common+0x1dc/0x26c > | set_memory_ro+0x18/0x24 > | module_enable_ro+0x98/0x238 > | do_init_module+0x1b0/0x310 > > Move the initialisation of 'vb->cpu' in new_vmap_block() ahead of the > addition to the xarray. > > Cc: Zhaoyang Huang > Cc: Hailong.Liu > Cc: Uladzislau Rezki (Sony) > Cc: Baoquan He > Cc: Christoph Hellwig > Cc: Lorenzo Stoakes > Cc: Thomas Gleixner > Cc: Andrew Morton > Cc: > Fixes: 8c61291fd850 ("mm: fix incorrect vbq reference in purge_fragmented_block") > Signed-off-by: Will Deacon > --- Good catch, this truly could happen and collapse system. Reviewed-by: Baoquan He > > I _think_ the insertion into the free list is ok, as the vb shouldn't be > considered for purging if it's clean. It would be great if somebody more > familiar with this code could confirm either way, however. It's OK, please see below comment. static void *new_vmap_block(unsigned int order, gfp_t gfp_mask) { ...... vaddr = vmap_block_vaddr(va->va_start, 0); spin_lock_init(&vb->lock); vb->va = va; /* At least something should be left free */ BUG_ON(VMAP_BBMAP_BITS <= (1UL << order)); bitmap_zero(vb->used_map, VMAP_BBMAP_BITS); vb->free = VMAP_BBMAP_BITS - (1UL << order); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Here we have cut away one piece according to vb_alloc() and set vb->free. vb->dirty = 0; vb->dirty_min = VMAP_BBMAP_BITS; vb->dirty_max = 0; bitmap_set(vb->used_map, 0, (1UL << order)); INIT_LIST_HEAD(&vb->free_list); ... } static bool purge_fragmented_block(struct vmap_block *vb, struct list_head *purge_list, bool force_purge) { struct vmap_block_queue *vbq = &per_cpu(vmap_block_queue, vb->cpu); if (vb->free + vb->dirty != VMAP_BBMAP_BITS || ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The above setting of vb->free and vb->dirty will fail conditional check here. So it won't be purged. vb->dirty == VMAP_BBMAP_BITS) return false; /* Don't overeagerly purge usable blocks unless requested */ if (!(force_purge || vb->free < VMAP_PURGE_THRESHOLD)) return false; ... } > > mm/vmalloc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 6b783baf12a1..64c0a2c8a73c 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -2626,6 +2626,7 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask) > vb->dirty_max = 0; > bitmap_set(vb->used_map, 0, (1UL << order)); > INIT_LIST_HEAD(&vb->free_list); > + vb->cpu = raw_smp_processor_id(); > > xa = addr_to_vb_xa(va->va_start); > vb_idx = addr_to_vb_idx(va->va_start); > @@ -2642,7 +2643,6 @@ static void *new_vmap_block(unsigned int order, gfp_t gfp_mask) > * integrity together with list_for_each_rcu from read > * side. > */ > - vb->cpu = raw_smp_processor_id(); > vbq = per_cpu_ptr(&vmap_block_queue, vb->cpu); > spin_lock(&vbq->lock); > list_add_tail_rcu(&vb->free_list, &vbq->free); > -- > 2.46.0.76.ge559c4bf1a-goog >