]> git.dujemihanovic.xyz Git - u-boot.git/commitdiff
spl: binman: Check at runtime if binman symbols were filled in
authorAlper Nebi Yasak <alpernebiyasak@gmail.com>
Sat, 18 Jun 2022 12:13:11 +0000 (15:13 +0300)
committerSimon Glass <sjg@chromium.org>
Tue, 28 Jun 2022 02:09:52 +0000 (03:09 +0100)
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>
15 files changed:
common/spl/spl.c
include/binman_sym.h
tools/binman/elf.py
tools/binman/elf_test.py
tools/binman/ftest.py
tools/binman/test/021_image_pad.dts
tools/binman/test/024_sorted.dts
tools/binman/test/028_pack_4gb_outside.dts
tools/binman/test/029_x86_rom.dts
tools/binman/test/053_symbols.dts
tools/binman/test/149_symbols_tpl.dts
tools/binman/test/155_symbols_tpl_x86.dts
tools/binman/test/187_symbols_sub.dts
tools/binman/test/u_boot_binman_syms.c
tools/binman/test/u_boot_binman_syms_size.c

index 5dd122611c27db104bb5a6c7b9c618b89a25e66c..29e0898f03de15b1dd01c675fd24c3fad00ed0eb 100644 (file)
@@ -41,6 +41,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
index 8586ef8731b683d2c11aa9364fdc1ce4b9366e1d..528d7e4e90ed321f3baf0f637621fa960b568532 100644 (file)
@@ -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)
                __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
  *
  * @_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) */
 
 
 #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) */
index afa05e58fdd85e947e621fbd3b9d18b78d452f04..6d440ddf21df663841b413fccf617f7de8bf1cec 100644 (file)
@@ -25,6 +25,9 @@ try:
 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()
index 02bc108374925165676e933719f7bf6ba6e62c50..5a51c64cfee32812bf1a94d5b43050a725a9c119 100644 (file)
@@ -127,7 +127,7 @@ class TestElf(unittest.TestCase):
         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 @@ class TestElf(unittest.TestCase):
         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):
index b5cf549703ad2d6ef140b5b13ca44de359a59366..fa1f421c052165f78d744cf5e8c89e750ae9d511 100644 (file)
@@ -43,8 +43,8 @@ from patman import tout
 # 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 @@ class TestFunctional(unittest.TestCase):
         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 @@ class TestFunctional(unittest.TestCase):
         # 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 @@ class TestFunctional(unittest.TestCase):
 
     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 @@ class TestFunctional(unittest.TestCase):
         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 @@ class TestFunctional(unittest.TestCase):
 
     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"""
index 1ff8dab296fcff8c6237b996ad529f882c47fe7f..c5abbbcdd6abe6af3a408d22c1bd287b31f0e75d 100644 (file)
@@ -10,7 +10,7 @@
                };
 
                u-boot {
-                       offset = <24>;
+                       offset = <28>;
                };
        };
 };
index b79d9adf68271db3ee7d50e79af675adab09c283..b54f9b14191d8d27ee269883ab26ebc9443eee0e 100644 (file)
@@ -7,7 +7,7 @@
        binman {
                sort-by-offset;
                u-boot {
-                       offset = <26>;
+                       offset = <30>;
                };
 
                u-boot-spl {
index 11a1f6059e263c8f1db70ad36e34ef456c89eed3..b6ad7fb56a534efcb1a22d46975d43cfcb92d37f 100644 (file)
@@ -13,7 +13,7 @@
                };
 
                u-boot-spl {
-                       offset = <0xffffffe7>;
+                       offset = <0xffffffe3>;
                };
        };
 };
index 88aa007bbae19586402cdb2bab04eb6e56147d0b..ad8f9d6e1bdd35ebba7681a9963fd055fc08186a 100644 (file)
@@ -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>;
                };
        };
 };
index 2965809276480848744b0387ce36f69903453637..b28f34a72faafdd52dc6790c43af9678fd9b036b 100644 (file)
@@ -10,7 +10,7 @@
                };
 
                u-boot {
-                       offset = <0x18>;
+                       offset = <0x1c>;
                };
 
                u-boot-spl2 {
index 0a4ab3f1fabdd321b5fa49cc4071686678efe153..4e649c45978d49c0629c2cf5a6f76561488b7891 100644 (file)
                };
 
                u-boot-spl2 {
-                       offset = <0x1c>;
+                       offset = <0x20>;
                        type = "u-boot-spl";
                };
 
                u-boot {
-                       offset = <0x34>;
+                       offset = <0x3c>;
                };
 
                section {
index 9d7dc51b3d9b68c4fa78b2ad21c7215f9e50827f..e1ce33e67fbc7f180efaeca39f3d1b57518afc6a 100644 (file)
                };
 
                u-boot-spl2 {
-                       offset = <0xffffff1c>;
+                       offset = <0xffffff20>;
                        type = "u-boot-spl";
                };
 
                u-boot {
-                       offset = <0xffffff34>;
+                       offset = <0xffffff3c>;
                };
 
                section {
index 54511a737112eb903a1a6acc88e9e4c280ba1e87..3ab62d37215261de90f7cdbac39da1b2fad23e5c 100644 (file)
@@ -11,7 +11,7 @@
                        };
 
                        u-boot {
-                               offset = <24>;
+                               offset = <28>;
                        };
                };
 
index 89fee5567e1d4f7bf99fd07fb6f9e278869b676c..ed761246aec751030b5b7d6ebfa4ba5427cd5460 100644 (file)
@@ -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);
index c4a053f96f1a1ce9865d1f1e95054d5046b9f57a..fa41b3d9a332cf6be1e2922f8b39b12c0defaf57 100644 (file)
@@ -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);