Re: [RFC PATCH v4 00/13] module: core code clean up

From: Christophe Leroy <christophe.leroy@csgroup.eu>
To: Aaron Tomlin <atomlin@atomlin.com>
Cc: Luis Chamberlain <mcgrof@kernel.org>,
	Aaron Tomlin <atomlin@redhat.com>,
	Michal Suchanek <msuchanek@suse.de>,
	"cl@linux.com" <cl@linux.com>,
	"pmladek@suse.com" <pmladek@suse.com>,
	"mbenes@suse.cz" <mbenes@suse.cz>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"jeyu@kernel.org" <jeyu@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
	"live-patching@vger.kernel.org" <live-patching@vger.kernel.org>,
	"ghalat@redhat.com" <ghalat@redhat.com>,
	"allen.lkml@gmail.com" <allen.lkml@gmail.com>,
	"void@manifault.com" <void@manifault.com>,
	"joe@perches.com" <joe@perches.com>
Subject: Re: [RFC PATCH v4 00/13] module: core code clean up
Date: Mon, 7 Feb 2022 17:17:35 +0000	[thread overview]
Message-ID: <b0a54f00-b9ac-df55-e8d2-d3eb95039a95@csgroup.eu> (raw)
In-Reply-To: <20220207164659.ap42at2nphxu4q6o@ava.usersys.com>

Le 07/02/2022 à 17:46, Aaron Tomlin a écrit :
> On Thu 2022-02-03 18:01 +0000, Christophe Leroy wrote:
>> I don't know exactly what was the motivation for commit 93651f80dcb6
>> ("modules: fix compile error if don't have strict module rwx") at the
>> first place but it is just wrong and we should fix it.
> 
> Christophe,
> 
> I think we are in agreement. If I understand correctly, it should not be
> possible to enable CONFIG_STRICT_MODULE_RWX without
> CONFIG_ARCH_HAS_STRICT_MODULE_RWX (or inversely), as per arch/Kconfig:
> 
>    config STRICT_MODULE_RWX
> 	  bool "Set loadable kernel module data as NX and text as RO" if ARCH_OPTIONAL_KERNEL_RWX
> 	  depends on ARCH_HAS_STRICT_MODULE_RWX && MODULES
> 	  default !ARCH_OPTIONAL_KERNEL_RWX || ARCH_OPTIONAL_KERNEL_RWX_DEFAULT
> 
> The objective of Linus' commit ad21fc4faa2a1 ("arch: Move
> CONFIG_DEBUG_RODATA and CONFIG_SET_MODULE_RONX to be common") and in
> particular commit 0f5bf6d0afe4b ("arch: Rename CONFIG_DEBUG_RODATA and
> CONFIG_DEBUG_MODULE_RONX") does seem correct. So, architectures that would
> prefer to make this feature selectable rather than enabled by default
> should continue to have this option.
> 
>> module_enable_x() should work just fine regardless of
>> CONFIG_ARCH_HAS_STRICT_MODULE_RWX.
> 
> As per the above, we should fix commit 93651f80dcb6 ("modules: fix compile
> error if don't have strict module rwx") so a stub for module_enable_x()
> would no longer be required, right?
> 

Yes and that's the purpose of the patch I proposed at 
https://patchwork.kernel.org/project/linux-modules/patch/203348805c9ac9851d8939d15cb9802ef047b5e2.1643919758.git.christophe.leroy@csgroup.eu/

Allthough I need to find out what's the problem reported by the robot.

As suggested by Luis, this fix should go once all ongoing work is done. 
But it would be nice if you could just remove patch 5 from you series, 
otherwise we would have to revert it later.

Thanks
Christophe