Re: Unaligned accesses when loading modules

[Jan Beulich <jbeulich@suse.com>]

On 16.01.2025 16:29, Mikulas Patocka wrote:
> 
> 
> On Thu, 16 Jan 2025, Jan Beulich wrote:
> 
>> On 15.01.2025 19:02, Mikulas Patocka wrote:
>>> On Tue, 14 Jan 2025, Sami Tolvanen wrote:
>>>> On Tue, Jan 14, 2025 at 6:07 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>>>>> On PA-RISC, with the kernel 6.12.9, I get unaligned pointer warnings when
>>>>> a module is loaded. The warnings are caused by the fact that the
>>>>> .gnu.linkonce.this_module section is not aligned to the appropriate
>>>>> boundary. If I dump the module content with "objdump -h configs.ko", I get
>>>>> this. Note that the .gnu.linkonce.this_module has "File off 000042d2" and
>>>>> "Algn 2**4".
>>>>>
>>>>> On x86-64, the same misalignment can be seen, but it doesn't cause
>>>>> warnings because unaligned pointers are handled in hardware.
>>>>>
>>>>> This seems to be a bug in the linker, because when I compile an old kernel
>>>>> with a new linker, I also get the misalignment. Do you have an idea how to
>>>>> work around this bug?
>>>>
>>>> Does explicitly specifying section alignment in the module linker
>>>> script fix this by any chance?
>>>>
>>>>> kernel-6.12.9, binutils from Debian ports:
>>>>> [...]
>>>>> kernel 6.10, older binutils:
>>>>
>>>> Which exact versions of binutils were used here? I don't see the
>>>> alignment issue with binutils 2.42 on either x86_64 or parisc64, so I
>>>> assume you're testing with something newer?
>>>>
>>>> $ hppa64-linux-gnu-ld.bfd --version
>>>> GNU ld (GNU Binutils for Debian) 2.42.50.20240625
>>>>
>>>> $ hppa64-linux-gnu-objdump -h configs.ko | grep -E '(format|this_module)'
>>>> configs.ko:     file format elf64-hppa-linux
>>>>  17 .gnu.linkonce.this_module 00000300  0000000000000000
>>>> 0000000000000000  00005c50  2**4
>>>>
>>>> Sami
>>>
>>> Hi
>>>
>>> I use version "GNU ld (GNU Binutils for Debian) 2.43.50.20250108".
>>>
>>> It was broken in the commit 1f1b5e506bf0d9bffef8525eb9bee19646713eb6 in 
>>> the binutils-gdb git and partially fixed in the commit 
>>> d41df13ab36b224a622c0bdf28a96a0dee79db77 - the section is still not 
>>> aligned at their specified boundary (16), but at least it is aligned on 8 
>>> bytes, which avoids the warnings.
>>
>> When you say "broken", can you please explain what it is that is _broken_?
>> Things have changed, yes, but the produced ELF is - afaict - still within
>> spec. The "partial fix" as you call it wasn't really a fix, but a band-aid
>> for some broken consumers of ELF. Plus modpost, being one such example,
>> was supposedly corrected already (Linux commit 8fe1a63d3d99). Said "partial
>> fix" was also confirmed to help modpost [1] - are you saying that wasn't
>> quite true?
> 
> By "broken" I mean that the file offset is not aligned to the section's 
> alignment.

Except that this isn't broken at all. The section's alignment has no meaning
for the file offset (in ordinary object files that is; things are different
for executables); it solely affects the eventual virtual address assignment
by the linker.

> By "partial fix" I mean that the file offset is aligned to 8 bytes, but 
> the section's alignment is 16.
> 
> When Linux loads a module, it takes the .gnu.linkonce.this_module section 
> from the module file and points a pointer to "struct module *" to it (see 
> "info->mod = (void *)info->hdr + info->sechdrs[info->index.mod].sh_offset;").
> So, if the section is misaligned, you get warnings about kernel accesses 
> to unaligned memory.

Right, but that's a problem with the code you quote. The mere use of a
cast plus the assignment of a void * pointer to one requiring better
alignment is already indicative of possible problems.

Jan