Re: [UNIKRAFT PATCH] lib/syscall_shim: Force evaluation order of syscall arguments

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

Hi Felix,

thanks a lot for your work! I remember that we had once an issue of 
swapped argument orders but we did not figure out what caused it. Your 
reasoning makes sense to me and the patch looks good. We will take it 
upstream.
Thanks!

Simon

Reviewed-by: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>

On 02.11.20 18:10, Felix Moebius wrote:
> The file gen_uk_syscall.awk is used to generate the file uk_syscall.c.
> The generated code calls the syscall handlers like this:
>
> uk_syscall_e_fstat(va_arg(arg, long), va_arg(arg, long))
>
> This is a problem since the order in which these va_arg expressions are being
> evaluated is unspecified. This leads to arguments being swapped, depending
> on how gcc decides to evaluate this.
>
> When printing to stdout from musl this leads to an endless loop in
> __stdio_write.
>
> This patch fixes this by introducing temporary variables to store the varargs
> and then passing these variables to the syscall handlers, taking into account
> the number of required arguments.
>
> Signed-off-by: Felix Moebius <felix.moebius@xxxxxxxxx>
> ---
>   lib/syscall_shim/gen_uk_syscall.awk | 13 ++++++++++---
>   1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/lib/syscall_shim/gen_uk_syscall.awk 
> b/lib/syscall_shim/gen_uk_syscall.awk
> index 70c4df4..dc1a379 100644
> --- a/lib/syscall_shim/gen_uk_syscall.awk
> +++ b/lib/syscall_shim/gen_uk_syscall.awk
> @@ -7,6 +7,11 @@ BEGIN {
>   
>   	print "long uk_vsyscall(long nr, va_list arg)\n{"
>         print "\t(void) arg;\n"
> +
> +       printf "\t__maybe_unused long "
> +       for (i = 1; i < max_args; i++)
> +               printf "a%d, ", i
> +       printf "a%d;\n", max_args
>         print "\tswitch (nr) {"
>   }
>   
> @@ -16,11 +21,13 @@ BEGIN {
>         sys_name = "SYS_" name
>         args_nr = $2 + 0
>         printf "\tcase %s:\n", sys_name;
> +       for (i = 1; i <= args_nr; i++)
> +               printf "\t\ta%s = va_arg(arg, long);\n", i;
>         printf "\t\treturn uk_syscall_e_%s(", name;
>         for (i = 1; i < args_nr; i++)
> -               printf("va_arg(arg, long), ")
> +               printf "a%d, ", i;
>         if (args_nr > 0)
> -               printf("va_arg(arg, long)")
> +               printf "a%d", args_nr;
>         printf(");\n")
>   }
>   
> @@ -30,4 +37,4 @@ END {
>         printf "\t\terrno = -ENOSYS;\n"
>         printf "\t\treturn -1;\n"
>         printf "\t}\n}\n"
> -}
> +}
> \ No newline at end of file