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 576C2C02181 for ; Fri, 24 Jan 2025 11:07:25 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DAC3E6B00A1; Fri, 24 Jan 2025 06:07:24 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id D5BA66B00A4; Fri, 24 Jan 2025 06:07:24 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id BFC50280059; Fri, 24 Jan 2025 06:07:24 -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 9B8EC6B00A1 for ; Fri, 24 Jan 2025 06:07:24 -0500 (EST) Received: from smtpin23.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 0E5F6B0306 for ; Fri, 24 Jan 2025 11:07:24 +0000 (UTC) X-FDA: 83042069208.23.8294EF8 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf25.hostedemail.com (Postfix) with ESMTP id 54E69A0010 for ; Fri, 24 Jan 2025 11:07:22 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="g8qJYU/p"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1737716842; a=rsa-sha256; cv=none; b=8cqrKNyY2rRRkit+OfFQHNwM22zOQ3/7wdh1c5gDfNakBnePXWJnWmMe8/2qeu9wuzkTDp f4blYX/4hVDTCpmF0qoEi4vNoJXMgPqMgfmMucdB14TYiL997nftz+YG9Q/sSAb+T0NvFm crMVHcnzMz89lVtdbROoRj1B8vfsTJA= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b="g8qJYU/p"; dmarc=pass (policy=quarantine) header.from=kernel.org; spf=pass (imf25.hostedemail.com: domain of rppt@kernel.org designates 139.178.84.217 as permitted sender) smtp.mailfrom=rppt@kernel.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1737716842; 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=GsHc0DkyTGM6Cpzq70Lj5XZc9IxktRLAdwDaGEdlVhU=; b=lX9YU0lfwqoMlf7AyfjmKDM3n6DTn+7YTOi8H8pr4PNIEexGFX0EcUqTExwMwQ7LAxIn5S Xf+qlthLIJu/16xWAPGz6kTp1CNK1qnVw+FT6moIcG3miz68txkB1FEjtSdXuIrwaQvjLh 3KuhnMrKq4kXc5EDJeyluuJoHs4wDx8= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id CDEFD5C5DE5; Fri, 24 Jan 2025 11:06:40 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 833FDC4CED2; Fri, 24 Jan 2025 11:07:07 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1737716840; bh=wSfyHXcmcOSuIJyW+bb7SIJC8asuo8BMjmJ6ZKag59U=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=g8qJYU/pXyq687Cg0zGfUkpXlGffvuM6D0jtxkliKJACp5JlMojm1GDrNE8vwkQHh QhQ6ovm02lHCCSi4Q+bICN5Ri53ztGf22K3e0skaJ4pRVYXnu3siPiddwrn71c1YpM keFYn5AIrov0gSoot4RZ0oOIf/s0nFcQ/mMYx6r53nKF2FyEV8e+apGJAN5yv5UYhI nIw+SIrFrRkO/Gzpy0loesaORzuJhjgA3nl3p1C1IHchkGMB2VNwUZ4rQfDyfRcmKr FqCOto/w0cN/rIRN+Pw1dI1+KtqsXHjMG2qvVSJC18R98+nJwFyTMBLA5ebT7cyqm7 eo4aCP4xXm6ZA== Date: Fri, 24 Jan 2025 13:06:57 +0200 From: Mike Rapoport To: Petr Pavlu Cc: x86@kernel.org, Andrew Morton , Andy Lutomirski , Anton Ivanov , Borislav Petkov , Brendan Higgins , Daniel Gomez , Daniel Thompson , Dave Hansen , David Gow , Douglas Anderson , Ingo Molnar , Jason Wessel , Jiri Kosina , Joe Lawrence , Johannes Berg , Josh Poimboeuf , "Kirill A. Shutemov" , Lorenzo Stoakes , Luis Chamberlain , Mark Rutland , Masami Hiramatsu , Miroslav Benes , "H. Peter Anvin" , Peter Zijlstra , Petr Mladek , Rae Moar , Richard Weinberger , Sami Tolvanen , Shuah Khan , Song Liu , Steven Rostedt , Thomas Gleixner , kgdb-bugreport@lists.sourceforge.net, kunit-dev@googlegroups.com, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-mm@kvack.org, linux-modules@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-um@lists.infradead.org, live-patching@vger.kernel.org Subject: Re: [PATCH v2 06/10] module: introduce MODULE_STATE_GONE Message-ID: References: <20250121095739.986006-1-rppt@kernel.org> <20250121095739.986006-7-rppt@kernel.org> <4a9ca024-fc25-4fe0-94d5-65899b2cec6b@suse.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4a9ca024-fc25-4fe0-94d5-65899b2cec6b@suse.com> X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 54E69A0010 X-Stat-Signature: hoi98kthyogsw4s61jn4rrrnnius46ba X-Rspam-User: X-HE-Tag: 1737716842-865617 X-HE-Meta: U2FsdGVkX1/3YlN3Q0GnJRh6eS4GfrSZjkr0iAo652XSBKfFuW7WEIEjJJKGHJ6NQHkD2UE0BQXlGDOZVP/lAE8pZksmM0vAb1DCFFq4Z9rCKKSLGnaiAbydbHAYFAj9nSUYM4N1ugvOqMpab3TfeoavhPgB6UMme66wDwhBqkV//mcro6uzThIexGd8dlB9dKL7EzvPTYrnAQo7wZgk62Yd1fKujNWlunOJjJJa8IJcTok7lwSPiRIjw72IObKvVfR8uROlcD5N+zuHriYb27+CIFCX6BPxFvPbwMYf4j72cl+DzxwQuSDJxp4JNjaHe72CguR5w3QN+69/ydK5k2NEKX2Pdsn4C7LuRgHXHxsVoavr2wWdYYs0ewx1UZ7jxHpEVx3lmHpLZicW/QhqutZ/AIqKWAootafaLMx3bQJHrgLvBY4OtwbOkkizEbgUzHp5LEj6mNio4R+VoCJTEExxceOTswcjZ8iHPIn8nei4/g33EJchZfrTXJWX3ntT/e2/PWAjYfBd0e4pUxCYf1ejXlnj2OY5UR2AZLqTEjqH6gJ/acV6aNa5GFGbrvRl+8OiryD0QI98O8hSAw1bsJ1g6OGGXWohCdMBmaq+9XdI5A+XV9zu9UYn3EGvzhrG7GSY9I7gdHWEcs8l+RTZ/JrFy5Y3+He+6/3xkYYU1+Jso6Mk2WU9HKED9sRWazF2zCIv1BTGvGC0L2sd8JQsBz9E08KNeOZzsJ6TK7reFYun06WVLjYg1u6UuJDjRCStVsZ3/njRPj1J6wWfO5baJgh4MeCPKFdWRxJsFGl015tlmyikqkWIRwfhUe4jvj1q5iddXVnhdMwkCg4TdhVsZShq62lbzFCoRw+BbfpUj/SBKomSBTq2buv8c6r7sAyLLSFx57zRVZpfMfFhRc9lW57BMpbe+2ESHch6V5wJx4zgb5VBM4Gb03ZgAEdWdNEm+xDYQgKwwkGRi4OFNkG ahb9N3OQ EysyUvQLWOH7ompN/5hH8hE59w/7lLogvlZXwwN91I0fQ1IXMrdTClJfuUeCPSfkI7uvX9Pw+YoUNwwY4BifYd93u/K9wh+wGizIysk+B5sIkf5P8gkp/dXoLVVMBvGe8ldz1J4Ixa4689Ea8FzE76uzZsYc+sysUP2OGvMwwW3yCaqIZsy8L5ss8NruV4PFn9pADx2cXoZT4O1KI46o5F4a0NOlEGc67mNz8O1cEkbsiLTgzziuUstMhI8mrGH8he7QNu5aH8Y2F9UmpQaqI2j43fYtr9xB2WSQA6gXWiRocZ36YPDqn3LdTiw== 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 Thu, Jan 23, 2025 at 03:16:28PM +0100, Petr Pavlu wrote: > On 1/21/25 10:57, Mike Rapoport wrote: > > In order to use execmem's API for temporal remapping of the memory > > allocated from ROX cache as writable, there is a need to distinguish > > between the state when the module is being formed and the state when it is > > deconstructed and freed so that when module_memory_free() is called from > > error paths during module loading it could restore ROX mappings. > > > > Replace open coded checks for MODULE_STATE_UNFORMED with a helper > > function module_is_formed() and add a new MODULE_STATE_GONE that will be > > set when the module is deconstructed and freed. > > I don't fully follow why this case requires a new module state. My > understanding it that the function load_module() has the necessary > context that after calling layout_and_allocate(), the updated ROX > mappings need to be restored. I would then expect the function to be > appropriately able to unwind this operation in case of an error. It > could be done by having a helper that walks the mappings and calls > execmem_restore_rox(), or if you want to keep it in module_memory_free() > as done in the patch #7 then a flag could be passed down to > module_deallocate() -> free_mod_mem() -> module_memory_free()? Initially I wanted to track ROX <-> RW transitions in struct module_memory so that module_memory_free() could do the right thing depending on memory state. But that meant either ugly games with const'ness in strict_rwx.c, an additional helper or a new global module state. The latter seemed the most elegant to me. If a new global module state is really that intrusive, I can drop it in favor a helper that will be called from error handling paths. E.g. something like the patch below (on top of this series and with this patch reverted) diff --git a/kernel/module/main.c b/kernel/module/main.c index 7164cd353a78..4a02503836d7 100644 --- a/kernel/module/main.c +++ b/kernel/module/main.c @@ -1268,13 +1268,20 @@ static int module_memory_alloc(struct module *mod, enum mod_mem_type type) return 0; } +static void module_memory_restore_rox(struct module *mod) +{ + for_class_mod_mem_type(type, text) { + struct module_memory *mem = &mod->mem[type]; + + if (mem->is_rox) + execmem_restore_rox(mem->base, mem->size); + } +} + static void module_memory_free(struct module *mod, enum mod_mem_type type) { struct module_memory *mem = &mod->mem[type]; - if (mod->state == MODULE_STATE_UNFORMED && mem->is_rox) - execmem_restore_rox(mem->base, mem->size); - execmem_free(mem->base); } @@ -2617,6 +2624,7 @@ static int move_module(struct module *mod, struct load_info *info) return 0; out_err: + module_memory_restore_rox(mod); for (t--; t >= 0; t--) module_memory_free(mod, t); if (codetag_section_found) @@ -3372,6 +3380,7 @@ static int load_module(struct load_info *info, const char __user *uargs, mod->mem[type].size); } + module_memory_restore_rox(mod); module_deallocate(mod, info); free_copy: /* > It is at least good that MODULE_STATE_GONE is only set in free_module() > past the sysfs teardown, so it never shows in > /sys/module//initstate. Otherwise, this would require teaching kmod > about this state as well. > > -- > Thanks, > Petr -- Sincerely yours, Mike.