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