Re: Please revert git commit 1ad3dcc0

Andrew Morton wrote:
Bernd Schmidt <bernds_cb1@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: Please revert git commit 1ad3dcc0
    ... EMFILE when the file descriptor table is full. ... ret = result; ... if (addr == RELOC_FAILED) { ...
    (Linux-Kernel)
  • Please revert git commit 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. ...
    (Linux-Kernel)