Fix objcopy for little-endian MIPS64 objects.

[jhb]

Summary

MIPS64 does not store the 'r_info' field of a relocation table entry as
a 64-bit value consisting of a 32-bit symbol index in the high 32 bits
and a 32-bit type in the low 32 bits as on other architectures. Instead,
the 64-bit 'r_info' field is really a 32-bit symbol index followed by four
individual byte type fields. For big-endian MIPS64, treating this as a
64-bit integer happens to be compatible with the layout expected by other
architectures (symbol index in upper 32-bits of resulting "native" 64-bit
integer). However, for little-endian MIPS64 the parsed 64-bit integer
contains the symbol index in the low 32 bits and the 4 individual byte
type fields in the upper 32-bits (but as if the upper 32-bits were
byte-swapped).

To cope, add two helper routines in gelf_getrel.c to translate between the
correct native 'r_info' value and the value obtained after the normal
byte-swap translation. Use these routines in gelf_getrel(), gelf_getrela(),
gelf_update_rel(), and gelf_update_rela(). This fixes 'readelf -r' on
little-endian MIPS64 objects which was previously decoding incorrect
relocations as well as 'objcopy: invalid symbox index' warnings from
objcopy when extracting debug symbols from kernel modules.

Even with this fixed, objcopy was still crashing when trying to extract
debug symbols from little-endian MIPS64 modules. The workaround in
gelf_*rel*() depends on the current ELF object having a valid ELF header
so that the 'e_machine' field can be compared against EM_MIPS. objcopy
was parsing the relocation entries to possibly rewrite the 'r_info' fields
in the update_relocs() function before writing the initial ELF header to
the destination object file. Move the initial write of the ELF header
earlier before copy_contents() so that update_relocs() uses the correct
symbol index values.

Note that this change should really go upstream. The binutils readelf
source has a similar hack for MIPS64EL though I implemented this version
from scratch using the MIPS64 ABI PDF as a reference.

Test Plan

• Build a MALTA64EL kernel config with kernel modules re-enabled.

[imp]

Do we also need changes to rtld and/or the kernel loader?

Otherwise this looks good to me. My accept is provisional on there not needing to be other changes.

contrib/elftoolchain/elfcopy/main.c

383 ↗

(On Diff #43540)

"First" here seems somehow wrong after the comment move.

[jhb]

I have not booted or tested mips64el to see if kernel modules actually work, but this does fix the segfaults when building the darn things and does fix readelf -r output to match the GNU one. If the loader and kernel need changes for mips64el modules those wouldn't be part of elftoolchain, so I think this change is self-contained as-is. (That is, any changes to the boot loader and kernel would need to be a separate commit anyway)

contrib/elftoolchain/elfcopy/main.c

383 ↗

(On Diff #43540)

Note that gelf_update_ehder() doesn't do any output section processing, it just writes the ELF header itself which isn't a section.

[jkoshy_users.sourceforge.net]

contrib/elftoolchain/libelf/_libelf.h

225 ↗

(On Diff #43540)

The declarations are probably unnecessary -- these helpers aren't used elsewhere at this point.

contrib/elftoolchain/libelf/gelf_rel.c

136 ↗

(On Diff #43540)

These tests should probably be moved to a helper function -- e.g., something like "_is_mips64el(Elf *e)"?

[emaste]

LGTM with @jkoshy_users.sourceforge.net's comments addressed

[jhb]

Add a _is_mips64el() helper.

[jhb]

contrib/elftoolchain/libelf/_libelf.h

225 ↗

(On Diff #43540)

They are used in two different files though (gelf_rel.c and gelf_rela.c), so I need prototypes somewhere. This seemed like the place for prototypes for functions private to this library that weren't exported via the API?

[emaste]

contrib/elftoolchain/libelf/_libelf.h

200 ↗

(On Diff #46247)

should it be _libelf_is_mips64el?

[jkoshy_users.sourceforge.net]

contrib/elftoolchain/libelf/_libelf.h

200 ↗

(On Diff #46247)

Yes, internal functions use the _libelf prefix -- I should have suggested the right naming convention right up front.

A plain "_is_mips64el" might have been ok if the function had been static within one file.

225 ↗

(On Diff #43540)

Oh right, I had missed the fact that two files are going to be affected by the change.

In that case, I would suggest that the declarations remain in _libelf.h (as in the patch),
but that the function implementations be moved to a standalone ".c" file.

[jhb]

• Move mips64el routines to a dedicated file.

Sorry, I had forgotten I hadn't uploaded this earlier because I was waiting
on a tinderbox to finish.

[jhb]

Ping? Unfortunately it seems like a bug in arcanist that the new file is empty. (It isn't actually empty)

[jhb]

• Reupload to see if the new file works.

[jhb]

Sigh, here's a copy and paste of the file since arcanist is too broken:

/*-
 * Copyright (c) 2018 John Baldwin
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions
 * are met:
 * 1. Redistributions of source code must retain the above copyright
 *    notice, this list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright
 *    notice, this list of conditions and the following disclaimer in the
 *    documentation and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
 * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
 * ARE DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE
 * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
 * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
 * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
 * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
 * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
 * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 * SUCH DAMAGE.
 */

#include <gelf.h>

#include "_libelf.h"

ELFTC_VCSID("$Id$");

int
_libelf_is_mips64el(Elf *e)
{

	return (e->e_kind == ELF_K_ELF && e->e_byteorder == ELFDATA2LSB &&
	    e->e_u.e_elf.e_ehdr.e_ehdr64->e_machine == EM_MIPS);
}

/*
 * For MIPS64, the r_info field is actually stored as a 32-bit symbol
 * index (r_sym) followed by four single-byte fields (r_ssym, r_type3,
 * r_type2, and r_type).  The byte-swap for the little-endian case
 * jumbles this incorrectly so compensate.
 */
Elf64_Xword
_libelf_mips64el_r_info_tof(Elf64_Xword r_info)
{
	Elf64_Xword new_info;
	uint8_t ssym, type3, type2, type;

	ssym = r_info >> 24;
	type3 = r_info >> 16;
	type2 = r_info >> 8;
	type = r_info;
	new_info = r_info >> 32;
	new_info |= (Elf64_Xword)ssym << 32;
	new_info |= (Elf64_Xword)type3 << 40;
	new_info |= (Elf64_Xword)type2 << 48;
	new_info |= (Elf64_Xword)type << 56;
	return (new_info);
}

Elf64_Xword
_libelf_mips64el_r_info_tom(Elf64_Xword r_info)
{
	Elf64_Xword new_info;
	uint8_t ssym, type3, type2, type;

	ssym = r_info >> 32;
	type3 = r_info >> 40;
	type2 = r_info >> 48;
	type = r_info >> 56;
	new_info = (r_info & 0xffffffff) << 32;
	new_info |= (Elf64_Xword)ssym << 24;
	new_info |= (Elf64_Xword)type3 << 16;
	new_info |= (Elf64_Xword)type2 << 8;
	new_info |= (Elf64_Xword)type;
	return (new_info);
}

[emaste]

LGTM. I would say go ahead with re@ approval and I hope @jkoshy_users.sourceforge.net will take care of the upstream change; I'll resolve any differences with the next import.

[emaste]

We discovered corrupt relocation info in BE mips64 modules and @sbruno bisected to this change - see PR 231790.