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

[Joakim Tjernlund]

Alexander Graf <address@hidden> wrote on 2014/07/14 17:21:33:

> From: Alexander Graf <address@hidden>
> To: Joakim Tjernlund <address@hidden>, 
> Cc: address@hidden
> Date: 2014/07/14 17:21
> Subject: Re: [PATCH] linux-user: Add binfmt wrapper
> 
> 
> 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 <address@hidden>
> 
> Please make sure to CC Riku on patches like this - he is the linux-user 
> maintainer.

Doesn't he read the devel list? Anyhow CC:ed

> 
> > ---
> >   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.

ehh, didn't run it through checkpatch.pl. Easy to fix next time.

> 
> > +    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?

How does dists. handle your original binfmt-wrapper? This is not much
different I think. Here you got a choice to create a hardlink or a copy.
Any chroot will only have to bind mount binfmt-wrapper into the chroot or 
lxc container.

> 
> 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.

What to do with P only then? Seems like most dists uses only P 

> 
> > +   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.

Me too :)

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