Re: [Qemu-devel] [PATCH] Introduce --enable-binfmt-misc configure option

[Riku Voipio <riku.voipio@iki.fi>]

On Wed, Dec 03, 2008 at 01:29:36PM +0200, Kirill A. Shutemov wrote:
> It makes qemu compatible with binfmt_misc's flags 'P' and 'O'.
> 
> 'P' - preserve-argv[0].  Legacy behavior of binfmt_misc is to overwrite the
>       original argv[0] with the full path to the binary.  When this flag is
>       included, binfmt_misc will add an argument to the argument vector for
>       this purpose, thus preserving the original argv[0].
> 
> 'O' - open-binary. Legacy behavior of binfmt_misc is to pass the full path
>       of the binary to the interpreter as an argument. When this flag is
>       included, binfmt_misc will open the file for reading and pass its
>       descriptor as an argument, instead of the full path, thus allowing
>       the interpreter to execute non-readable binaries.
>
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>
> ---
>  configure              |   90 ++++++++++++++++++++++++++----------------------
>  linux-user/linuxload.c |    7 +---
>  linux-user/main.c      |   39 ++++++++++++++++++++-
>  linux-user/qemu.h      |    2 +-
>  4 files changed, 89 insertions(+), 49 deletions(-)
> 
> diff --git a/configure b/configure
> index 57b3b5a..aeeae72 100755
> --- a/configure
> +++ b/configure
> @@ -122,6 +122,7 @@ kvm="yes"
>  kerneldir=""
>  aix="no"
>  blobs="yes"
> +binfmt_misc="no"
>  
>  # OS specific
>  targetos=`uname -s`
> @@ -380,6 +381,8 @@ for opt do
>    ;;
>    --kerneldir=*) kerneldir="$optarg"
>    ;;
> +  --enable-binfmt-misc) binfmt_misc="yes"
> +  ;;
>    *) echo "ERROR: unknown option $opt"; show_help="yes"
>    ;;
>    esac
> @@ -491,6 +494,7 @@ echo "  --disable-vde            disable support for vde network"
>  echo "  --disable-aio            disable AIO support"
>  echo "  --disable-blobs          disable installing provided firmware blobs"
>  echo "  --kerneldir=PATH         look for kernel includes in PATH"
> +echo "  --enable-binfmt-misc     makes usermode compatible with binfmt_misc's flags 'P' and 'O'"
>  echo ""
>  echo "NOTE: The object files are built at the place where configure is launched"
>  exit 1
> @@ -1041,57 +1045,58 @@ else
>    binsuffix="/bin"
>  fi
>  
> -echo "Install prefix    $prefix"
> -echo "BIOS directory    $prefix$datasuffix"
> -echo "binary directory  $prefix$binsuffix"
> +echo "Install prefix      $prefix"
> +echo "BIOS directory      $prefix$datasuffix"
> +echo "binary directory    $prefix$binsuffix"

Whitespace changes mixed with code changes :-/

> +#include "elf.h"
>  /* For tb_lock */
>  #include "exec-all.h"
>  
> @@ -2214,9 +2215,10 @@ void init_task_state(TaskState *ts)
>      ts->sigqueue_table[i].next = NULL;
>  }
>   
> -int main(int argc, char **argv)
> +int main(int argc, char **argv, char **envp)
>  {
>      const char *filename;
> +    int fd = -1;
>      const char *cpu_model;
>      struct target_pt_regs regs1, *regs = &regs1;
>      struct image_info info1, *info = &info1;
> @@ -2377,7 +2379,40 @@ int main(int argc, char **argv)
>      }
>      *dst = NULL; /* NULL terminate target_environ */
>  
> -    if (loader_exec(filename, argv+optind, target_environ, regs, info) != 0) {
> +#ifdef BINFMT_MISC
> +#if HOST_LONG_BITS == 32
> +#define Elf_Dyn Elf32_Dyn
> +#else 
> +#define Elf_Dyn Elf64_Dyn
> +#endif
> +    {
> +        Elf_Dyn *auxv;
> +
> +        optind++; /* Handle binfmt_misc's option 'P' */
> +
> +        /* Handle binfmt_misc's option 'O' */
> +        while(*envp++ != NULL); /* skip envp. we are on auxv now */
> +        for(auxv = (Elf_Dyn *)envp; auxv->d_tag != AT_NULL; auxv++) {
> +            if( auxv->d_tag == AT_EXECFD) {
> +                fd = auxv->d_un.d_val;
> +                break;
> +            }
> +        }
> +
> +        if (fd < 0) {
> +            printf("Cannot find binary file descriptor\n");
> +            _exit(1);
> +        }
> +    }
> +#else
> +    fd = open(filename, O_RDONLY);
> +    if (fd < 0) {
> +        printf("Cannot open file %s: %s\n", filename, strerror(errno));
> +        _exit(1);
> +    }
> +#endif

If I read this correctly, it means this patch means that linux-user
doesn't work from command line if configured with --enable-binfmt-misc.

I think it would be better to add a wrapper (as recommended by
binfmt-misc docs in kernel) that sets these binfmt options to new qemu
command line arguments ( --argv0, --open-fd). Assuming the binfmt_misc
passed FD survives exec, the wrapper should work fine. This wrapper
could well be shipped with qemu.