Re: [Qemu-devel] [PATCH] linux-user: Add binfmt wrapper

[Alexander Graf <agraf@suse.de>]

On 14.07.14 16:38, Joakim Tjernlund wrote:
> The popular binfmt-wrapper patch adds an additional
> executable which mangle argv suitable for binfmt flag P.
> In a chroot you need the both (statically linked) qemu-$arch
> and qemu-$arch-binfmt-wrapper. This is sub optimal and a
> better approach is to recognize the -binfmt-wrapper extension
> within linux-user(qemu-$arch) and mangle argv there.
> This just produces on executable which can be either copied to
> the chroot or bind mounted with the appropriate -binfmt-wrapper
> suffix.
>
> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>

Please make sure to CC Riku on patches like this - he is the linux-user 
maintainer.

> ---
>   linux-user/main.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 71a33c7..212067a 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3828,6 +3828,19 @@ int main(int argc, char **argv, char **envp)
>       int i;
>       int ret;
>       int execfd;
> +    char *binfmt;
> +
> +    i = strlen( argv[0] ) - strlen ( "-binfmt-wrapper" );

The spaces are odd. Did this patch pass checkpatch.pl? Same comment goes 
for almost all function invocations.

> +    binfmt = argv[0] + i;
> +    if (i > 0 && strcmp ( binfmt, "-binfmt-wrapper" ) == 0) {

This magic needs to be documented somewhere. In fact, I find it pretty 
hard to use in real world scenarios. Imagine a distribution - should it 
package every target binary twice? Should it create hardlinks all over?

I think we should try and find better magic :). Looking at the 
binfmt_misc loading code, I think we can cheat a bit. If we pass the 'O' 
flag (open target binary for handler), binfmt_misc will tell us the 
binary fd in AT_EXECFD:

                 NEW_AUX_ENT(AT_EXECFD, bprm->interp_data);

We could then use this as a hint that we were spawned by binfmt_misc 
rather than directly and interpret the first argv as target_argv[0].

Then we can also add the P and O flags to scripts/qemu-binfmt-conf.sh 
and have a solution that works well for everyone.

> +	if (argc < 3 ) {
> +	    fprintf ( stderr, "%s: Please use me through binfmt with P flag\n", argv[0] );
> +	    exit(1);
> +	}
> +	handle_arg_argv0(argv[2]); /* binfmt wrapper */
> +	memmove(&argv[2], &argv[3], (argc-2)*sizeof(argv));

I can't say I'm a big fan of this memmove, but everything else I can 
think of is going to be even uglier.


Alex

> +	argc--;
> +    }
>   
>       module_call_init(MODULE_INIT_QOM);
>