On Fri, Aug 16, 2019 at 12:01 PM <zhe...@windriver.com> wrote:
>
>
> 在 2019年8月16日 23:29，Paul Gortmaker <paul.gortma...@windriver.com> 写下:
> >
> > [[linux-yocto] [PATCH] modules: always page-align module section
> > allocations] On 16/08/2019 (Fri 15:36) zhe...@windriver.com wrote:
> >
> > It helps maintainers if the version is embedded in the subject, like:
> >
> > [PATCH v4.18] modules: always page-align module section....
> >
> > > From: Jessica Yu <j...@kernel.org>
> > >
> > > Some arches (e.g., arm64, x86) have moved towards non-executable
> > > module_alloc() allocations for security hardening reasons. That means
> > > that the module loader will need to set the text section of a module to
> > > executable, regardless of whether or not CONFIG_STRICT_MODULE_RWX is set.
> > >
> > > When CONFIG_STRICT_MODULE_RWX=y, module section allocations are always
> > > page-aligned to handle memory rwx permissions. On some arches with
> > > CONFIG_STRICT_MODULE_RWX=n however, when setting the module text to
> > > executable, the BUG_ON() in frob_text() gets triggered since module
> > > section allocations are not page-aligned when CONFIG_STRICT_MODULE_RWX=n.
> > > Since the set_memory_* API works with pages, and since we need to call
> > > set_memory_x() regardless of whether CONFIG_STRICT_MODULE_RWX is set, we
> > > might as well page-align all module section allocations for ease of
> > > managing rwx permissions of module sections (text, rodata, etc).
> > >
> > > Fixes: 2eef1399a866 ("modules: fix BUG when load module with rodata=n")
> > > Reported-by: Martin Kaiser <li...@kaiser.cx>
> > > Reported-by: Bartosz Golaszewski <b...@bgdev.pl>
> > > Tested-by: David Lechner <da...@lechnology.com>
> > > Tested-by: Martin Kaiser <mar...@kaiser.cx>
> > > Tested-by: Bartosz Golaszewski <bgolaszew...@baylibre.com>
> > > Signed-off-by: Jessica Yu <j...@kernel.org>
> > >
> > > commit 38f054d549a869f22a02224cd276a27bf14b6171 upstream
> >
> > Normally we put this right at the top of the long log. Also this commit
> > ID appears bogus - it does not exist in mainline.
>
> Yes, it's from modules-next,
> https://git.kernel.org/pub/scm/linux/kernel/git/jeyu/linux.git/commit/?h=modules-next
I went ahead and cleaned up the commit log, since we never say "commit
<foo> upstream", if "upstream" is not linus' main git repo. I've
pointed at modules-next, and hopefully it is well behaved and won't be
rebased.
I didn't see it in -rc5, so hopefully it makes it upstream soon.
>
> I can't wait as It blocks something.
>
> >
> > Also it seems very odd that the block of Signed-off lines are stuck in
> > the middle of the long log.
> >
> > >
> > > When loading modules with CONFIG_ARCH_HAS_STRICT_MODULE_RWX enabled and
> > > CONFIG_STRICT_MODULE_RWX disabled, the memory allocated for modules would
> > > not be page-aligned and cause the following BUG during frob_text.
> > >
> > > ------------[ cut here ]------------
> > > kernel BUG at kernel/module.c:1907!
> > > Internal error: Oops - BUG: 0 [#1] ARM
> > > Modules linked in:
> > > CPU: 0 PID: 89 Comm: systemd-modules Not tainted 5.3.0-rc2 #1
> > > Hardware name: ARM-Versatile (Device Tree Support)
> > > PC is at frob_text.constprop.0+0x2c/0x40
> > > LR is at load_module+0x14b4/0x1d28
> > > pc : [<c0082930>] lr : [<c0084bb0>] psr: 20000013
> > > sp : ce44fe58 ip : 00000000 fp : 00000000
> > > r10: 00000000 r9 : ce44feb8 r8 : 00000000
> > > r7 : 00000001 r6 : bf00032c r5 : ce44ff40 r4 : bf000320
> > > r3 : bf000400 r2 : 00000fff r1 : 00000220 r0 : bf000000
> > > Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none
> > > Control: 00093177 Table: 0e4c0000 DAC: 00000051
> > > Process systemd-modules (pid: 89, stack limit = 0x9fccc8dc)
> > > Stack: (0xce44fe58 to 0xce450000)
> > > fe40: 00000000
> > > cf1b05b8
> > > fe60: 00000001 ce47cf08 bf002754 c07ae5d8 d0a2a484 bf002060 bf0004f8
> > > 00000000
> > > fe80: b6d17910 c017cf1c ce47cf00 d0a29000 ce47cf00 ce44ff34 000014fc
> > > 00000000
> > > fea0: 00000000 00000000 bf00025c 00000001 00000000 00000000 6e72656b
> > > 00006c65
> > > fec0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> > > 00000000
> > > fee0: 00000000 00000000 00000000 00000000 00000000 c0ac9048 7fffffff
> > > 00000000
> > > ff00: b6d17910 00000005 0000017b c0009208 ce44e000 00000000 b6ebfe54
> > > c008562c
> > > ff20: 7fffffff 00000000 00000003 cefd28f8 00000001 d0a29000 000014fc
> > > 00000000
> > > ff40: d0a292cb d0a29380 d0a29000 000014fc d0a29f0c d0a29d90 d0a29a60
> > > 00000520
> > > ff60: 00000710 00000718 00000826 00000000 00000000 00000000 00000708
> > > 00000023
> > > ff80: 00000024 0000001c 00000000 00000016 00000000 c0ac9048 0041c620
> > > 00000000
> > > ffa0: 00000000 c0009000 0041c620 00000000 00000005 b6d17910 00000000
> > > 00000000
> > > ffc0: 0041c620 00000000 00000000 0000017b 0041f078 00000000 004098b0
> > > b6ebfe54
> > > ffe0: bedb6bc8 bedb6bb8 b6d0f91c b6c945a0 60000010 00000005 00000000
> > > 00000000
> > > [<c0082930>] (frob_text.constprop.0) from [<c0084bb0>]
> > > (load_module+0x14b4/0x1d28)
> > > [<c0084bb0>] (load_module) from [<c008562c>] (sys_finit_module+0xa0/0xc4)
> > > [<c008562c>] (sys_finit_module) from [<c0009000>]
> > > (ret_fast_syscall+0x0/0x50)
> > > Exception stack(0xce44ffa8 to 0xce44fff0)
> > > ffa0: 0041c620 00000000 00000005 b6d17910 00000000
> > > 00000000
> > > ffc0: 0041c620 00000000 00000000 0000017b 0041f078 00000000 004098b0
> > > b6ebfe54
> > > ffe0: bedb6bc8 bedb6bb8 b6d0f91c b6c945a0
> > > Code: e7f001f2 e5931008 e1110002 0a000001 (e7f001f2)
> > > ---[ end trace e904557128d9aed5 ]---
> > >
> > > Signed-off-by: He Zhe <zhe...@windriver.com>
> > > ---
> > > This is for linux-yocto-dev all branches.
> >
> > The version information is important, and shouldnt be hidden down here.
> > If you put it within the [PATCH ....] square bracket section, the
> > maintainer/merger person will see it and the "git am" will throw it away
> > once applied.
>
> Fair points. Thanks, Paul.
I also chopped off the additions to the commit log and just used the
original log. The extra information in this case was just blurred into
the original log, so rather than reformat everything, I just dropped
the extra.
Cheers,
Bruce
>
> Zhe
>
> >
> > Paul.
> > --
> >
> > >
> > > kernel/module.c | 7 +------
> > > 1 file changed, 1 insertion(+), 6 deletions(-)
> > >
> > > diff --git a/kernel/module.c b/kernel/module.c
> > > index 5933395..cd8df51 100644
> > > --- a/kernel/module.c
> > > +++ b/kernel/module.c
> > > @@ -64,14 +64,9 @@
> > >
> > > /*
> > > * Modules' sections will be aligned on page boundaries
> > > - * to ensure complete separation of code and data, but
> > > - * only when CONFIG_STRICT_MODULE_RWX=y
> > > + * to ensure complete separation of code and data
> > > */
> > > -#ifdef CONFIG_STRICT_MODULE_RWX
> > > # define debug_align(X) ALIGN(X, PAGE_SIZE)
> > > -#else
> > > -# define debug_align(X) (X)
> > > -#endif
> > >
> > > /* If this is set, the section belongs in the init part of the module */
> > > #define INIT_OFFSET_MASK (1UL << (BITS_PER_LONG-1))
> > > --
> > > 2.7.4
> > >
> > > --
> > > _______________________________________________
> > > linux-yocto mailing list
> > > linux-yocto@yoctoproject.org
> > > https://lists.yoctoproject.org/listinfo/linux-yocto
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await
thee at its end
- "Use the force Harry" - Gandalf, Star Trek II
--
_______________________________________________
linux-yocto mailing list
linux-yocto@yoctoproject.org
https://lists.yoctoproject.org/listinfo/linux-yocto
