| Message ID | 20220627190551.517561-1-deller@gmx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] modules: Ensure natural alignment for .altinstructions and __bug_table sections | expand |
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? This information should go into the commit log. Luis > --- > 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 >
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 >>
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) }
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> --- scripts/module.lds.S | 2 ++ 1 file changed, 2 insertions(+) -- 2.35.3