From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932175AbdBIP3h (ORCPT ); Thu, 9 Feb 2017 10:29:37 -0500 Received: from shadbolt.e.decadent.org.uk ([88.96.1.126]:60770 "EHLO shadbolt.e.decadent.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751519AbdBIP3T (ORCPT ); Thu, 9 Feb 2017 10:29:19 -0500 Message-ID: <1486653999.2900.63.camel@decadent.org.uk> Subject: Re: [PATCH 4.4 05/48] mm: fix devm_memremap_pages crash, use mem_hotplug_{begin, done} From: Ben Hutchings To: Dan Williams , Andrew Morton Cc: stable@vger.kernel.org, Christoph Hellwig , Linus Torvalds , Greg Kroah-Hartman , linux-kernel@vger.kernel.org, linux-mm@kvack.org Date: Thu, 09 Feb 2017 15:26:39 +0000 In-Reply-To: <20170118104625.789178853@linuxfoundation.org> References: <20170118104625.550018627@linuxfoundation.org> <20170118104625.789178853@linuxfoundation.org> Content-Type: multipart/signed; micalg="pgp-sha512"; protocol="application/pgp-signature"; boundary="=-IUE/iiWc6aE7gDvmMIDT" X-Mailer: Evolution 3.22.4-1 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 82.70.136.246 X-SA-Exim-Mail-From: ben@decadent.org.uk X-SA-Exim-Scanned: No (on shadbolt.decadent.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-IUE/iiWc6aE7gDvmMIDT Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2017-01-18 at 11:46 +0100, Greg Kroah-Hartman wrote: > 4.4-stable review patch.=C2=A0=C2=A0If anyone has any objections, please = let me know. >=20 > ------------------ >=20 > From: Dan Williams >=20 > commit f931ab479dd24cf7a2c6e2df19778406892591fb upstream. >=20 > Both arch_add_memory() and arch_remove_memory() expect a single threaded > context. [...] > The result is that two threads calling devm_memremap_pages() > simultaneously can end up colliding on pgd initialization.=C2=A0=C2=A0Thi= s leads > to crash signatures like the following where the loser of the race > initializes the wrong pgd entry: [...] > Hold the standard memory hotplug mutex over calls to > arch_{add,remove}_memory(). [...] This is not a sufficient fix, because memory_hotplug.c still assumes there's only one 'writer': void put_online_mems(void) { ... if (!--mem_hotplug.refcount && unlikely(mem_hotplug.active_writer)) wake_up_process(mem_hotplug.active_writer); ... } void mem_hotplug_begin(void) { mem_hotplug.active_writer =3D current; memhp_lock_acquire(); for (;;) { mutex_lock(&mem_hotplug.lock); if (likely(!mem_hotplug.refcount)) break; __set_current_state(TASK_UNINTERRUPTIBLE); mutex_unlock(&mem_hotplug.lock); schedule(); } } With multiple writers, one or more of them may hang or {get,put}_online_mems() may=C2=A0mess up the hotplug reference count. Is there a good reason that memory_hotplug.c isn't using an rwsem? Ben. --=20 Ben Hutchings All the simple programs have been written, and all the good names taken. --=-IUE/iiWc6aE7gDvmMIDT Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEErCspvTSmr92z9o8157/I7JWGEQkFAlicii8ACgkQ57/I7JWG EQlmOhAA0YlOHwRr13tcbxEhWm/pKgnNDE2ethYz2yPoinft4VRKnxjEO0pqAIre ONgjKPEm3N9RlBSQfCLy87zVYh3jJ3lzDQGRrcoI+07pPMXlTg4eiCXZcoh2p3gk hbxdgnzys2dLhVmKs22/vw3Zkc2auQ+fU65iclSktL3N/H4b85THcAFw46r4VLIP UYP6SAx9VernfY3ZeFGcoPJJzcoRXxAKd5T4it/VwnOMysLV1mT0fjy4YkgO+EEB rRGQJMJNMu0kC7mRNg3mIP6hYAC060Roc0o7Y/AfJ/VtUOlSIzL1ipqLBeSBpyDE Bdb0N0NtaYouuvWp6Sz+NTs44Ax+8r5FZomGFEZjRKYBbQQ7qUdEKgOGXyjniATO pMLeW8lvE/TZeZxcBslJrFWhBAlQ6KjGRruR+JeORrbrBKCkEJe5INvy5PfRD7WE 5Q1I8+ctGxD2COpxFM2beVpx88Cyuxi/V38juD1UtvsdXjnZIisY8M7VmR56j4Uy v78mjNOF+yv2yPuBZfFa0G0AZNlEgdsG1GOq2MMqPr85Rb5LNxeVJbWhAM5uKVRx EKUddB+06OJOtb7/t5JTw6UI9OIdCP62YvjWVc5KphIwV1OqyKKSfMg5APbr/ioK FH2sjr4SJCi2YPKlqT6ntVJEDK8y2N1QUtpMkVJqAfT7KqUhZKw= =DyUp -----END PGP SIGNATURE----- --=-IUE/iiWc6aE7gDvmMIDT--