[PATCH] linux-user: make binfmt flag O require P

Joakim Tjernlund posted 1 patch 4 years, 4 months ago
Test asan failed
Test checkpatch failed
Test FreeBSD failed
Test docker-mingw@fedora failed
Test docker-clang@ubuntu failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20191214122022.17255-1-joakim.tjernlund@infinera.com
Maintainers: Riku Voipio <riku.voipio@iki.fi>, Laurent Vivier <laurent@vivier.eu>
linux-user/main.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
[PATCH] linux-user: make binfmt flag O require P
Posted by Joakim Tjernlund 4 years, 4 months ago
From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>

QEMU can autodetect if it is started from Linux binfmt loader
when binfmt flag O is on.
Use that and require binfmt flag P as well which will enable QEMU
to pass in correct argv0 to the application.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
---
 linux-user/main.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 6ff7851e86..1b626e5762 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -544,7 +544,7 @@ static void usage(int exitcode)
     exit(exitcode);
 }
 
-static int parse_args(int argc, char **argv)
+static int parse_args(int argc, char **argv, int assume_P_flag)
 {
     const char *r;
     int optind;
@@ -560,7 +560,17 @@ static int parse_args(int argc, char **argv)
             arginfo->handle_opt(r);
         }
     }
-
+    if (assume_P_flag) {
+        /* Assume started by binmisc and binfmt P flag is set */
+        if (argc < 3) {
+            fprintf(stderr, "%s: Please use me through binfmt with P flag\n",
+                    argv[0]);
+            exit(1);
+        }
+        exec_path = argv[1];
+        /* Next argv must be argv0 for the app */
+        return 2;
+    }
     optind = 1;
     for (;;) {
         if (optind >= argc) {
@@ -659,7 +669,8 @@ int main(int argc, char **argv, char **envp)
     qemu_add_opts(&qemu_trace_opts);
     qemu_plugin_add_opts();
 
-    optind = parse_args(argc, argv);
+    execfd = qemu_getauxval(AT_EXECFD);
+    optind = parse_args(argc, argv,  execfd > 0);
 
     if (!trace_init_backends()) {
         exit(1);
@@ -682,7 +693,6 @@ int main(int argc, char **argv, char **envp)
 
     init_qemu_uname_release();
 
-    execfd = qemu_getauxval(AT_EXECFD);
     if (execfd == 0) {
         execfd = open(exec_path, O_RDONLY);
         if (execfd < 0) {
-- 
2.24.1


Re: [PATCH] linux-user: make binfmt flag O require P
Posted by Laurent Vivier 4 years, 4 months ago
Le 14/12/2019 à 13:20, Joakim Tjernlund a écrit :
> From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> 
> QEMU can autodetect if it is started from Linux binfmt loader
> when binfmt flag O is on.
> Use that and require binfmt flag P as well which will enable QEMU
> to pass in correct argv0 to the application.

I agree it's a good idea to try to detect the P flag if we use the O but
it changes the current behavior and breaks the compatibility with
previous version. This will prevent to run old and new version of QEMU
on the same system.

I already try to find a solution to this problem.

The first one is at QEMU level:

https://patchew.org/QEMU/20191024153847.31815-1-laurent@vivier.eu/

Another would be at linux level to provide a way to detect binfmt flags
(like you try to do with AT_EXECFD):

https://lore.kernel.org/patchwork/patch/1158151/

I also found another one from another author:

https://patchwork.kernel.org/patch/10902935/

Thanks,
Laurent


> Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> ---
>  linux-user/main.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 6ff7851e86..1b626e5762 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -544,7 +544,7 @@ static void usage(int exitcode)
>      exit(exitcode);
>  }
>  
> -static int parse_args(int argc, char **argv)
> +static int parse_args(int argc, char **argv, int assume_P_flag)
>  {
>      const char *r;
>      int optind;
> @@ -560,7 +560,17 @@ static int parse_args(int argc, char **argv)
>              arginfo->handle_opt(r);
>          }
>      }
> -
> +    if (assume_P_flag) {
> +        /* Assume started by binmisc and binfmt P flag is set */
> +        if (argc < 3) {
> +            fprintf(stderr, "%s: Please use me through binfmt with P flag\n",
> +                    argv[0]);
> +            exit(1);
> +        }
> +        exec_path = argv[1];
> +        /* Next argv must be argv0 for the app */
> +        return 2;
> +    }
>      optind = 1;
>      for (;;) {
>          if (optind >= argc) {
> @@ -659,7 +669,8 @@ int main(int argc, char **argv, char **envp)
>      qemu_add_opts(&qemu_trace_opts);
>      qemu_plugin_add_opts();
>  
> -    optind = parse_args(argc, argv);
> +    execfd = qemu_getauxval(AT_EXECFD);
> +    optind = parse_args(argc, argv,  execfd > 0);
>  
>      if (!trace_init_backends()) {
>          exit(1);
> @@ -682,7 +693,6 @@ int main(int argc, char **argv, char **envp)
>  
>      init_qemu_uname_release();
>  
> -    execfd = qemu_getauxval(AT_EXECFD);
>      if (execfd == 0) {
>          execfd = open(exec_path, O_RDONLY);
>          if (execfd < 0) {
> 


Re: [PATCH] linux-user: make binfmt flag O require P
Posted by Joakim Tjernlund 4 years, 4 months ago
On Sun, 2019-12-15 at 17:54 +0100, Laurent Vivier wrote:
> 
> Le 14/12/2019 à 13:20, Joakim Tjernlund a écrit :
> > From: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > 
> > QEMU can autodetect if it is started from Linux binfmt loader
> > when binfmt flag O is on.
> > Use that and require binfmt flag P as well which will enable QEMU
> > to pass in correct argv0 to the application.
> 
> I agree it's a good idea to try to detect the P flag if we use the O but
> it changes the current behavior and breaks the compatibility with
> previous version. This will prevent to run old and new version of QEMU
> on the same system.

Only if you already are using O flag only. Distributions can adjust so that O and P
are used in tandem.

> 
> I already try to find a solution to this problem.
> 
> The first one is at QEMU level:
> 
> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchew.org%2FQEMU%2F20191024153847.31815-1-laurent%40vivier.eu%2F&amp;data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7C5c4859947f4949d8824d08d7817f7aef%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637120256850791761&amp;sdata=w6YgDlrUPxk7BSPdOVRyyWzb64usUiF9EMSOMonskPY%3D&amp;reserved=0
> 
> Another would be at linux level to provide a way to detect binfmt flags
> (like you try to do with AT_EXECFD):

The kernel way to report flags are really the way forward here. There seems
to be little interest from QEMU community to fix this though, why is that?


 Jocke
> 
> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fpatchwork%2Fpatch%2F1158151%2F&amp;data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7C5c4859947f4949d8824d08d7817f7aef%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637120256850791761&amp;sdata=abV%2BELktrjTOCS6gEp38%2BYuj17HQfCclj0YhyN0X7Bg%3D&amp;reserved=0
> 
> I also found another one from another author:
> 
> https://nam03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.kernel.org%2Fpatch%2F10902935%2F&amp;data=02%7C01%7Cjoakim.tjernlund%40infinera.com%7C5c4859947f4949d8824d08d7817f7aef%7C285643de5f5b4b03a1530ae2dc8aaf77%7C1%7C1%7C637120256850791761&amp;sdata=TftL59%2Baj2IAAoQ6Femfb8w%2F6%2FGc%2FjrM49iQvw4b1MM%3D&amp;reserved=0
> 
> Thanks,
> Laurent
> 
> 
> > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@infinera.com>
> > ---
> >  linux-user/main.c | 18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> > 
> > diff --git a/linux-user/main.c b/linux-user/main.c
> > index 6ff7851e86..1b626e5762 100644
> > --- a/linux-user/main.c
> > +++ b/linux-user/main.c
> > @@ -544,7 +544,7 @@ static void usage(int exitcode)
> >      exit(exitcode);
> >  }
> > 
> > -static int parse_args(int argc, char **argv)
> > +static int parse_args(int argc, char **argv, int assume_P_flag)
> >  {
> >      const char *r;
> >      int optind;
> > @@ -560,7 +560,17 @@ static int parse_args(int argc, char **argv)
> >              arginfo->handle_opt(r);
> >          }
> >      }
> > -
> > +    if (assume_P_flag) {
> > +        /* Assume started by binmisc and binfmt P flag is set */
> > +        if (argc < 3) {
> > +            fprintf(stderr, "%s: Please use me through binfmt with P flag\n",
> > +                    argv[0]);
> > +            exit(1);
> > +        }
> > +        exec_path = argv[1];
> > +        /* Next argv must be argv0 for the app */
> > +        return 2;
> > +    }
> >      optind = 1;
> >      for (;;) {
> >          if (optind >= argc) {
> > @@ -659,7 +669,8 @@ int main(int argc, char **argv, char **envp)
> >      qemu_add_opts(&qemu_trace_opts);
> >      qemu_plugin_add_opts();
> > 
> > -    optind = parse_args(argc, argv);
> > +    execfd = qemu_getauxval(AT_EXECFD);
> > +    optind = parse_args(argc, argv,  execfd > 0);
> > 
> >      if (!trace_init_backends()) {
> >          exit(1);
> > @@ -682,7 +693,6 @@ int main(int argc, char **argv, char **envp)
> > 
> >      init_qemu_uname_release();
> > 
> > -    execfd = qemu_getauxval(AT_EXECFD);
> >      if (execfd == 0) {
> >          execfd = open(exec_path, O_RDONLY);
> >          if (execfd < 0) {
> >