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 662F8C02181 for ; Fri, 24 Jan 2025 13:00:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id EC3006B00AA; Fri, 24 Jan 2025 08:00:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E71C16B00AC; Fri, 24 Jan 2025 08:00:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D1227280053; Fri, 24 Jan 2025 08:00:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id B4AFA6B00AA for ; Fri, 24 Jan 2025 08:00:08 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 2EB5F1212EA for ; Fri, 24 Jan 2025 13:00:01 +0000 (UTC) X-FDA: 83042353002.22.FB4E2DF Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) by imf12.hostedemail.com (Postfix) with ESMTP id F150340007 for ; Fri, 24 Jan 2025 12:59:58 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=cYbvme0Z; spf=pass (imf12.hostedemail.com: domain of petr.pavlu@suse.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=petr.pavlu@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1737723599; 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:in-reply-to:references:references:dkim-signature; bh=y4p8V7L+SybE9UNVkk/PglJRrxpw6EHRMFcIJ2bc0MU=; b=NCUWaG0v17BOlbvtGCervRV3rSAzkJQoCPZeqWINyGhq8RFXuReSHEufc0F6E+aVlDMWVm 5RIhSgGBK1gV89zqFxIueYkLVkgO1Dy04waoMf9IDG8c48lw9JsrzFPN1oG2YGBpilxOG5 zogcU8L0XQ8ZN/49HlF9XO2J8newanM= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b=cYbvme0Z; spf=pass (imf12.hostedemail.com: domain of petr.pavlu@suse.com designates 209.85.128.42 as permitted sender) smtp.mailfrom=petr.pavlu@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1737723599; a=rsa-sha256; cv=none; b=e2S6UjNhCn836CkVza12OIRdfhckn6ioGfc+Na+DWwKOb/ZxW1Sf64vhdFR7kK2Z1xApzu VJc5fqZK4i1MbeQRSmBli7elgJBhGtMM/BZtDvS9ul6VVy6WMnZLya9FyZBoLnIk4J7KIs Y4tKsnJb61BDe7bRIykxwBrzWoTclKU= Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-4361815b96cso13981765e9.1 for ; Fri, 24 Jan 2025 04:59:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1737723597; x=1738328397; darn=kvack.org; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :from:to:cc:subject:date:message-id:reply-to; bh=y4p8V7L+SybE9UNVkk/PglJRrxpw6EHRMFcIJ2bc0MU=; b=cYbvme0ZPpOtfhNxwXHz2kRLVOqR/u5oFgZvUXFsUhWp2R9sR3kuiWz+1IlonqQJhT qmmjTPs+rk3NdpXktxgJktZyjdqk7Mw0Ht0iohKU6DKaAOumSSt8CJshciq3suP3ZUk8 OFHptf7Rur/1bgDosF6lpvkRUpwP7E9/OdUPjS4xYYCQOWFDbnyDw6OFeWjvnDxy1np0 q285UkwMykX0l7KMVry4noJsragqDIjZmzrZWsqmN9NND4OOP5VuQuH5Bh+RujXm7+G0 oietaUDnbHs/sOQ1q4kQ7Kbln5jbzZSn6ASZD/AmBseircLwUMSS9TZHAS/ojiutqakl 6+jQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737723597; x=1738328397; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=y4p8V7L+SybE9UNVkk/PglJRrxpw6EHRMFcIJ2bc0MU=; b=RLif3QcfBIlyYkoXrHubsm5K+vifhXou5ri8U/U07rVJhJ35GjjgAjQgQr/4uoNifY /Q9I6sGcGq9K+oCsOmDYLcOBMo4SEL3zJM1fdTFtCm3gvkpCMK2amvMtUBPanzViioVR DoWTk/Uwi3EMvziXDRdAPJkJ+B/j7/wtso+6n1p2rP6vIhurYY2+ub+5AI2SN2e4QQ9D 7Fn5uChhHhLjPWBUTTL4pnKsqQV7PLYGExt/zJpLcJ4iL+biGnuci/cENcftP4SowGCe dmhjbk7TtfYDbRexLlH3ki9ozpEuP3TEdK8nUsCxD+W2SSTlZH7PL/Nl2yfu4oBgKHq5 Uhpw== X-Forwarded-Encrypted: i=1; AJvYcCV4lkLQbjIp9MV3KElyKPIniYqHAoMYe6MXmV8K8p05dyIDOAczLhD1wURCxV7UIFcZDpcC9YiGXg==@kvack.org X-Gm-Message-State: AOJu0Yw16qQTymoc7XIsjoTbRaW5/00zqaTh0fcZbo2X+UkFsT0Di2bB FskyYZK1LfDqo0dWX287JhF7rV8AT8a95RFxVdk87Ja76IIZy2UwBwwhV6UFqcI= X-Gm-Gg: ASbGncu31EwO2zaNOW71JRWqZC4m7tdKYcZg13NrNhyeORokOmYEI2a4gvVoYi11c63 oyJHAjLHMNSjgtyTrco2S823nk/SiOFhbtSJlAQ35IzUvbxsLP+jdW2DOic5Tm53mAVposTn9Je 6qbqNdz5kdIs9FzfVvzrwGW+iZslH7QvaVhHD5GW/VAwg9/olSKNSsoZNMELHM5UVp0/lWmkc3t wNrHKyMkgQU/mGbkJoarX9Yhqbnebvp9DdA5wc8v7kEvxFR9jTqeqQXsbImxArU/mmzCQ0unLI9 HqV+Nc/foZrruFlw15M= X-Google-Smtp-Source: AGHT+IEDbDZN2m5CHXDDsa3oj6Ebp9OHdbtruLTcN0mhHSAJojPcoO/LDhDZ6xjm5yK5+6pSh7FY3A== X-Received: by 2002:a05:600c:63d5:b0:438:a46b:1a6 with SMTP id 5b1f17b1804b1-438a46b0388mr219468875e9.18.1737723597250; Fri, 24 Jan 2025 04:59:57 -0800 (PST) Received: from [10.100.51.161] ([193.86.92.181]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-438bd501721sm25207325e9.9.2025.01.24.04.59.55 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 24 Jan 2025 04:59:56 -0800 (PST) Message-ID: <8c6972c4-c1bb-402a-a72d-f92b87ee5a89@suse.com> Date: Fri, 24 Jan 2025 13:59:55 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2 06/10] module: introduce MODULE_STATE_GONE To: Mike Rapoport 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 References: <20250121095739.986006-1-rppt@kernel.org> <20250121095739.986006-7-rppt@kernel.org> <4a9ca024-fc25-4fe0-94d5-65899b2cec6b@suse.com> Content-Language: en-US From: Petr Pavlu In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: F150340007 X-Stat-Signature: uktdxhro1h9ipgu5hit1gm8zpsn4e1d8 X-Rspam-User: X-HE-Tag: 1737723598-451066 X-HE-Meta: U2FsdGVkX19Yq7vndsKhcltLFvxy4bU6QA44ctJOJMXPWmJQrI6GJrF6Ul61C+JYhnMv3gd0IBgMU7m/yZZX1BbADAgDaqcOU3dBfhjqLDnkCY4798SddqUs3apNUHrw6sRou/Fvfijv2yseEo7rN2KVjhN1W290XH6XdYIJ7KLF5q69dS4XZNxab+aXuh+2NBjOkOmR+XyUalwMqUNVqAh+TE5+jw0/NOFgStHIC77OIEnjlaq9hX/TAJ99uBtJ/ITFi0lgDAGeijR7F8Fc5T9LPK9f+g6PaZZmJAWNHsEjWbzQRtv0Gh+DaL3e1QeyuVW7rTD1PDUFVmw72B2m3GJLqdPMoL2/FJg6+JN/REbTaSqytokjKo24f9mpG3dIEeZEmjYUrTxzFwhI05n7zmMX4YuqlexvqMrS05dAjtyDfv45EheNahJgvlhdj/8L6+rtY+9FY3uyMq5FIgdiSTy7zw3CtsBjKY98k3R6uRRqKL6GyssIGOTb1J3y5VcwGYfzJUO0b2yCmTZrbartyvVTjMnFYF0CRRk15pojpS4flbpUXU/IxrRxLN5PwysUAEt75bY4ahkTmOghflkrWTfoKoAWRALI11d5kvWYtUhPlLG9jG9OGJ/6IuZpYi7usUZ5GfzO1lx+qWRM2hZ5O7prhT3U2/Dc75fT+PiCCZhK4Imx0aTAZFlnUK8fZK54/8Ox446JfxkODDE+BnaFThOgsd6sNljk9DjM9oiMMmUGpzf+1OK35UGJAqShB95vklY2J1KDYHQAA66Tt/0+hhoMi9hY8XongJ4cekhsm1udwAO7MhHqmyn1CRyJB4MV8l5SEAeGY9unimVOVrPbRVD8fcp7UOA1LzMQDeJTImiAHImIhHHd87qihJ+8RGo3LVrFZgkvIRx0Hp9dTQ7k0lPZTtMMdv1uaqKRAbGaTqX36pg6nXnyc+1BBGJJmVbEHAB6CDoal80ByME1sFf ufl4b/aZ d0bk5B+yqBnDgH0JbHu0nQwgBjXvTayjRCYlOYmoCOG5bU1Xnzh3pzZNsH9RX1cXorkwClL4OTwqhp/PXAXbnu9eee2yZoJmmqA7JD6fFwvylBUmenoA869u4XSvXUU5e2UgiMzBw3imEVH8/9JLhK3q6xTzxAog++qazV4Bz4pVeBRnygcniEeb7Iz0QjsAWsaKW3zB8RQvGmbXmvGRm0EMfV06lQIbcL2/bp8/S16WMi827l5l/E5TFsJlpB8PY5hQRMkNuA77bIx8oLb39vGY180eOFMZTjGs8WEmDxQdXGniKbfLHV0u5DgwHQBsrtwcrHwWww7EvuKuRn3hHvaVUfCktvadjwywYFVcu6t6r3Pk= 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 1/24/25 12:06, Mike Rapoport wrote: > 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: > /* > This looks better to me. My view is that the module_state tracks major stages of a module during its lifecycle. It provides information to the module loader itself, other subsystems that need to closely interact with modules, and to the userspace via the initstate sysfs attribute. Adding a new state means potentially more complexity for all these parts. In this case, the state was needed because of a logic that is local only to the module loader, or even just to the function load_module(). I think it is better to avoid adding a new state only for that. -- Thanks, Petr