2020-04-29 15:27:51

by Josh Poimboeuf

[permalink] [raw]
Subject: [PATCH v4 11/11] module: Make module_enable_ro() static again

Now that module_enable_ro() has no more external users, make it static
again.

Suggested-by: Jessica Yu <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
include/linux/module.h | 6 ------
kernel/module.c | 4 ++--
2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index e4ef7b36feda..2c2e988bcf10 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -858,12 +858,6 @@ extern int module_sysfs_initialized;

#define __MODULE_STRING(x) __stringify(x)

-#ifdef CONFIG_STRICT_MODULE_RWX
-extern void module_enable_ro(const struct module *mod, bool after_init);
-#else
-static inline void module_enable_ro(const struct module *mod, bool after_init) { }
-#endif
-
#ifdef CONFIG_GENERIC_BUG
void module_bug_finalize(const Elf_Ehdr *, const Elf_Shdr *,
struct module *);
diff --git a/kernel/module.c b/kernel/module.c
index f0e414a01d91..572d626508d2 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2016,7 +2016,7 @@ static void frob_writable_data(const struct module_layout *layout,
(layout->size - layout->ro_after_init_size) >> PAGE_SHIFT);
}

-void module_enable_ro(const struct module *mod, bool after_init)
+static void module_enable_ro(const struct module *mod, bool after_init)
{
if (!rodata_enabled)
return;
@@ -2057,7 +2057,7 @@ static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
}

#else /* !CONFIG_STRICT_MODULE_RWX */
-/* module_{enable,disable}_ro() stubs are in module.h */
+static void module_enable_ro(const struct module *mod, bool after_init) {}
static void module_enable_nx(const struct module *mod) { }
static int module_enforce_rwx_sections(Elf_Ehdr *hdr, Elf_Shdr *sechdrs,
char *secstrings, struct module *mod)
--
2.21.1


2020-04-30 11:34:06

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

+++ Josh Poimboeuf [29/04/20 10:24 -0500]:
>Now that module_enable_ro() has no more external users, make it static
>again.
>
>Suggested-by: Jessica Yu <[email protected]>
>Signed-off-by: Josh Poimboeuf <[email protected]>

Thanks! Since this patch is separate from the rest and it's based on
modules-next, I can just take this last patch through the modules tree.

Jessica

2020-04-30 11:39:25

by Miroslav Benes

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

On Thu, 30 Apr 2020, Jessica Yu wrote:

> +++ Josh Poimboeuf [29/04/20 10:24 -0500]:
> >Now that module_enable_ro() has no more external users, make it static
> >again.
> >
> >Suggested-by: Jessica Yu <[email protected]>
> >Signed-off-by: Josh Poimboeuf <[email protected]>
>
> Thanks! Since this patch is separate from the rest and it's based on
> modules-next, I can just take this last patch through the modules tree.

It depends on 8/11 of the series.

Acked-by: Miroslav Benes <[email protected]>

for the patch.

M

2020-04-30 11:44:41

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

+++ Miroslav Benes [30/04/20 13:35 +0200]:
>On Thu, 30 Apr 2020, Jessica Yu wrote:
>
>> +++ Josh Poimboeuf [29/04/20 10:24 -0500]:
>> >Now that module_enable_ro() has no more external users, make it static
>> >again.
>> >
>> >Suggested-by: Jessica Yu <[email protected]>
>> >Signed-off-by: Josh Poimboeuf <[email protected]>
>>
>> Thanks! Since this patch is separate from the rest and it's based on
>> modules-next, I can just take this last patch through the modules tree.
>
>It depends on 8/11 of the series.
>
>Acked-by: Miroslav Benes <[email protected]>
>
>for the patch.

Ah yeah, you are right (you meant patch 9/11 right)? Will take both
through modules-next.

2020-04-30 12:25:17

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

+++ Jessica Yu [30/04/20 13:40 +0200]:
>+++ Miroslav Benes [30/04/20 13:35 +0200]:
>>On Thu, 30 Apr 2020, Jessica Yu wrote:
>>
>>>+++ Josh Poimboeuf [29/04/20 10:24 -0500]:
>>>>Now that module_enable_ro() has no more external users, make it static
>>>>again.
>>>>
>>>>Suggested-by: Jessica Yu <[email protected]>
>>>>Signed-off-by: Josh Poimboeuf <[email protected]>
>>>
>>>Thanks! Since this patch is separate from the rest and it's based on
>>>modules-next, I can just take this last patch through the modules tree.
>>
>>It depends on 8/11 of the series.
>>
>>Acked-by: Miroslav Benes <[email protected]>
>>
>>for the patch.
>
>Ah yeah, you are right (you meant patch 9/11 right)? Will take both
>through modules-next.

So I'm speaking nonsense apparently. I suggested taking them because
the module patches were based on modules-next. But Miroslav correctly
pointed out that these patches still depend on livepatch removing
module_disable_ro() usage before we can even remove them from
module.c.

So ignore what I said earlier, the whole patchset should be applied
together (I'm assuming the livepatching for-next branch). In any case,
should there be any conflicts with modules-next they should be easy to
resolve. Sorry for the noise!

Jessica

2020-06-05 13:27:04

by Guenter Roeck

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

On Wed, Apr 29, 2020 at 10:24:53AM -0500, Josh Poimboeuf wrote:
> Now that module_enable_ro() has no more external users, make it static
> again.
>
> Suggested-by: Jessica Yu <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> Acked-by: Miroslav Benes <[email protected]>

Apparently this patch made it into the upstream kernel on its own,
not caring about its dependencies. Results are impressive.

Build results:
total: 155 pass: 101 fail: 54
Qemu test results:
total: 431 pass: 197 fail: 234

That means bisects will be all but impossible until this is fixed.
Was that really necessary ?

Guenter

2020-06-05 14:24:37

by Jessica Yu

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

+++ Guenter Roeck [05/06/20 06:24 -0700]:
>On Wed, Apr 29, 2020 at 10:24:53AM -0500, Josh Poimboeuf wrote:
>> Now that module_enable_ro() has no more external users, make it static
>> again.
>>
>> Suggested-by: Jessica Yu <[email protected]>
>> Signed-off-by: Josh Poimboeuf <[email protected]>
>> Acked-by: Miroslav Benes <[email protected]>
>
>Apparently this patch made it into the upstream kernel on its own,
>not caring about its dependencies. Results are impressive.
>
>Build results:
> total: 155 pass: 101 fail: 54
>Qemu test results:
> total: 431 pass: 197 fail: 234
>
>That means bisects will be all but impossible until this is fixed.
>Was that really necessary ?

Sigh, I am really sorry about this. We made a mistake in handling
inter-tree dependencies between livepatching and modules-next,
unfortunately :-( Merging the modules-next pull request next should
resolve the module_enable_ro() not defined for
!ARCH_HAS_STRICT_MODULE_RWX build issue. The failure was hidden in
linux-next since both trees were always merged together. Again, it
doesn't excuse us from build testing our separate trees more
rigorously.

2020-06-05 14:42:02

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH v4 11/11] module: Make module_enable_ro() static again

On Fri, Jun 05, 2020 at 04:20:10PM +0200, Jessica Yu wrote:
> +++ Guenter Roeck [05/06/20 06:24 -0700]:
> > On Wed, Apr 29, 2020 at 10:24:53AM -0500, Josh Poimboeuf wrote:
> > > Now that module_enable_ro() has no more external users, make it static
> > > again.
> > >
> > > Suggested-by: Jessica Yu <[email protected]>
> > > Signed-off-by: Josh Poimboeuf <[email protected]>
> > > Acked-by: Miroslav Benes <[email protected]>
> >
> > Apparently this patch made it into the upstream kernel on its own,
> > not caring about its dependencies. Results are impressive.
> >
> > Build results:
> > total: 155 pass: 101 fail: 54
> > Qemu test results:
> > total: 431 pass: 197 fail: 234
> >
> > That means bisects will be all but impossible until this is fixed.
> > Was that really necessary ?
>
> Sigh, I am really sorry about this. We made a mistake in handling
> inter-tree dependencies between livepatching and modules-next,
> unfortunately :-( Merging the modules-next pull request next should
> resolve the module_enable_ro() not defined for
> !ARCH_HAS_STRICT_MODULE_RWX build issue. The failure was hidden in
> linux-next since both trees were always merged together. Again, it
> doesn't excuse us from build testing our separate trees more
> rigorously.

This is mostly my fault for basing my patches on linux-next -- oops.

We've also been trained to be lazy by the 0-day bot, which has been
slacking lately.

--
Josh