Binman lets us declare symbols in SPL/TPL that refer to other entries in the same binman image as them. These symbols are filled in with the correct values while binman assembles the images, but this is done in-memory only. Symbols marked as optional can be filled with BINMAN_SYM_MISSING as an error value if their referred entry is missing.
However, the unmodified SPL/TPL binaries are still available on disk, and can be used by people. For these files, nothing ensures that the symbols are set to this error value, and they will be considered valid when they are not.
Empirically, all symbols show up as zero in a sandbox_vpl build when we run e.g. tpl/u-boot-tpl directly. On the other hand, zero is a perfectly fine value for a binman-written symbol, so we cannot say the symbols have wrong values based on that.
Declare a magic symbol that binman always fills in with a fixed value. Check this value as an indicator that symbols were filled in correctly. Return the error value for all symbols when this magic symbol has the wrong value.
For binman tests, we need to make room for the new symbol in the mocked SPL/TPL data by extending them by four bytes. This messes up some test image layouts. Fix the affected values, and check the magic symbol wherever it makes sense.
Signed-off-by: Alper Nebi Yasak alpernebiyasak@gmail.com ---
(no changes since v1)
common/spl/spl.c | 1 + include/binman_sym.h | 45 ++++++++++++++++++++- tools/binman/elf.py | 12 ++++-- tools/binman/elf_test.py | 12 +++--- tools/binman/ftest.py | 33 +++++++-------- tools/binman/test/021_image_pad.dts | 2 +- tools/binman/test/024_sorted.dts | 2 +- tools/binman/test/028_pack_4gb_outside.dts | 2 +- tools/binman/test/029_x86_rom.dts | 6 +-- tools/binman/test/053_symbols.dts | 2 +- tools/binman/test/149_symbols_tpl.dts | 4 +- tools/binman/test/155_symbols_tpl_x86.dts | 4 +- tools/binman/test/187_symbols_sub.dts | 2 +- tools/binman/test/u_boot_binman_syms.c | 4 ++ tools/binman/test/u_boot_binman_syms_size.c | 4 ++ 15 files changed, 97 insertions(+), 38 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 89b875d0e017..6f76205f8d7a 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -40,6 +40,7 @@ #include <wdt.h>
DECLARE_GLOBAL_DATA_PTR; +DECLARE_BINMAN_MAGIC_SYM;
#ifndef CONFIG_SYS_UBOOT_START #define CONFIG_SYS_UBOOT_START CONFIG_SYS_TEXT_BASE diff --git a/include/binman_sym.h b/include/binman_sym.h index 8586ef8731b6..528d7e4e90ed 100644 --- a/include/binman_sym.h +++ b/include/binman_sym.h @@ -11,6 +11,8 @@ #ifndef __BINMAN_SYM_H #define __BINMAN_SYM_H
+/* BSYM in little endian, keep in sync with tools/binman/elf.py */ +#define BINMAN_SYM_MAGIC_VALUE (0x4d595342UL) #define BINMAN_SYM_MISSING (-1UL)
#if CONFIG_IS_ENABLED(BINMAN_SYMBOLS) @@ -62,6 +64,37 @@ __attribute__((aligned(4), weak, unused, \ section(".binman_sym")))
+/** + * _binman_sym_magic - Internal magic symbol for validity checks + * + * When building images, binman fills in this symbol with the magic + * value #defined above. This is used to check at runtime if the + * symbol values were filled in and are OK to use. + */ +extern ulong _binman_sym_magic; + +/** + * DECLARE_BINMAN_MAGIC_SYM - Declare the internal magic symbol + * + * This macro declares the _binman_sym_magic symbol so that it exists. + * Declaring it here would cause errors during linking due to multiple + * definitions of the symbol. + */ +#define DECLARE_BINMAN_MAGIC_SYM \ + ulong _binman_sym_magic \ + __attribute__((aligned(4), section(".binman_sym"))) + +/** + * BINMAN_SYMS_OK - Check if the symbol values are valid + * + * This macro checks if the magic symbol's value is filled properly, + * which indicates that other symbols are OK to use as well. + * + * Return: 1 if binman symbol values are usable, 0 if not + */ +#define BINMAN_SYMS_OK \ + (*(ulong *)&_binman_sym_magic == BINMAN_SYM_MAGIC_VALUE) + /** * binman_sym() - Access a previously declared symbol * @@ -72,10 +105,14 @@ * @_type: Type f the symbol (e.g. unsigned long) * @entry_name: Name of the entry to look for (e.g. 'u_boot_spl') * @_prop_name: Property value to get from that entry (e.g. 'pos') - * @returns value of that property (filled in by binman) + * + * Return: value of that property (filled in by binman), or + * BINMAN_SYM_MISSING if the value is unavailable */ #define binman_sym(_type, _entry_name, _prop_name) \ - (*(_type *)&binman_symname(_entry_name, _prop_name)) + (BINMAN_SYMS_OK ? \ + (*(_type *)&binman_symname(_entry_name, _prop_name)) : \ + BINMAN_SYM_MISSING)
#else /* !CONFIG_IS_ENABLED(BINMAN_SYMBOLS) */
@@ -85,6 +122,10 @@
#define binman_sym_extern(_type, _entry_name, _prop_name)
+#define DECLARE_BINMAN_MAGIC_SYM + +#define BINMAN_SYMS_OK (0) + #define binman_sym(_type, _entry_name, _prop_name) BINMAN_SYM_MISSING
#endif /* CONFIG_IS_ENABLED(BINMAN_SYMBOLS) */ diff --git a/tools/binman/elf.py b/tools/binman/elf.py index afa05e58fdd8..6d440ddf21df 100644 --- a/tools/binman/elf.py +++ b/tools/binman/elf.py @@ -25,6 +25,9 @@ except: # pragma: no cover ELF_TOOLS = False
+# BSYM in little endian, keep in sync with include/binman_sym.h +BINMAN_SYM_MAGIC_VALUE = 0x4d595342 + # Information about an EFL symbol: # section (str): Name of the section containing this symbol # address (int): Address of the symbol (its value) @@ -223,9 +226,12 @@ def LookupAndWriteSymbols(elf_fname, entry, section): raise ValueError('%s has size %d: only 4 and 8 are supported' % (msg, sym.size))
- # Look up the symbol in our entry tables. - value = section.GetImage().LookupImageSymbol(name, sym.weak, msg, - base.address) + if name == '_binman_sym_magic': + value = BINMAN_SYM_MAGIC_VALUE + else: + # Look up the symbol in our entry tables. + value = section.GetImage().LookupImageSymbol(name, sym.weak, + msg, base.address) if value is None: value = -1 pack_string = pack_string.lower() diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py index 02bc10837492..5a51c64cfee3 100644 --- a/tools/binman/elf_test.py +++ b/tools/binman/elf_test.py @@ -127,7 +127,7 @@ def testOutsideFile(self): elf_fname = self.ElfTestFile('u_boot_binman_syms') with self.assertRaises(ValueError) as e: elf.LookupAndWriteSymbols(elf_fname, entry, section) - self.assertIn('entry_path has offset 4 (size 8) but the contents size ' + self.assertIn('entry_path has offset 8 (size 8) but the contents size ' 'is a', str(e.exception))
def testMissingImageStart(self): @@ -161,18 +161,20 @@ def testNoValue(self): This should produce -1 values for all thress symbols, taking up the first 16 bytes of the image. """ - entry = FakeEntry(24) + entry = FakeEntry(28) section = FakeSection(sym_value=None) elf_fname = self.ElfTestFile('u_boot_binman_syms') elf.LookupAndWriteSymbols(elf_fname, entry, section) - self.assertEqual(tools.get_bytes(255, 20) + tools.get_bytes(ord('a'), 4), - entry.data) + expected = (struct.pack('<L', elf.BINMAN_SYM_MAGIC_VALUE) + + tools.get_bytes(255, 20) + + tools.get_bytes(ord('a'), 4)) + self.assertEqual(expected, entry.data)
def testDebug(self): """Check that enabling debug in the elf module produced debug output""" try: tout.init(tout.DEBUG) - entry = FakeEntry(20) + entry = FakeEntry(24) section = FakeSection() elf_fname = self.ElfTestFile('u_boot_binman_syms') with test_util.capture_sys_output() as (stdout, stderr): diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b5cf549703ad..fa1f421c0521 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -43,8 +43,8 @@ # Contents of test files, corresponding to different entry types U_BOOT_DATA = b'1234' U_BOOT_IMG_DATA = b'img' -U_BOOT_SPL_DATA = b'56780123456789abcdefghi' -U_BOOT_TPL_DATA = b'tpl9876543210fedcbazyw' +U_BOOT_SPL_DATA = b'56780123456789abcdefghijklm' +U_BOOT_TPL_DATA = b'tpl9876543210fedcbazywvuts' BLOB_DATA = b'89' ME_DATA = b'0abcd' VGA_DATA = b'vga' @@ -1406,8 +1406,9 @@ def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None, elf_fname = self.ElfTestFile('u_boot_binman_syms') syms = elf.GetSymbols(elf_fname, ['binman', 'image']) addr = elf.GetSymbolAddress(elf_fname, '__image_copy_start') + self.assertEqual(syms['_binman_sym_magic'].address, addr) self.assertEqual(syms['_binman_u_boot_spl_any_prop_offset'].address, - addr) + addr + 4)
self._SetupSplElf('u_boot_binman_syms') data = self._DoReadFileDtb(dts, entry_args=entry_args, @@ -1415,17 +1416,17 @@ def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None, # The image should contain the symbols from u_boot_binman_syms.c # Note that image_pos is adjusted by the base address of the image, # which is 0x10 in our test image - sym_values = struct.pack('<LQLL', 0x00, - u_boot_offset + len(U_BOOT_DATA), + sym_values = struct.pack('<LLQLL', elf.BINMAN_SYM_MAGIC_VALUE, + 0x00, u_boot_offset + len(U_BOOT_DATA), 0x10 + u_boot_offset, 0x04) - expected = (sym_values + base_data[20:] + + expected = (sym_values + base_data[24:] + tools.get_bytes(0xff, 1) + U_BOOT_DATA + sym_values + - base_data[20:]) + base_data[24:]) self.assertEqual(expected, data)
def testSymbols(self): """Test binman can assign symbols embedded in U-Boot""" - self.checkSymbols('053_symbols.dts', U_BOOT_SPL_DATA, 0x18) + self.checkSymbols('053_symbols.dts', U_BOOT_SPL_DATA, 0x1c)
def testSymbolsNoDtb(self): """Test binman can assign symbols embedded in U-Boot SPL""" @@ -3610,20 +3611,20 @@ def testPackIntelFitMissing(self):
def _CheckSymbolsTplSection(self, dts, expected_vals): data = self._DoReadFile(dts) - sym_values = struct.pack('<LQLL', *expected_vals) + sym_values = struct.pack('<LLQLL', elf.BINMAN_SYM_MAGIC_VALUE, *expected_vals) upto1 = 4 + len(U_BOOT_SPL_DATA) - expected1 = tools.get_bytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[20:] + expected1 = tools.get_bytes(0xff, 4) + sym_values + U_BOOT_SPL_DATA[24:] self.assertEqual(expected1, data[:upto1])
upto2 = upto1 + 1 + len(U_BOOT_SPL_DATA) - expected2 = tools.get_bytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[20:] + expected2 = tools.get_bytes(0xff, 1) + sym_values + U_BOOT_SPL_DATA[24:] self.assertEqual(expected2, data[upto1:upto2])
- upto3 = 0x34 + len(U_BOOT_DATA) + upto3 = 0x3c + len(U_BOOT_DATA) expected3 = tools.get_bytes(0xff, 1) + U_BOOT_DATA self.assertEqual(expected3, data[upto2:upto3])
- expected4 = sym_values + U_BOOT_TPL_DATA[20:] + expected4 = sym_values + U_BOOT_TPL_DATA[24:] self.assertEqual(expected4, data[upto3:upto3 + len(U_BOOT_TPL_DATA)])
def testSymbolsTplSection(self): @@ -3631,14 +3632,14 @@ def testSymbolsTplSection(self): self._SetupSplElf('u_boot_binman_syms') self._SetupTplElf('u_boot_binman_syms') self._CheckSymbolsTplSection('149_symbols_tpl.dts', - [0x04, 0x1c, 0x10 + 0x34, 0x04]) + [0x04, 0x20, 0x10 + 0x3c, 0x04])
def testSymbolsTplSectionX86(self): """Test binman can assign symbols in a section with end-at-4gb""" self._SetupSplElf('u_boot_binman_syms_x86') self._SetupTplElf('u_boot_binman_syms_x86') self._CheckSymbolsTplSection('155_symbols_tpl_x86.dts', - [0xffffff04, 0xffffff1c, 0xffffff34, + [0xffffff04, 0xffffff20, 0xffffff3c, 0x04])
def testPackX86RomIfwiSectiom(self): @@ -4488,7 +4489,7 @@ def testCompressExtra(self):
def testSymbolsSubsection(self): """Test binman can assign symbols from a subsection""" - self.checkSymbols('187_symbols_sub.dts', U_BOOT_SPL_DATA, 0x18) + self.checkSymbols('187_symbols_sub.dts', U_BOOT_SPL_DATA, 0x1c)
def testReadImageEntryArg(self): """Test reading an image that would need an entry arg to generate""" diff --git a/tools/binman/test/021_image_pad.dts b/tools/binman/test/021_image_pad.dts index 1ff8dab296fc..c5abbbcdd6ab 100644 --- a/tools/binman/test/021_image_pad.dts +++ b/tools/binman/test/021_image_pad.dts @@ -10,7 +10,7 @@ u-boot-spl { };
u-boot { - offset = <24>; + offset = <28>; }; }; }; diff --git a/tools/binman/test/024_sorted.dts b/tools/binman/test/024_sorted.dts index b79d9adf6827..b54f9b14191d 100644 --- a/tools/binman/test/024_sorted.dts +++ b/tools/binman/test/024_sorted.dts @@ -7,7 +7,7 @@ / { binman { sort-by-offset; u-boot { - offset = <26>; + offset = <30>; };
u-boot-spl { diff --git a/tools/binman/test/028_pack_4gb_outside.dts b/tools/binman/test/028_pack_4gb_outside.dts index 11a1f6059e26..b6ad7fb56a53 100644 --- a/tools/binman/test/028_pack_4gb_outside.dts +++ b/tools/binman/test/028_pack_4gb_outside.dts @@ -13,7 +13,7 @@ u-boot { };
u-boot-spl { - offset = <0xffffffe7>; + offset = <0xffffffe3>; }; }; }; diff --git a/tools/binman/test/029_x86_rom.dts b/tools/binman/test/029_x86_rom.dts index 88aa007bbae1..ad8f9d6e1bdd 100644 --- a/tools/binman/test/029_x86_rom.dts +++ b/tools/binman/test/029_x86_rom.dts @@ -7,13 +7,13 @@ / { binman { sort-by-offset; end-at-4gb; - size = <32>; + size = <36>; u-boot { - offset = <0xffffffe0>; + offset = <0xffffffdc>; };
u-boot-spl { - offset = <0xffffffe7>; + offset = <0xffffffe3>; }; }; }; diff --git a/tools/binman/test/053_symbols.dts b/tools/binman/test/053_symbols.dts index 296580927648..b28f34a72faa 100644 --- a/tools/binman/test/053_symbols.dts +++ b/tools/binman/test/053_symbols.dts @@ -10,7 +10,7 @@ u-boot-spl { };
u-boot { - offset = <0x18>; + offset = <0x1c>; };
u-boot-spl2 { diff --git a/tools/binman/test/149_symbols_tpl.dts b/tools/binman/test/149_symbols_tpl.dts index 0a4ab3f1fabd..4e649c45978d 100644 --- a/tools/binman/test/149_symbols_tpl.dts +++ b/tools/binman/test/149_symbols_tpl.dts @@ -11,12 +11,12 @@ u-boot-spl { };
u-boot-spl2 { - offset = <0x1c>; + offset = <0x20>; type = "u-boot-spl"; };
u-boot { - offset = <0x34>; + offset = <0x3c>; };
section { diff --git a/tools/binman/test/155_symbols_tpl_x86.dts b/tools/binman/test/155_symbols_tpl_x86.dts index 9d7dc51b3d9b..e1ce33e67fbc 100644 --- a/tools/binman/test/155_symbols_tpl_x86.dts +++ b/tools/binman/test/155_symbols_tpl_x86.dts @@ -14,12 +14,12 @@ u-boot-spl { };
u-boot-spl2 { - offset = <0xffffff1c>; + offset = <0xffffff20>; type = "u-boot-spl"; };
u-boot { - offset = <0xffffff34>; + offset = <0xffffff3c>; };
section { diff --git a/tools/binman/test/187_symbols_sub.dts b/tools/binman/test/187_symbols_sub.dts index 54511a737112..3ab62d372152 100644 --- a/tools/binman/test/187_symbols_sub.dts +++ b/tools/binman/test/187_symbols_sub.dts @@ -11,7 +11,7 @@ u-boot-spl { };
u-boot { - offset = <24>; + offset = <28>; }; };
diff --git a/tools/binman/test/u_boot_binman_syms.c b/tools/binman/test/u_boot_binman_syms.c index 89fee5567e1d..ed761246aec7 100644 --- a/tools/binman/test/u_boot_binman_syms.c +++ b/tools/binman/test/u_boot_binman_syms.c @@ -5,9 +5,13 @@ * Simple program to create some binman symbols. This is used by binman tests. */
+typedef unsigned long ulong; + #include <linux/kconfig.h> #include <binman_sym.h>
+DECLARE_BINMAN_MAGIC_SYM; + binman_sym_declare(unsigned long, u_boot_spl_any, offset); binman_sym_declare(unsigned long long, u_boot_spl2, offset); binman_sym_declare(unsigned long, u_boot_any, image_pos); diff --git a/tools/binman/test/u_boot_binman_syms_size.c b/tools/binman/test/u_boot_binman_syms_size.c index c4a053f96f1a..fa41b3d9a332 100644 --- a/tools/binman/test/u_boot_binman_syms_size.c +++ b/tools/binman/test/u_boot_binman_syms_size.c @@ -5,7 +5,11 @@ * Simple program to create some binman symbols. This is used by binman tests. */
+typedef unsigned long ulong; + #include <linux/kconfig.h> #include <binman_sym.h>
+DECLARE_BINMAN_MAGIC_SYM; + binman_sym_declare(char, u_boot_spl, pos);