[Solved] UBSan and Multiboot 2 misaligned member access

[ximinity]

Hello,

Recently I've gotten around to implementing the Undefined Behaviour Sanitizer
(UBSan) handler functions for GCC's UBSan in my kernel. I now compile both the
kernel and C library with -fsanitize=undefined and I've included it in my
automated tests. This all works as expected. With the tests I encountered
only one error with misaligned member access.

The kernel I'm writing is Multiboot 2 compliant. Multiboot 2 provides a structure
to the kernel at boot which contains various tags. These tags can contain
different kinds of information, for example a tag for the memory map and one for
the kernel ELF symbols. It is this ELF tag that causes the issue
(https://www.gnu.org/software/grub/manua ... iboot.html section
3.6.7 ELF-Symbols). I use the ELF tag in the vmm (among others parts) to mark
kernel pages appropriately (e.g. writable or no-exec). The tag is defined as
follows.

Code: Select all

u32 - type
u32 - size
u32 - num
u32 - entsize
u32 - shndx
varies - section headers

Note that the specification says that num etc. are u16. This seems to be an
error as both the examples given in the specification and grub use the above
structure. The section headers part is the section header table.

I have an elf64_shdr structure for ELF-64 objects. I use a function to iterate
through all section headers given in the Multiboot 2 tag. This function calls a
given callback with a pointer to the section header. However, from the
structure that is given above you can see that the section headers are 4-byte
aligned (it starts at offset 20). It is this that causes a 'member access
within misaligned address X for type struct elf64_shdr which requires 8 byte
alignment' when I access a member from the elf64_shdr struct in the callback.

Everything works in terms of functionality. The data is correct and the kernel
works properly but I don't know how to get around this misaligned access.
I could copy the section headers to somewhere where they are aligned but
this seems rather wasteful.

[nullplan]

How about using the correct structure? Evidently, the tag structure is not an Elf64_Shdr, because that one has to be 8 bytes aligned. So just declare your own structure and be done with it.

[ximinity]

Thanks for the response.

I don't think I understand your answer. Maybe I explained it incorrectly.
For the Multiboot 2 ELF symbol tag I use the following structure

Code: Select all

struct mboot2_tag_elf_sections
{
       uint32_t type;
       uint32_t size;
       uint32_t num;
       uint32_t entsize;
       uint32_t shndx;
       char sections[];
};

The num, entsize, and shndx entries correspond to the ‘shdr_*’ entries in
the elf specification in the program header.

The kernel is an elf64 binary. Now, according to the specification the elf
section headers start at the sections member in the above structure. For elf64
section headers I use the following structure

Code: Select all

struct elf64_shdr
{
        uint32_t name;
        uint32_t type;
        uint64_t flags;
        uint64_t addr;
        uint64_t offset;
        uint64_t size;
        uint32_t link;
        uint32_t info;
        uint64_t addralign;
        uint64_t entsize;
};

At some point I iterate through all of the section headers that are in the
mboot2_tag_elf_sections tag with the following function.

Code: Select all

void mboot2_it_elf64(
        const struct mboot2_info *header, elf64_shdr_it it, void *data)
{
        uint32_t i;
        uintptr_t shdr;
        struct mboot2_tag_elf *tag;

        tag = (struct mboot2_tag_elf *)mboot2_get_tag(
                header, MBOOT2_TAG_ELF_TYPE);
        if (!tag)
                return;

        for (i = 0; i < tag->num; ++i) {
                shdr = (uintptr_t)tag->sections + tag->entsize * i;
                it((struct elf64_shdr *)shdr, data);
        }
}

The issue then occurs when I access a member of the elf64_shdr struct in the
callback, for example:

Code: Select all

/* called with mboot2_it_elf64(header, example, NULL) */
void example(const struct elf64_shdr *shdr, void *data)
{
        uint64_t flags;
	
        flags = shdr->flags; /* misaligned access */
}

So the end result is that all of the structures seem correct. The tag itself
contains elf64 section headers at the section member. I've manually checked if
the values of the section headers are correct by comparing it to the results of
objdump -h on the kernel binary.

The issue isn't with incorrect data but because the sections member of the
mboot2_tag_elf_sections structure starts at an offset of 20 bytes the section
headers member is thus 4-byte aligned and not 8-byte aligned.

Because of the above I don't understand what you mean with 'using the correct
structure'.

Edit:
Unless you mean defining a structure that is a copy of the elf64_shdr
defined above but marking it to avoid alignment requirements and using that
one instead?

[xenos]

You might find this interesting:

http://pzemtsov.github.io/2016/11/06/bu ... n-x86.html

Also here:

http://uclibc.org/docs/psABI-x86_64.pdf

So following the ABI, a uint64_t should be 8-byte aligned. The compiler relies on the ABI when doing optimizations, so any data that is fed into your code must follow the ABI. If your multiboot structure gives you a non-aligned uint64_t, it is not ABI compliant - and that is what UBSan complains about. So the "proper" way to handle it is to sanitize the data / make it compliant, as explained in the first link.

[ximinity]

You might find this interesting:

http://pzemtsov.github.io/2016/11/06/bu ... n-x86.html

Also here:

http://uclibc.org/docs/psABI-x86_64.pdf

So following the ABI, a uint64_t should be 8-byte aligned. The compiler relies on the ABI when doing optimizations, so any data that is fed into your code must follow the ABI. If your multiboot structure gives you a non-aligned uint64_t, it is not ABI compliant - and that is what UBSan complains about. So the "proper" way to handle it is to sanitize the data / make it compliant, as explained in the first link.

Thanks for the links, the first one especially was interesting.

The elf64_shdr structure used here must be 8-byte aligned. Because I use a pointer to
an unaligned elf64_shdr this introduces undefined behavior as it now points to a structure that is
not properly aligned and this is why UBSan complains whenever I access any member
using this pointer, right?

In order to make it compliant I now memcpy the i'th section in the for loop into an aligned local
structure and pass this structure to the callback function. This fixes UBSan's complains about the
non-aligned access. Thanks for the help.

[xenos]

The elf64_shdr structure used here must be 8-byte aligned. Because I use a pointer to an unaligned elf64_shdr this introduces undefined behavior as it now points to a structure that is
not properly aligned and this is why UBSan complains whenever I access any member
using this pointer, right?

Yes, exactly.