cbfstool: Skip relocation entries pointing to undefined symbol

The linker can make relocation entries of a symbol which has a value
of zero point to the undefined symbol entry.  It is permitted since
when the symbol value is zero as the documentation of the relocation
entry `r_info' field states:

"If the index is STN_UNDEF, the undefined symbol index, the relocation
 uses 0 as the symbol value."

The ELF binary does not really have any missing symbols.  It is an
optimization as the symbol points to the undefined symbol because its
value is zero.

A typical way to hit this cbfstool limitation is to define an empty
region using the REGION macro in the linker script.  Here is an
example if we assume `CONFIG_MY_REGION' is set to 0:

    .car.data {
            [...]
	    REGION(my_region, CONFIG_MY_REGION_SIZE)
	    [...]
    }

A region is defined as follow:

    #define REGION_SIZE(name) ((size_t)_##name##_size)

    #define DECLARE_REGION(name)	\
            extern u8 _##name[];	\
            extern u8 _e##name[];	\
            extern u8 _##name##_size[];

So the size of the region is actually the address of the
`_##name##_size' symbol.  Therefore, the `_my_region_size' symbol
address is zero and the linker can make the relocation entry of this
symbol point to the undefined symbol index.

In such a situation, cbfstool hits a segmentation fault when it
attempts to relocate the symbol in `parse_elf_to_xip_stage()'
function.  We resolves this issue by making cbfstool skips relocation
entries pointing to the undefined symbol similarly to the way it skips
relocation relative to absolute symbols.  A symbol which value is zero
can be considered an absolute symbol and therefore should not be
relocated.

Of course, we could argue that we could just prevent the declaration
of an empty region as illustrated in the following example:

    .car.data {
            [...]
	    #if CONFIG_MY_REGION_SIZE > 0
            REGION(my_region, CONFIG_MY_REGION_SIZE)
	    #endif
	    [...]
    }

However, this is not a satisfying solution because:

1. It requires to add unnecessary code in the linker script as an empty
   region is a valid declaration.  Such a workaround requires the code
   using it to mark the region symbols as weak symbols to handle the
   situation where the region is not defined.

2. There could be other situations which have yet to be uncovered which
   would lead the same cbfstool crash.

3. A binary with an empty region is a valid ELF file and cbfstool
   should not crash when it is asked to create an eXecute-In-Place stage
   out of it.

Change-Id: I2803fd3e96e7ff7a0b22d72d50bfbce7acaeb941
Signed-off-by: Jeremy Compostella <jeremy.compostella@intel.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/77699
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Arthur Heymans <arthur@aheymans.xyz>

1 files changed, 16 insertions, 0 deletions

diff --git a/util/cbfstool/rmodule.c b/util/cbfstool/rmodule.c
index b21f777cd2..d47639c8c1 100644
--- a/util/cbfstool/rmodule.c
+++ b/util/cbfstool/rmodule.c
@@ -168,6 +168,19 @@ static int relocation_for_weak_extern_symbols(struct rmod_context *ctx, Elf64_Re
 	return 0;
 }
 
+static int relocation_for_undefined_symbol(struct rmod_context *ctx, Elf64_Rela *r)
+{
+	Elf64_Sym *s = &ctx->pelf.syms[ELF64_R_SYM(r->r_info)];
+
+	if (s->st_shndx == SHN_UNDEF) {
+		DEBUG("Omitting relocation for undefined symbol: %s\n",
+		      &ctx->strtab[s->st_name]);
+		return 1;
+	}
+
+	return 0;
+}
+
 /*
  * Relocation processing loops.
  */
@@ -213,6 +226,9 @@ static int for_each_reloc(struct rmod_context *ctx, struct reloc_filter *f,
 			if (relocation_for_weak_extern_symbols(ctx, r))
 				continue;
 
+			if (relocation_for_undefined_symbol(ctx, r))
+				continue;
+
 			/* Allow the provided filter to have precedence. */
 			if (f != NULL) {
 				filter_emit = f->filter(f, r);