[U-Boot] [PATCH] socfpga: Add a signing tool that automatically signs the preloader.
No need to use the altera tool any more...
Signed-off-by: Charles Manning cdhmanning@gmail.com --- spl/Makefile | 8 ++ tools/Makefile | 9 +- tools/socfpga-signer.c | 294 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 310 insertions(+), 1 deletion(-) create mode 100644 tools/socfpga-signer.c
diff --git a/spl/Makefile b/spl/Makefile index c0abe3d..086b1e6 100644 --- a/spl/Makefile +++ b/spl/Makefile @@ -144,8 +144,12 @@ $(OBJTREE)/MLO: $(obj)u-boot-spl.bin
$(OBJTREE)/MLO.byteswap: $(obj)u-boot-spl.bin $(OBJTREE)/tools/mkimage -T omapimage -n byteswap \ + -a $(CONFIG_SPL_TEXT_BASE) -d $< $@
+$(OBJTREE)/socfpga-signed-preloader.bin: $(obj)u-boot-spl.bin + $(OBJTREE)/tools/socfpga-signer -i $< -o $@ -p + ifneq ($(CONFIG_IMX_CONFIG),) $(OBJTREE)/SPL: $(obj)u-boot-spl.bin $(OBJTREE)/tools/mkimage -n $(SRCTREE)/$(CONFIG_IMX_CONFIG) -T imximage \ @@ -154,6 +158,10 @@ endif
ALL-y += $(obj)u-boot-spl.bin
+ifdef CONFIG_SOCFPGA +ALL-y += $(OBJTREE)/socfpga-signed-preloader.bin +endif + ifdef CONFIG_SAMSUNG ALL-y += $(obj)$(BOARD)-spl.bin endif diff --git a/tools/Makefile b/tools/Makefile index 686840a..1ed2281 100644 --- a/tools/Makefile +++ b/tools/Makefile @@ -73,7 +73,8 @@ BIN_FILES-$(CONFIG_MX28) += mxsboot$(SFX) BIN_FILES-$(CONFIG_NETCONSOLE) += ncb$(SFX) BIN_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1$(SFX) BIN_FILES-$(CONFIG_KIRKWOOD) += kwboot$(SFX) - +BIN_FILES-$(CONFIG_SOCFPGA) += socfpga-signer$(SFX) + # Source files which exist outside the tools directory EXT_OBJ_FILES-$(CONFIG_BUILD_ENVCRC) += common/env_embedded.o EXT_OBJ_FILES-y += common/image.o @@ -104,6 +105,7 @@ NOPED_OBJ_FILES-y += os_support.o OBJ_FILES-$(CONFIG_SHA1_CHECK_UB_IMG) += ubsha1.o NOPED_OBJ_FILES-y += ublimage.o OBJ_FILES-$(CONFIG_KIRKWOOD) += kwboot.o +OBJ_FILES-$(CONFIG_SOCFPGA) += socfpga-signer.o
# Don't build by default #ifeq ($(ARCH),ppc) @@ -243,6 +245,11 @@ $(obj)kwboot$(SFX): $(obj)kwboot.o $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ $(HOSTSTRIP) $@
+ +$(obj)socfpga-signer$(SFX): $(obj)socfpga-signer.o + $(HOSTCC) $(HOSTCFLAGS) $(HOSTLDFLAGS) -o $@ $^ + $(HOSTSTRIP) $@ + # Some of the tool objects need to be accessed from outside the tools directory $(obj)%.o: $(SRCTREE)/common/%.c $(HOSTCC) -g $(HOSTCFLAGS_NOPED) -c -o $@ $< diff --git a/tools/socfpga-signer.c b/tools/socfpga-signer.c new file mode 100644 index 0000000..4861066 --- /dev/null +++ b/tools/socfpga-signer.c @@ -0,0 +1,294 @@ + +/* Reference doc http://www.altera.com.cn/literature/hb/cyclone-v/cv_5400A.pdf +* + * Header is a structure of the following format. + * this is positioned at 0x40. + * + * Endian is LSB. + * + * Offset Length Usage + * ----------------------- + * 0 4 Validation word 0x31305341 + * 4 1 Version (whatever, zero is fine) + * 5 1 Flags (unused, zero is fine) + * 6 2 Length (in units of u32, including the end checksum). + * 8 2 Zero + * 0x0A 2 Checksum over the heder. NB Not CRC32 + * + * At the end of the code we have a 32-bit CRC checksum over whole binary + * excluding the CRC. This uses the CRC32 calc out of the well known Apple + * crc32.c code. + * + */ + +#include <stdio.h> +#include <fcntl.h> +#include <stdint.h> +#include <unistd.h> +#include <stdlib.h> +#include <string.h> + + +#define HEADER_OFFSET 0x40 +#define HEADER_SIZE 0x0C +#define BUFFER_SIZE (0x10000) +#define MAX_IMAGE_SIZE 0xFF00 +#define VALIDATION_WORD 0x31305341 +#define FILL_BYTE 0x00 + + +static uint16_t hdr_checksum(uint8_t *buf, int len) +{ + uint16_t ret = 0; + int i; + + for(i = 0; i < len; i++) { + ret += (((uint16_t) *buf) & 0xff); + buf++; + } + return ret; +} + + +static void le16(uint8_t *buf, uint16_t v) +{ + buf[0] = (v >> 0) & 0xff; + buf[1] = (v >> 8) & 0xff; +} + +static void le32(uint8_t *buf, uint32_t v) +{ + buf[0] = (v >> 0) & 0xff; + buf[1] = (v >> 8) & 0xff; + buf[2] = (v >> 16) & 0xff; + buf[3] = (v >> 24) & 0xff; +} + +static int align4(int v) +{ + return ((v + 3) / 4) * 4; +} + +static void build_header(uint8_t *buf, + uint8_t version, + uint8_t flags, + uint16_t length_bytes) +{ + memset(buf, 0, HEADER_SIZE); + + le32(buf + 0, VALIDATION_WORD); + buf[4] = version; + buf[5] = flags; + le16(buf + 6, length_bytes/4); + le16(buf + 10, hdr_checksum(buf, 10)); +} + +static uint32_t crc_table[256] = { + + 0x00000000, 0x04c11db7, 0x09823b6e, 0x0d4326d9, + 0x130476dc, 0x17c56b6b, 0x1a864db2, 0x1e475005, + 0x2608edb8, 0x22c9f00f, 0x2f8ad6d6, 0x2b4bcb61, + 0x350c9b64, 0x31cd86d3, 0x3c8ea00a, 0x384fbdbd, + 0x4c11db70, 0x48d0c6c7, 0x4593e01e, 0x4152fda9, + 0x5f15adac, 0x5bd4b01b, 0x569796c2, 0x52568b75, + 0x6a1936c8, 0x6ed82b7f, 0x639b0da6, 0x675a1011, + 0x791d4014, 0x7ddc5da3, 0x709f7b7a, 0x745e66cd, + 0x9823b6e0, 0x9ce2ab57, 0x91a18d8e, 0x95609039, + 0x8b27c03c, 0x8fe6dd8b, 0x82a5fb52, 0x8664e6e5, + 0xbe2b5b58, 0xbaea46ef, 0xb7a96036, 0xb3687d81, + 0xad2f2d84, 0xa9ee3033, 0xa4ad16ea, 0xa06c0b5d, + 0xd4326d90, 0xd0f37027, 0xddb056fe, 0xd9714b49, + 0xc7361b4c, 0xc3f706fb, 0xceb42022, 0xca753d95, + 0xf23a8028, 0xf6fb9d9f, 0xfbb8bb46, 0xff79a6f1, + 0xe13ef6f4, 0xe5ffeb43, 0xe8bccd9a, 0xec7dd02d, + 0x34867077, 0x30476dc0, 0x3d044b19, 0x39c556ae, + 0x278206ab, 0x23431b1c, 0x2e003dc5, 0x2ac12072, + 0x128e9dcf, 0x164f8078, 0x1b0ca6a1, 0x1fcdbb16, + 0x018aeb13, 0x054bf6a4, 0x0808d07d, 0x0cc9cdca, + 0x7897ab07, 0x7c56b6b0, 0x71159069, 0x75d48dde, + 0x6b93dddb, 0x6f52c06c, 0x6211e6b5, 0x66d0fb02, + 0x5e9f46bf, 0x5a5e5b08, 0x571d7dd1, 0x53dc6066, + 0x4d9b3063, 0x495a2dd4, 0x44190b0d, 0x40d816ba, + 0xaca5c697, 0xa864db20, 0xa527fdf9, 0xa1e6e04e, + 0xbfa1b04b, 0xbb60adfc, 0xb6238b25, 0xb2e29692, + 0x8aad2b2f, 0x8e6c3698, 0x832f1041, 0x87ee0df6, + 0x99a95df3, 0x9d684044, 0x902b669d, 0x94ea7b2a, + 0xe0b41de7, 0xe4750050, 0xe9362689, 0xedf73b3e, + 0xf3b06b3b, 0xf771768c, 0xfa325055, 0xfef34de2, + 0xc6bcf05f, 0xc27dede8, 0xcf3ecb31, 0xcbffd686, + 0xd5b88683, 0xd1799b34, 0xdc3abded, 0xd8fba05a, + 0x690ce0ee, 0x6dcdfd59, 0x608edb80, 0x644fc637, + 0x7a089632, 0x7ec98b85, 0x738aad5c, 0x774bb0eb, + 0x4f040d56, 0x4bc510e1, 0x46863638, 0x42472b8f, + 0x5c007b8a, 0x58c1663d, 0x558240e4, 0x51435d53, + 0x251d3b9e, 0x21dc2629, 0x2c9f00f0, 0x285e1d47, + 0x36194d42, 0x32d850f5, 0x3f9b762c, 0x3b5a6b9b, + 0x0315d626, 0x07d4cb91, 0x0a97ed48, 0x0e56f0ff, + 0x1011a0fa, 0x14d0bd4d, 0x19939b94, 0x1d528623, + 0xf12f560e, 0xf5ee4bb9, 0xf8ad6d60, 0xfc6c70d7, + 0xe22b20d2, 0xe6ea3d65, 0xeba91bbc, 0xef68060b, + 0xd727bbb6, 0xd3e6a601, 0xdea580d8, 0xda649d6f, + 0xc423cd6a, 0xc0e2d0dd, 0xcda1f604, 0xc960ebb3, + 0xbd3e8d7e, 0xb9ff90c9, 0xb4bcb610, 0xb07daba7, + 0xae3afba2, 0xaafbe615, 0xa7b8c0cc, 0xa379dd7b, + 0x9b3660c6, 0x9ff77d71, 0x92b45ba8, 0x9675461f, + 0x8832161a, 0x8cf30bad, 0x81b02d74, 0x857130c3, + 0x5d8a9099, 0x594b8d2e, 0x5408abf7, 0x50c9b640, + 0x4e8ee645, 0x4a4ffbf2, 0x470cdd2b, 0x43cdc09c, + 0x7b827d21, 0x7f436096, 0x7200464f, 0x76c15bf8, + 0x68860bfd, 0x6c47164a, 0x61043093, 0x65c52d24, + 0x119b4be9, 0x155a565e, 0x18197087, 0x1cd86d30, + 0x029f3d35, 0x065e2082, 0x0b1d065b, 0x0fdc1bec, + 0x3793a651, 0x3352bbe6, 0x3e119d3f, 0x3ad08088, + 0x2497d08d, 0x2056cd3a, 0x2d15ebe3, 0x29d4f654, + 0xc5a92679, 0xc1683bce, 0xcc2b1d17, 0xc8ea00a0, + 0xd6ad50a5, 0xd26c4d12, 0xdf2f6bcb, 0xdbee767c, + 0xe3a1cbc1, 0xe760d676, 0xea23f0af, 0xeee2ed18, + 0xf0a5bd1d, 0xf464a0aa, 0xf9278673, 0xfde69bc4, + 0x89b8fd09, 0x8d79e0be, 0x803ac667, 0x84fbdbd0, + 0x9abc8bd5, 0x9e7d9662, 0x933eb0bb, 0x97ffad0c, + 0xafb010b1, 0xab710d06, 0xa6322bdf, 0xa2f33668, + 0xbcb4666d, 0xb8757bda, 0xb5365d03, 0xb1f740b4 +}; + +uint32_t crc32(uint32_t crc, void *_buf, int length) +{ + uint8_t *buf = _buf; + + crc ^= ~0; + + while (length--) { + crc = (crc << 8) ^ crc_table[((crc >> 24) ^ *buf) & 0xff]; + buf++; + } + + crc ^= ~0; + + return crc; +} + +static int massage_buffer(uint8_t *buf, + uint8_t version, uint8_t flags, + int len, int pad_64k) +{ + uint32_t crcval; + + /* Align the length up */ + len = align4(len); + + /* Build header, adding 4 bytes to length to hold the CRC32. */ + build_header(buf + HEADER_OFFSET, version, flags, len + 4); + + crcval = crc32(0, buf, len); + //crcval = crc_calc ((uint32_t *)buf, len/4); + printf("crc %x\n", crcval); + + le32(buf + len, crcval); + + if(!pad_64k) + return len + 4; + + return (1<<16); +} + +static uint8_t buffer[BUFFER_SIZE]; +static uint8_t flags; +static uint8_t version; +static int pad_64k; +static int n_copies=1; + +static char *in_filename = ""; +static char *out_filename = ""; + + + +static void bad_exit(const char *str) +{ + printf("%s\n", str); + printf("options are:\n" + " -i file_name input file\n" + " -o file_name output file\n" + " -p pad to 64k\n" + " -n n_copies number or repeat copies. Implies -p\n"); + exit(1); +} + +static int parse_opts(int argc, char *argv[]) +{ + int opt; + + while ((opt = getopt(argc, argv, "i:o:n:p")) != -1) { + + switch (opt) { + case 'i': + in_filename = strdup(optarg); + break; + case 'o': + out_filename = strdup(optarg); + break; + case 'n': + n_copies = atoi(optarg); + pad_64k = 1; + break; + case 'p': + pad_64k = 1; + break; + default: + bad_exit("Bad argument"); + return -1; + } + } + + return 0; +} + + +int main (int argc, char *argv[]) +{ + int inh; + int outh; + int len; + int wrote; + int total_write = 0; + int i; + + memset(buffer, FILL_BYTE, sizeof(buffer)); + + if (parse_opts(argc, argv) < 0) + bad_exit("Bad argument"); + + inh = open(in_filename, O_RDONLY); + + if (inh < 0) + bad_exit("Could not open input file"); + + len = read(inh, buffer, sizeof(buffer)); + + close(inh); + + if (len < 0 || len > MAX_IMAGE_SIZE -4) + bad_exit("Input file too short or too long"); + + printf("Input file %s is %d (0x%04x) bytes long\n", in_filename, len, len); + + len = massage_buffer(buffer, version, flags, len, pad_64k); + + + + outh = open(out_filename, O_CREAT | O_TRUNC | O_RDWR, 0666); + + for(i=0; i < n_copies; i++) { + wrote = write(outh, buffer, len); + if (wrote != len) + bad_exit("Bad write to output file"); + total_write += wrote; + } + + close(outh); + + printf("Output file %s is %d (0x%04x) bytes long\n", out_filename, total_write, total_write); + + return 0; + +} +
On Thu, Feb 20, 2014 at 12:01:40PM +1300, Charles Manning wrote:
No need to use the altera tool any more...
Signed-off-by: Charles Manning cdhmanning@gmail.com
[snip]
- //crcval = crc_calc ((uint32_t *)buf, len/4);
- printf("crc %x\n", crcval);
Debug stuff left in?
+static char *out_filename = "";
+static void bad_exit(const char *str)
Extra spaces.
Finally, we can't do this as an extension of mkimage? We do other headers with that already, does it really not fit that model? Thanks!
Hi Tom
On Thursday 20 February 2014 13:32:37 Tom Rini wrote:
On Thu, Feb 20, 2014 at 12:01:40PM +1300, Charles Manning wrote:
No need to use the altera tool any more...
Signed-off-by: Charles Manning cdhmanning@gmail.com
[snip]
- //crcval = crc_calc ((uint32_t *)buf, len/4);
- printf("crc %x\n", crcval);
Debug stuff left in?
+static char *out_filename = "";
+static void bad_exit(const char *str)
Extra spaces.
Thanks for the feedback.
I will fix those and resubmit.
Finally, we can't do this as an extension of mkimage? We do other headers with that already, does it really not fit that model? Thanks!
I looked at doing a mkimage integration, but is this really the way to go in all cases?
The signed image does not really apply a header as such. It mashes in a "header" at position 0x40 and then appends a checksum at the end, then pads the file out to 64k.
The final file is: * 64 bytes of executable (interrupt vector table) * "header" * more code *crc32 *padding with 0x00 to 64k.
Adding it to mkimage and "cooking" the binary was not too hard. I could not figure out how to extend the size of the buffer and pad it out to 64k. I'm sure I could figure that out... it's just software..
If it really **must** be part of mkimage, I'll do that.
If so, what mkimage operation need to be supported?
Changing type makes no sense (apart from signing a binary). There are no uboot-format header/marker or anything. Verification probably makes sense, but that's it.
Thoughts??
-- Charles
Dear Charles,
In message 201402201358.19495.manningc2@actrix.gen.nz you wrote:
I looked at doing a mkimage integration, but is this really the way to go in all cases?
...only where it fits, of course.
The signed image does not really apply a header as such. It mashes in a "header" at position 0x40 and then appends a checksum at the end, then pads the file out to 64k.
The final file is:
- 64 bytes of executable (interrupt vector table)
- "header"
- more code
*crc32 *padding with 0x00 to 64k.
Sounds exactly as if you were making an image, so yes, mkimage fits.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de Humanity has the stars in its future, and that future is too important to be lost under the burden of juvenile folly and ignorant superstition. - Isaac Asimov
On Thu, Feb 20, 2014 at 12:23:22PM +0100, Wolfgang Denk wrote:
Dear Charles,
In message 201402201358.19495.manningc2@actrix.gen.nz you wrote:
I looked at doing a mkimage integration, but is this really the way to go in all cases?
...only where it fits, of course.
The signed image does not really apply a header as such. It mashes in a "header" at position 0x40 and then appends a checksum at the end, then pads the file out to 64k.
The final file is:
- 64 bytes of executable (interrupt vector table)
- "header"
- more code
*crc32 *padding with 0x00 to 64k.
Sounds exactly as if you were making an image, so yes, mkimage fits.
Right. This is like the omapimage case or similar where we take input, beat it around and add stuff, then have what the HW wants, but not a file we can further query with mkimage, and that's fine.
On Thu, Feb 20, 2014 at 12:01 +1300, Charles Manning wrote:
No need to use the altera tool any more...
I like the idea of removing this external dependency. But there are some issues with your submission.
The commit message could use some more helpful text for those who are not familiar with the details of how the preloader gets mangled before it becomes acceptable to the ROM loader.
The patch does not apply to current U-Boot source code, can you re-check please? Although this might have been because master has moved between your sending the patch and my trying to apply it.
Then there are whitespace issues, make sure to check the coding style before submission. Other comments are further down below.
--- a/spl/Makefile +++ b/spl/Makefile @@ -144,8 +144,12 @@ $(OBJTREE)/MLO: $(obj)u-boot-spl.bin
$(OBJTREE)/MLO.byteswap: $(obj)u-boot-spl.bin $(OBJTREE)/tools/mkimage -T omapimage -n byteswap \
- -a $(CONFIG_SPL_TEXT_BASE) -d $< $@
this looks suspicious: there has been a continued line, which you now continue with nothing, and have the remainder stand alone? Does it even work?
--- /dev/null +++ b/tools/socfpga-signer.c @@ -0,0 +1,294 @@
+[ ... ]
+#include <stdio.h> +#include <fcntl.h> +#include <stdint.h> +#include <unistd.h> +#include <stdlib.h> +#include <string.h>
alpha-sort those, makes checking for duplicates easier and reduces conflicts during maintenance
+#define HEADER_OFFSET 0x40 +#define HEADER_SIZE 0x0C +#define BUFFER_SIZE (0x10000) +#define MAX_IMAGE_SIZE 0xFF00 +#define VALIDATION_WORD 0x31305341 +#define FILL_BYTE 0x00
there's no point in the parentheses around a single value
isn't the maximum preloader size said to be 60KiB? aka 64KiB minus 4KiB for internal use by the ROM loader? that would be 0x1000 less than 0x10000, not just 0x100
+static uint16_t hdr_checksum(uint8_t *buf, int len) +{
- uint16_t ret = 0;
- int i;
- for(i = 0; i < len; i++) {
ret += (((uint16_t) *buf) & 0xff);buf++;- }
- return ret;
+}
the cast and masking should be unnecessary, you are dereferencing a pointer to a byte after all
+static void le16(uint8_t *buf, uint16_t v) +{
- buf[0] = (v >> 0) & 0xff;
- buf[1] = (v >> 8) & 0xff;
+}
+static void le32(uint8_t *buf, uint32_t v) +{
- buf[0] = (v >> 0) & 0xff;
- buf[1] = (v >> 8) & 0xff;
- buf[2] = (v >> 16) & 0xff;
- buf[3] = (v >> 24) & 0xff;
+}
can you come up with more descriptive names? it took a while before I noticed (at call sites) that "le16()" not only did conversion, but manipulated a buffer as well
does "put_le16()" sound better to you?
+static int align4(int v) +{
- return ((v + 3) / 4) * 4;
+}
isn't there some ROUNDUP() macro already? (it is in U-Boot, but I'm not sure for user space)
+static void build_header(uint8_t *buf,
uint8_t version,uint8_t flags,uint16_t length_bytes)+{
- memset(buf, 0, HEADER_SIZE);
- le32(buf + 0, VALIDATION_WORD);
- buf[4] = version;
- buf[5] = flags;
- le16(buf + 6, length_bytes/4);
- le16(buf + 10, hdr_checksum(buf, 10));
+}
+static uint32_t crc_table[256] = {
- 0x00000000, 0x04c11db7, 0x09823b6e, 0x0d4326d9,
- 0x130476dc, 0x17c56b6b, 0x1a864db2, 0x1e475005,
- [ ... ]
- 0xafb010b1, 0xab710d06, 0xa6322bdf, 0xa2f33668,
- 0xbcb4666d, 0xb8757bda, 0xb5365d03, 0xb1f740b4
+};
+uint32_t crc32(uint32_t crc, void *_buf, int length) +{
- uint8_t *buf = _buf;
- crc ^= ~0;
- while (length--) {
crc = (crc << 8) ^ crc_table[((crc >> 24) ^ *buf) & 0xff];buf++;- }
- crc ^= ~0;
- return crc;
+}
are you re-inventing CRC32 here? is there nothing that you can re-use?
+static int massage_buffer(uint8_t *buf,
uint8_t version, uint8_t flags,int len, int pad_64k)+{
- uint32_t crcval;
- /* Align the length up */
- len = align4(len);
- /* Build header, adding 4 bytes to length to hold the CRC32. */
- build_header(buf + HEADER_OFFSET, version, flags, len + 4);
- crcval = crc32(0, buf, len);
- //crcval = crc_calc ((uint32_t *)buf, len/4);
- printf("crc %x\n", crcval);
dead code? remainders from during development? should go
- le32(buf + len, crcval);
- if(!pad_64k)
return len + 4;- return (1<<16);
+}
is this not BUFFER_SIZE? phrasing the same condition differently obfuscates what happens, and makes you miss stuff during maintenance
virtually yours Gerhard Sittig
participants (4)
-
Charles Manning -
Gerhard Sittig -
Tom Rini -
Wolfgang Denk