[PATCH 4/5] sandbox: implement reset

Heinrich Schuchardt xypron.glpk at gmx.de
Tue Oct 27 14:33:30 CET 2020


On 27.10.20 13:12, Rasmus Villemoes wrote:
> On 25/10/2020 07.04, Heinrich Schuchardt wrote:
>> Up to now the sandbox would shutdown upon a cold reset request. Instead it
>> should be reset.
>>
>> In our coding we use static variables. The only safe way to return to an
>> initial state is to relaunch the U-Boot binary.
>>
>> The reset implementation uses a longjmp() to return to the main() function
>> and then relaunches U-Boot using execv().
>>
>
> That seems to be needlessly fragile.
>
> 1. getopt_long can permute the elements of the argv array
> 2. From reading "man longjmp", I'm not sure argc and argv are actually
> guaranteed to have the values they had originally upon reaching the
> setjmp() the second time
>
> Now, 1. is probably mostly when there's a mix of options and positional
> arguments, and ./u-boot doesn't take the latter. And 2. possibly also
> doesn't apply because we don't currently modify argc or argv in main()
> itself - but that could change with some future refactoring.
>
> So perhaps it works, and maybe that's even guaranteed with the current
> code and APIs that are used. But, is there any reason to muck with a
> complex beast like setjmp/longjmp when we could just

1) argc is passed by value.

argv is defined as "char * const argv[]". It is interesting that
getopt_long() ignores the const keyword which explicitly forbids
permuting the sequence. Cf.
https://sourceware.org/git/?p=glibc.git;a=blob;f=posix/getopt.c;h=8d251dd5fabb46e17f5d593bf12d0619a8867bee;hb=HEAD#l730

But even if the sequence is permuted why should the result be different
if getopt_long is called again?

2) longjmp() restores all registers including the stack pointer. argc
and &argv[0] are on the stack or in a register depending on the
architecture. Local variables() in main are also on the stack.

So the only "fragile" thing is the stack. But if U-Boot corrupts the
stack or memory, we are anyway lost.

>
> static char **saved_argv;
>
> os_relaunch(void) {
>   execve(saved_argv[0], saved_argv);
> }
>
> static int save_argv(int argc, char **argv)
> {
>    /* essentially the prologue of your os_relaunch() */
> }
>
> main() {
>   save_argv(argc, argv);
>   ...
> }
>
> (one can argue whether memcpy'ing the argv array is sufficient, or if
> one should really strdup() each element, since one is allowed to modify
> the strings, though again, I don't think we do that currently).

No C program is allowed to change the strings passed to main() in argv.
Unfortunately Simon forgot to add const.

The following does not compile:

int main(int argc, const char *argv[])
{
        argv[0][1] = 'a';
        return 0;
}

Best regards

Heinrich


More information about the U-Boot mailing list