Re: Another test-dup3 assertion with the new kernel

Re: Another test-dup3 assertion with the new kernel

[Richard W.M. Jones]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

On Wed, Oct 10, 2012 at 11:05:54AM -0600, Eric Blake wrote:
> On 10/10/2012 07:23 AM, Richard W.M. Jones wrote:
> > 
> > Fixing the other two kernel bugs, leaves me with two further
> > assertions.  This time I'm not so sure that this is a kernel bug.
> > 
> >       /* The destination must be valid.  */
> >       errno = 0;
> >       ASSERT (dup3 (fd, -2, o_flags) == -1);
> >       ASSERT (errno == EBADF);                  <--- here
> 
> This one sounds wrong in the kernel.
> 
> >       errno = 0;
> >       ASSERT (dup3 (fd, 10000000, o_flags) == -1);
> >       ASSERT (errno == EBADF);                  <--- and here
> 
> And for this one, EMFILE makes sense only IF the kernel can support
> OPEN_MAX of 10000000 (but it doesn't).
> 
> > 
> > In fact the new implementation of dup3 returns EMFILE (Too many open
> > files) because of the following test:
> > 
> >       if (newfd >= rlimit(RLIMIT_NOFILE))
> >               return -EMFILE;
> > 
> > combined with the fact that newfd is declared inside the kernel as an
> > unsigned int (both fd params have been declared as unsigned int at
> > least as far back as 2008 when the dup3 call was first added).
> 
> fds have always been signed in userland.  Treating an fd as unsigned
> gives the wrong error in this case, because userland cannot request an
> fd that large.  The POSIX wording for dup3 says that it should be
> identical to dup2 in most cases, and the wording for dup2 is explicit:
> 
> [EBADF] The fildes argument is not a valid open file descriptor or the
> argument fildes2 is negative or greater than or equal to {OPEN_MAX}.
> 
> No room for EMFILE when fildes2 treated as unsigned is huge, or even
> when fildes2 is 10000000 but the kernel does not support an OPEN_MAX
> that large.
> 
> > 
> > The change from EBADF to EMFILE seems to be intentional (kernel commit
> > 4e1e018ecc6f7bfd10fc75b3ff9715cc8164e0a2).
> 
> Maybe intentional, but sounds wrong to me.
> 
> > 
> > I have no opinion on whether the kernel or gnulib is wrong here.
> 
> I'm arguing that it a kernel bug; how does dup2 behave on the same test?

The same:

$ ./dup3-badf 
failed: dup3 (fd, -2, 0) => -1, errno = 24 (Too many open files)
failed: dup3 (fd, 1000000, 0) => -1, errno = 24 (Too many open files)
failed: dup2 (fd, -2) => -1, errno = 24 (Too many open files)
failed: dup2 (fd, 1000000) => -1, errno = 24 (Too many open files)

Test program is attached in case you want to try it.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

[-- Attachment #2: dup3-badf.c --]
[-- Type: text/plain, Size: 1185 bytes --]

/* Test if dup3 returns EBADF for bad fds.
 * by Richard W.M. Jones.
 */

#define _GNU_SOURCE

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <assert.h>

int
main ()
{
  char *file = "/dev/null";
  int fd = open (file, O_RDONLY);
  int r, errors = 0;

  assert (fd >= 0);

  r = dup3 (fd, -2, 0);
  if (r != -1 || errno != EBADF) {
    fprintf (stderr, "failed: dup3 (fd, -2, 0) => %d, errno = %d (%s)\n",
	     r, errno, strerror (errno));
    errors++;
  }

  r = dup3 (fd, 1000000, 0);
  if (r != -1 || errno != EBADF) {
    fprintf (stderr, "failed: dup3 (fd, 1000000, 0) => %d, errno = %d (%s)\n",
	     r, errno, strerror (errno));
    errors++;
  }

  /* Also test dup2 */
  r = dup2 (fd, -2);
  if (r != -1 || errno != EBADF) {
    fprintf (stderr, "failed: dup2 (fd, -2) => %d, errno = %d (%s)\n",
	     r, errno, strerror (errno));
    errors++;
  }

  r = dup2 (fd, 1000000);
  if (r != -1 || errno != EBADF) {
    fprintf (stderr, "failed: dup2 (fd, 1000000) => %d, errno = %d (%s)\n",
	     r, errno, strerror (errno));
    errors++;
  }

  exit (errors == 0 ? EXIT_SUCCESS : EXIT_FAILURE);
}