Re: [RFC PATCH 2/3] module: Don't fail module loading when setting ro_after_init section RO failed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





Le 12/11/2024 à 10:43, Daniel Gomez a écrit :
On Mon Nov 11, 2024 at 7:53 PM CET, Christophe Leroy wrote:


Le 09/11/2024 à 23:17, Daniel Gomez a écrit :
On Sat Nov 9, 2024 at 11:35 AM CET, Christophe Leroy wrote:
Once module init has succeded it is too late to cancel loading.
If setting ro_after_init data section to read-only fails, all we
can do is to inform the user through a warning.

Reported-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
Closes: https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fprotect2.fireeye.com%2Fv1%2Furl%3Fk%3Dd3deb284-b2a35ac3-d3df39cb-74fe485fff30-288375d7d91e4ad9%26q%3D1%26e%3D701066ca-634d-4525-a77d-1a44451f897a%26u%3Dhttps%253A%252F%252Feur01.safelinks.protection.outlook.com%252F%253Furl%253Dhttps%25253A%25252F%25252Flore.kernel.org%25252Fall%25252F20230915082126.4187913-1-ruanjinjie%252540huawei.com%25252F%2526data%253D05%25257C02%25257Cchristophe.leroy%252540csgroup.eu%25257C26b5ca7363e54210439b08dd010c4865%25257C8b87af7d86474dc78df45f69a2011bb5%25257C0%25257C0%25257C638667874457200373%25257CUnknown%25257CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%25253D%25253D%25257C0%25257C%25257C%25257C%2526sdata%253DZeJ%25252F3%25252B2Nx%25252FBf%25252FWLFEkhxKlDhZk8LNkz0fs%25252Fg2xMcOjY%25253D%2526reserved%253D0&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cc86adbd7bad24b1042bd08dd02fe7c8e%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638670014259822622%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=Gpxrx401fRdCGahGcI6GtJp%2BqLTZsnNqxsDoz4TAfU8%3D&reserved=0
Fixes: d1909c022173 ("module: Don't ignore errors from set_memory_XX()")
Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxxxxxx>
---
   kernel/module/main.c | 6 +++---
   1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/module/main.c b/kernel/module/main.c
index 2de4ad7af335..1bf4b0db291b 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2583,7 +2583,9 @@ static noinline int do_init_module(struct module *mod)
   #endif
   	ret = module_enable_rodata_ro_after_init(mod);
   	if (ret)
-		goto fail_mutex_unlock;
+		pr_warn("%s: %s() returned %d, ro_after_init data might still be writable\n",
+			mod->name, __func__, ret);
+
   	mod_tree_remove_init(mod);
   	module_arch_freeing_init(mod);
   	for_class_mod_mem_type(type, init) {
@@ -2622,8 +2624,6 @@ static noinline int do_init_module(struct module *mod)
return 0;

I think it would make sense to propagate the error. But that would
require changing modprobe.c. What kind of error can we expect when this
happens?

AFAIK, on powerpc it fails with EINVAL when
- The area is a vmalloc or module area and is a hugepage area
- The area is not vmalloc or io register and MMU is not powerpc radix MMU

Otherwise it propagates the error from apply_to_existing_page_range().
IIUC it will return EINVAL when it hits a leaf PTE in upper directories.

Looking at that path I see we can also fail at __apply_to_page_range()
-> apply_to_p4d_range() and return with -ENOMEM.

The -ENOMEM is when 'create' is true, usually when there is not enough memory available to create a page table ... in that case I guess you have much more problems to happen ...

set_memory_ro() on powerpc calls apply_to_existing_page_range() which implies 'create' is false.

Christophe




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux