Re: Please revert git commit 1ad3dcc0

[Bernd Schmidt <bernds_cb1@t-online.de>]

Andrew Morton wrote:
> Bernd Schmidt <bernds_cb1@t-online.de> wrote:
> > please revert 1ad3dcc0.  That was a patch to the binfmt_flat loader, 
> > which was motivated by an LTP testcase which checks that execve returns 
> > EMFILE when the file descriptor table is full.
> >
> > The patch is buggy: the code now keeps file descriptors open for the 
> > executable and its libraries, which has confused at least one 
> > application.  It's also unnecessary, since there is no code that uses 
> > the file descriptor, so the new EMFILE error return is totally artificial.

> I don't get it.  The substance of the patch is
>
> +	/* check file descriptor */
> +	exec_fileno = get_unused_fd();
> +	if (exec_fileno < 0) {
> +		ret = -EMFILE;
> +		goto err;
> +	}
> +	get_file(bprm->file);
> +	fd_install(exec_fileno, bprm->file);
>
> and that get_file() will be undone by exit().  Without this change we'll
> forget to do file limit checking.

It's not the get_file that's the problem, it's the get_unused_fd and 
fd_install.  These files are now open while the process lives and 
consume file descriptors.  This does not happen with the ELF loader, 
which does

        if (interpreter_type != INTERPRETER_AOUT)
                sys_close(elf_exec_fileno);

before transferring control to the application.  So, fewer file 
descriptors are available for the app, and they start at a higher number.

Before the change, we didn't allocate or install a file descriptor, 
hence there wasn't any reason to return EMFILE.  The spec at
  http://www.opengroup.org/onlinepubs/009695399/functions/exec.html
doesn't list EMFILE as a possible error.

If you're unconvinced, then at the very least we need to add a sys_close 
call in the success path.


Bernd
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/