From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33203) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X6i4T-0003Ib-10 for qemu-devel@nongnu.org; Mon, 14 Jul 2014 11:21:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1X6i4M-00084E-Ow for qemu-devel@nongnu.org; Mon, 14 Jul 2014 11:21:40 -0400 Received: from cantor2.suse.de ([195.135.220.15]:44314 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1X6i4M-00083T-IA for qemu-devel@nongnu.org; Mon, 14 Jul 2014 11:21:34 -0400 Message-ID: <53C3F57D.4080509@suse.de> Date: Mon, 14 Jul 2014 17:21:33 +0200 From: Alexander Graf MIME-Version: 1.0 References: <1405348708-13909-1-git-send-email-Joakim.Tjernlund@transmode.se> In-Reply-To: <1405348708-13909-1-git-send-email-Joakim.Tjernlund@transmode.se> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] linux-user: Add binfmt wrapper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Joakim Tjernlund Cc: qemu-devel@nongnu.org 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 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); >