[U-Boot] [PATCH v2] ARM64: zynqmp: Add support for standard distro boot commands

Michal Simek
2016-06-02T08:51:59Z

On 2.6.2016 10:30, Alexander Graf wrote:
>
>
> On 02.06.16 10:22, Michal Simek wrote:
>> Nand and QSPI are not defined now but this will be extended.
>> Based on selected bootmode boot_targets are rewritten.
>> Patch also contains detection if variables are saved. If yes don't
>> rewrite boot_targets variable.
>>
>> Also move variable setup to the end of file because SCSI needs to be
>> defined before others macros are using it.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> Changes in v2:
>> - Append default boot_targets to the list
>>
>> Patch depends on
>> "env: Setup GD_FLG_ENV_DEFAULT flag when default environment are used"
>>
>> ---
>> board/xilinx/zynqmp/zynqmp.c | 27 ++++++++++++++-----
>> include/configs/xilinx_zynqmp.h | 59 ++++++++++++++++++++++++++++++-----------
>> 2 files changed, 64 insertions(+), 22 deletions(-)
>>
>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>> index 4623cd49e9c7..204f8c526ab4 100644
>> --- a/board/xilinx/zynqmp/zynqmp.c
>> +++ b/board/xilinx/zynqmp/zynqmp.c
>> @@ -215,6 +215,11 @@ int board_late_init(void)
>> u32 reg = 0;
>> u8 bootmode;
>>
>> + if (!(gd->flags & GD_FLG_ENV_DEFAULT)) {
>> + debug("Saved variables - Skipping\n");
>> + return 0;
>> + }
>> +
>> reg = readl(&crlapb_base->boot_mode);
>> bootmode = reg & BOOT_MODES_MASK;
>>
>> @@ -222,31 +227,39 @@ int board_late_init(void)
>> switch (bootmode) {
>> case JTAG_MODE:
>> puts("JTAG_MODE\n");
>> - setenv("modeboot", "jtagboot");
>> + setenv("boot_targets", strcat("pxe dhcp ",
>> + getenv("boot_targets")));
>
> The strcat() function appends the src string to the dest
> string, overwriting the terminating null byte ('\0') at the end of dest,
> and then adds a terminating null byte. The strings may
> not overlap, and the dest string must have enough space for the
> result. If dest is not large enough, program behavior is unpredictable;
> buffer overruns are a favorite avenue for attacking
> secure programs.
>
> ---
>
> In other words, the code above creates a buffer overflow :). You need
> something like
>
> const char *new_targets = "pxe dhcp"; // <- make this a parameter to a
> function
>
> new_targets = malloc(strlen(new_targets) +
> strlen(getenv("boot_targets") + 2); // one byte for the space, one for
> the null-terminator
> sprintf(new_targets, "%s %s", new_targets, boot_targets);
> setenv("boot_targets", new_targets);
>
> Isn't string handling in C awesome? It's almost as readable and easy as
> doing it in assembly.

Time for holiday.

Thanks,
Michal