lkml.org 
[lkml]   [2006]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Please revert git commit 1ad3dcc0
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/

\
 
 \ /
  Last update: 2006-05-16 16:32    [W:0.070 / U:0.312 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site