Messages in this thread | | | Date | Fri, 1 Jul 2022 20:22:34 +0200 | Subject | Re: [PATCH 1/2] modules: Ensure natural alignment for .altinstructions and __bug_table sections | From | Helge Deller <> |
| |
On 7/1/22 20:02, Luis Chamberlain wrote: > On Mon, Jun 27, 2022 at 09:05:50PM +0200, Helge Deller wrote: >> In the kernel image vmlinux.lds.S linker scripts the .altinstructions >> and __bug_table sections are 32- or 64-bit aligned because they hold 32- >> and/or 64-bit values. >> >> But for modules the module.lds.S linker script doesn't define a default >> alignment yet, so the linker chooses the default byte-alignment, which >> then leads to unnecessary unaligned memory accesses at runtime. >> >> This patch adds the missing alignments. >> >> Signed-off-by: Helge Deller <deller@gmx.de> > > Good catch but does this fix a real world issue? When are > altinstructions used for modules? When are alternatives used > for modules? > > How did you notice this issue?
You are right - usually this is unnoticed, because either the hardware (on x86 cpus) or by exception handlers (e.g. on hppa or sparc) fix up such unaligned accesses at runtime.
I noticed that, because on hppa the 32-bit unalignment handler was broken due a bad commit, and as such wrong values were returned on unaligned accesses, which then led to other bugs. For hppa I fixed it with this commit: https://patchwork.kernel.org/project/linux-parisc/patch/20220626233911.1023515-1-deller@gmx.de/
This happened when loading the mptbase kernel module: https://lore.kernel.org/all/07d91863-dacc-a503-aa2b-05c3b92a1e39@bell.net/T/#mab602dfa32be5e229d5e192ab012af196d04d75d The summary why it went wrong is at the end of that thread.
On hppa we replace "sync" and some other instructions in kernel and modules - depending on the CPU type (detected at runtime), or on the environment (e.g. when running in qemu).
IMHO this really should be fixed, esp. since the fix is trivial.
> This information should go into the commit log.
Sure, I'll resend with updated info.
Thanks! Helge
>> scripts/module.lds.S | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/scripts/module.lds.S b/scripts/module.lds.S >> index 1d0e1e4dc3d2..3a3aa2354ed8 100644 >> --- a/scripts/module.lds.S >> +++ b/scripts/module.lds.S >> @@ -27,6 +27,8 @@ SECTIONS { >> .ctors 0 : ALIGN(8) { *(SORT(.ctors.*)) *(.ctors) } >> .init_array 0 : ALIGN(8) { *(SORT(.init_array.*)) *(.init_array) } >> >> + .altinstructions 0 : ALIGN(8) { KEEP(*(.altinstructions)) } >> + __bug_table 0 : ALIGN(8) { KEEP(*(__bug_table)) } >> __jump_table 0 : ALIGN(8) { KEEP(*(__jump_table)) } >> >> __patchable_function_entries : { *(__patchable_function_entries) } >> -- >> 2.35.3 >>
| |