Implement an emulated soft limit for file descriptors in addition to

the current reserved area, which effectively acts as a hard limit. The
setrlimit system call now simply updates the emulated limits as best
as possible - the hard limit is not allowed to move at all and just
returns EPERM if you try and change it.

This should stop reductions in the soft limit causing assertions when
valgrind tries to allocate descriptors from the reserved area.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2440

diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h
index 9043e522ad61afd711566286b12fae246f8d675c..544416ae58b475ab23f0db416df203dfb8e93af1 100644 (file)
--- a/coregrind/vg_include.h
+++ b/coregrind/vg_include.h
@@ -177,8 +177,9 @@ extern Int VG_(main_pid);
 /* pgrp of process (global to all threads) */
 extern Int VG_(main_pgrp);
 
-/* Maximum allowed application-visible file descriptor */
-extern Int VG_(max_fd);
+/* Application-visible file descriptor limits */
+extern Int VG_(fd_soft_limit);
+extern Int VG_(fd_hard_limit);
 
 /* Should we stop collecting errors if too many appear?  default: YES */
 extern Bool  VG_(clo_error_limit);

diff --git a/coregrind/vg_main.c b/coregrind/vg_main.c
index 4d53d2421a7cba4a4e32e58b5328eb23c1ca4daa..53777e4eb51c2b004277d120c4c815431c29db7d 100644 (file)
--- a/coregrind/vg_main.c
+++ b/coregrind/vg_main.c
@@ -143,8 +143,9 @@ Int VG_(main_pid);
 /* PGRP of process */
 Int VG_(main_pgrp);
 
-/* Maximum allowed application-visible file descriptor */
-Int VG_(max_fd) = -1;
+/* Application-visible file descriptor limits */
+Int VG_(fd_soft_limit) = -1;
+Int VG_(fd_hard_limit) = -1;
 
 /* As deduced from esp_at_startup, the client's argc, argv[] and
    envp[] as extracted from the client's stack at startup-time. */
@@ -1905,7 +1906,7 @@ static void process_cmd_line_options( UInt* client_auxv, const char* toolname )
    }
 
    /* Move log_fd into the safe range, so it doesn't conflict with any app fds */
-   eventually_log_fd = VG_(fcntl)(VG_(clo_log_fd), VKI_F_DUPFD, VG_(max_fd)+1);
+   eventually_log_fd = VG_(fcntl)(VG_(clo_log_fd), VKI_F_DUPFD, VG_(fd_hard_limit));
    if (eventually_log_fd < 0)
       VG_(message)(Vg_UserMsg, "valgrind: failed to move logfile fd into safe range");
    else {
@@ -2024,7 +2025,8 @@ static void setup_file_descriptors(void)
    }
 
    /* Reserve some file descriptors for our use. */
-   VG_(max_fd) = rl.rlim_cur - VG_N_RESERVED_FDS;
+   VG_(fd_soft_limit) = rl.rlim_cur - VG_N_RESERVED_FDS;
+   VG_(fd_hard_limit) = rl.rlim_cur - VG_N_RESERVED_FDS;
 
    /* Update the soft limit. */
    VG_(setrlimit)(VKI_RLIMIT_NOFILE, &rl);
@@ -2799,7 +2801,7 @@ int main(int argc, char **argv)
    //--------------------------------------------------------------
    // Process Valgrind's + tool's command-line options
    //   p: load_tool()               [for 'tool']
-   //   p: setup_file_descriptors()  [for 'VG_(max_fd)']
+   //   p: setup_file_descriptors()  [for 'VG_(fd_xxx_limit)']
    //   p: sk_pre_clo_init           [to set 'command_line_options' need]
    //--------------------------------------------------------------
    process_cmd_line_options(client_auxv, tool);

diff --git a/coregrind/vg_mylibc.c b/coregrind/vg_mylibc.c
index dcd2ba45c0d7923079d34b5200c1536cc0127aad..7b11f6eec39d2b4d3e387b9d415509c04fa3a48f 100644 (file)
--- a/coregrind/vg_mylibc.c
+++ b/coregrind/vg_mylibc.c
@@ -1263,15 +1263,15 @@ Int VG_(safe_fd)(Int oldfd)
 {
    Int newfd;
 
-   vg_assert(VG_(max_fd) != -1);
+   vg_assert(VG_(fd_hard_limit) != -1);
 
-   newfd = VG_(fcntl)(oldfd, VKI_F_DUPFD, VG_(max_fd)+1);
+   newfd = VG_(fcntl)(oldfd, VKI_F_DUPFD, VG_(fd_hard_limit));
    if (newfd != -1)
       VG_(close)(oldfd);
 
    VG_(fcntl)(newfd, VKI_F_SETFD, VKI_FD_CLOEXEC);
 
-   vg_assert(newfd > VG_(max_fd));
+   vg_assert(newfd >= VG_(fd_hard_limit));
    return newfd;
 }
 

diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c
index 6ffde3e40034b040d4491a2fe86556d33d371770..39cadd41d1f4cbc2194682c38a038c40728bb6d9 100644 (file)
--- a/coregrind/vg_syscalls.c
+++ b/coregrind/vg_syscalls.c
@@ -513,7 +513,7 @@ void record_fd_close(Int tid, Int fd)
 {
    OpenFd *i = allocated_fds;
 
-   if (fd > VG_(max_fd))
+   if (fd >= VG_(fd_hard_limit))
       return;                  /* Valgrind internal */
 
    while(i) {
@@ -546,7 +546,7 @@ void record_fd_open(Int tid, Int fd, char *pathname)
 {
    OpenFd *i;
 
-   if (fd > VG_(max_fd))
+   if (fd >= VG_(fd_hard_limit))
       return;                  /* Valgrind internal */
 
    /* Check to see if this fd is already open. */
@@ -1055,9 +1055,9 @@ static Addr do_brk(Addr newbrk)
    ------------------------------------------------------------------ */
 
 /* Return true if we're allowed to use or create this fd */
-static Bool fd_allowed(Int fd, const Char *syscall, ThreadId tid)
+static Bool fd_allowed(Int fd, const Char *syscall, ThreadId tid, Bool soft)
 {
-   if (fd < 0 || fd > VG_(max_fd) || fd == VG_(clo_log_fd)) {
+   if (fd < 0 || fd >= VG_(fd_hard_limit) || fd == VG_(clo_log_fd)) {
       VG_(message)(Vg_UserMsg, 
          "Warning: invalid file descriptor %d in syscall %s()",
          fd, syscall);
@@ -1070,6 +1070,9 @@ static Bool fd_allowed(Int fd, const Char *syscall, ThreadId tid)
       }
       return False;
    }
+   else if (soft && fd >= VG_(fd_soft_limit)) {
+      return False;
+   }
    return True;
 }
 
@@ -2099,7 +2102,7 @@ PRE(close)
    /* int close(int fd); */
    MAYBE_PRINTF("close ( %d )\n",arg1);
    /* Detect and negate attempts by the client to close Valgrind's log fd */
-   if (!fd_allowed(arg1, "close", tid))
+   if (!fd_allowed(arg1, "close", tid, False))
       res = -VKI_EBADF;
 }
 
@@ -2117,7 +2120,7 @@ PRE(dup)
 POST(dup)
 {
    MAYBE_PRINTF("%d\n", res);
-   if (!fd_allowed(res, "dup", tid)) {
+   if (!fd_allowed(res, "dup", tid, True)) {
       VG_(close)(res);
       res = -VKI_EMFILE;
    } else {
@@ -2130,7 +2133,7 @@ PRE(dup2)
 {
    /* int dup2(int oldfd, int newfd); */
    MAYBE_PRINTF("dup2 ( %d, %d ) ...\n", arg1,arg2);
-   if (!fd_allowed(arg2, "dup2", tid))
+   if (!fd_allowed(arg2, "dup2", tid, True))
       res = -VKI_EBADF;
 }
 
@@ -2152,7 +2155,7 @@ PRE(fcntl)
 POST(fcntl)
 {
    if (arg2 == VKI_F_DUPFD) {
-      if (!fd_allowed(res, "fcntl(DUPFD)", tid)) {
+      if (!fd_allowed(res, "fcntl(DUPFD)", tid, True)) {
          VG_(close)(res);
          res = -VKI_EMFILE;
       } else {
@@ -2191,7 +2194,7 @@ PRE(fcntl64)
 POST(fcntl64)
 {
    if (arg2 == VKI_F_DUPFD) {
-      if (!fd_allowed(res, "fcntl64(DUPFD)", tid)) {
+      if (!fd_allowed(res, "fcntl64(DUPFD)", tid, True)) {
          VG_(close)(res);
          res = -VKI_EMFILE;
       } else {
@@ -2512,7 +2515,8 @@ POST(getrlimit)
 
     switch(arg1) {
     case VKI_RLIMIT_NOFILE:
-       ((vki_rlimit *)arg2)->rlim_cur = VG_(max_fd);
+       ((vki_rlimit *)arg2)->rlim_cur = VG_(fd_soft_limit);
+       ((vki_rlimit *)arg2)->rlim_max = VG_(fd_hard_limit);
        break;
 
     case VKI_RLIMIT_DATA:
@@ -4145,7 +4149,7 @@ PRE(open)
 
 POST(open)
 {
-   if (!fd_allowed(res, "open", tid)) {
+   if (!fd_allowed(res, "open", tid, True)) {
       VG_(close)(res);
       res = -VKI_EMFILE;
    } else {
@@ -4160,7 +4164,7 @@ PRE(read)
    /* size_t read(int fd, void *buf, size_t count); */
    MAYBE_PRINTF("read ( %d, %p, %d )\n", arg1, arg2, arg3);
 
-   if (!fd_allowed(arg1, "read", tid))
+   if (!fd_allowed(arg1, "read", tid, False))
       res = -VKI_EBADF;   
 }
 
@@ -4174,7 +4178,7 @@ PRE(write)
 {
    /* size_t write(int fd, const void *buf, size_t count); */
    MAYBE_PRINTF("write ( %d, %p, %d )\n", arg1, arg2, arg3);
-   if (!fd_allowed(arg1, "write", tid))
+   if (!fd_allowed(arg1, "write", tid, False))
       res = -VKI_EBADF;
    else
       SYSCALL_TRACK( pre_mem_read, tid, "write(buf)", arg2, arg3 );
@@ -4189,7 +4193,7 @@ PRE(creat)
 
 POST(creat)
 {
-   if (!fd_allowed(res, "creat", tid)) {
+   if (!fd_allowed(res, "creat", tid, True)) {
       VG_(close)(res);
       res = -VKI_EMFILE;
    } else {
@@ -4211,8 +4215,8 @@ POST(pipe)
 {
    Int *p = (Int *)arg1;
 
-   if (!fd_allowed(p[0], "pipe", tid) ||
-       !fd_allowed(p[1], "pipe", tid)) {
+   if (!fd_allowed(p[0], "pipe", tid, True) ||
+       !fd_allowed(p[1], "pipe", tid, True)) {
       VG_(close)(p[0]);
       VG_(close)(p[1]);
       res = -VKI_EMFILE;
@@ -4265,7 +4269,7 @@ PRE(epoll_create)
 
 POST(epoll_create)
 {
-   if (!fd_allowed(res, "open", tid)) {
+   if (!fd_allowed(res, "open", tid, True)) {
       VG_(close)(res);
       res = -VKI_EMFILE;
    } else {
@@ -4322,7 +4326,7 @@ PRE(readv)
    Int i;
    struct iovec * vec;
    MAYBE_PRINTF("readv ( %d, %p, %d )\n",arg1,arg2,arg3);
-   if (!fd_allowed(arg1, "readv", tid)) {
+   if (!fd_allowed(arg1, "readv", tid, False)) {
       res = -VKI_EBADF;
    } else {
       SYSCALL_TRACK( pre_mem_read, tid, "readv(vector)", 
@@ -4527,7 +4531,17 @@ PRE(setrlimit)
    SYSCALL_TRACK( pre_mem_read, tid, "setrlimit(rlim)", 
                  arg2, sizeof(struct vki_rlimit) );
 
-   if (arg1 == VKI_RLIMIT_DATA) {
+   if (arg1 == VKI_RLIMIT_NOFILE) {
+      if (((vki_rlimit *)arg2)->rlim_cur > VG_(fd_hard_limit) ||
+          ((vki_rlimit *)arg2)->rlim_max != VG_(fd_hard_limit)) {
+         res = -VKI_EPERM;
+      }
+      else {
+         VG_(fd_soft_limit) = ((vki_rlimit *)arg2)->rlim_cur;
+         res = 0;
+      }
+   }
+   else if (arg1 == VKI_RLIMIT_DATA) {
       VG_(client_rlimit_data) = *(vki_rlimit *)arg2;
       res = 0;
    }
@@ -4771,8 +4785,8 @@ POST(socketcall)
       Int fd1 = ((UInt*)((UInt*)arg2)[3])[0];
       Int fd2 = ((UInt*)((UInt*)arg2)[3])[1];
       VG_TRACK( post_mem_write, ((UInt*)arg2)[3], 2*sizeof(int) );
-      if (!fd_allowed(fd1, "socketcall.socketpair", tid) ||
-          !fd_allowed(fd2, "socketcall.socketpair", tid)) {
+      if (!fd_allowed(fd1, "socketcall.socketpair", tid, True) ||
+          !fd_allowed(fd2, "socketcall.socketpair", tid, True)) {
          VG_(close)(fd1);
          VG_(close)(fd2);
          res = -VKI_EMFILE;
@@ -4787,7 +4801,7 @@ POST(socketcall)
    }
 
    case SYS_SOCKET:
-      if (!fd_allowed(res, "socket", tid)) {
+      if (!fd_allowed(res, "socket", tid, True)) {
         VG_(close)(res);
         res = -VKI_EMFILE;
       } else {
@@ -4807,7 +4821,7 @@ POST(socketcall)
 
    case SYS_ACCEPT: {
       /* int accept(int s, struct sockaddr *addr, int *addrlen); */
-      if (!fd_allowed(res, "accept", tid)) {
+      if (!fd_allowed(res, "accept", tid, True)) {
         VG_(close)(res);
         res = -VKI_EMFILE;
       } else {
@@ -5124,7 +5138,7 @@ PRE(writev)
    Int i;
    struct iovec * vec;
    MAYBE_PRINTF("writev ( %d, %p, %d )\n",arg1,arg2,arg3);
-   if (!fd_allowed(arg1, "writev", tid)) {
+   if (!fd_allowed(arg1, "writev", tid, False)) {
       res = -VKI_EBADF;
    } else {
       SYSCALL_TRACK( pre_mem_read, tid, "writev(vector)", 
@@ -5214,9 +5228,9 @@ POST(futex)
    if (!VG_(is_kerror)(res)) {
       VG_TRACK( post_mem_write, arg1, sizeof(int) );
       if (arg2 == VKI_FUTEX_FD) {
-         if (!fd_allowed(res, "futex", tid)) {
+         if (!fd_allowed(res, "futex", tid, True)) {
             VG_(close)(res);
-            res = -VKI_ENFILE;
+            res = -VKI_EMFILE;
          } else {
             if (VG_(clo_track_fds))
                record_fd_open(tid, res, VG_(arena_strdup)(VG_AR_CORE, (Char*)arg1));

diff --git a/none/tests/.cvsignore b/none/tests/.cvsignore
index f31a969889b292796fc700b4abe52656b17a3ec0..af94caa8c842c550f8977b0243488437ac87d525 100644 (file)
--- a/none/tests/.cvsignore
+++ b/none/tests/.cvsignore
@@ -42,6 +42,7 @@ rcl_assert
 rcrl
 readline1
 resolv
+rlimit_nofile
 seg_override
 sem
 semlimit

diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am
index 97ecf66984ccdde3ee965c6227a3d8957077b1b4..6de0be3e241e0ddf00999b70bd76214a73f6137d 100644 (file)
--- a/none/tests/Makefile.am
+++ b/none/tests/Makefile.am
@@ -50,6 +50,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \
        readline1.stderr.exp readline1.stdout.exp \
        readline1.vgtest \
        resolv.stderr.exp resolv.stdout.exp resolv.vgtest \
+       rlimit_nofile.stderr.exp rlimit_nofile.stdout.exp rlimit_nofile.vgtest \
        seg_override.stderr.exp \
        seg_override.stdout.exp seg_override.vgtest \
        sem.stderr.exp sem.stdout.exp sem.vgtest \
@@ -69,8 +70,8 @@ check_PROGRAMS = \
        args badseg bitfield1 bt_everything bt_literal closeall coolo_strlen \
        cpuid dastest discard exec-sigmask execve floored fork \
        fpu_lazy_eflags fucomip $(INSN_TESTS) \
-       int munmap_exe map_unmap mremap rcl_assert \
-       rcrl readline1 resolv seg_override sem semlimit sha1_test \
+       int munmap_exe map_unmap mremap rcl_assert rcrl readline1 \
+       resolv rlimit_nofile seg_override sem semlimit sha1_test \
        shortpush shorts smc1 susphello pth_blockedsig pushpopseg \
        syscall-restart1 syscall-restart2 system \
        coolo_sigaction gxx304 yield
@@ -119,6 +120,7 @@ rcl_assert_SOURCES  = rcl_assert.S
 rcrl_SOURCES           = rcrl.c
 readline1_SOURCES      = readline1.c
 resolv_SOURCES                 = resolv.c
+rlimit_nofile_SOURCES  = rlimit_nofile.c
 seg_override_SOURCES   = seg_override.c
 sem_SOURCES            = sem.c
 semlimit_SOURCES       = semlimit.c

diff --git a/none/tests/rlimit_nofile.c b/none/tests/rlimit_nofile.c
new file mode 100644 (file)
index 0000000..f581dc0
--- /dev/null
+++ b/none/tests/rlimit_nofile.c
@@ -0,0 +1,67 @@
+#include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/resource.h>
+#include <unistd.h>
+
+int main(int argc, char **argv)
+{
+   struct rlimit oldrlim;
+   struct rlimit newrlim;
+   int fd;
+  
+   if (getrlimit(RLIMIT_NOFILE, &oldrlim) < 0)
+   {
+      perror("getrlimit");
+      exit(1);
+   }
+
+   newrlim.rlim_cur = oldrlim.rlim_cur / 2;
+   newrlim.rlim_max = oldrlim.rlim_max;
+     
+   if (setrlimit(RLIMIT_NOFILE, &newrlim) < 0)
+   {
+      perror("setrlimit");
+      exit(1);
+   }
+     
+   if (getrlimit(RLIMIT_NOFILE, &newrlim) < 0)
+   {
+      perror("getrlimit");
+      exit(1);
+   }
+
+   if (newrlim.rlim_cur != oldrlim.rlim_cur / 2)
+   {
+      fprintf(stderr, "rlim_cur is %lu (should be %lu)\n",
+              newrlim.rlim_cur, oldrlim.rlim_cur / 2);
+   }
+
+   if (newrlim.rlim_max != oldrlim.rlim_max)
+   {
+      fprintf(stderr, "rlim_max is %lu (should be %lu)\n",
+              newrlim.rlim_max, oldrlim.rlim_max);
+   }
+
+   newrlim.rlim_cur -= 3; /* allow for stdin, stdout and stderr */
+
+   while (newrlim.rlim_cur-- > 0)
+   {
+      if (open("/dev/null", O_RDONLY) < 0)
+      {
+         perror("open");
+      }
+   }
+
+   if ((fd = open("/dev/null", O_RDONLY)) >= 0)
+   {
+      fprintf(stderr, "open succeeded with fd %d - it should have failed!\n", fd);
+   }
+   else if (errno != EMFILE)
+   {
+      perror("open");
+   }
+   
+   exit(0);
+}

diff --git a/none/tests/rlimit_nofile.stderr.exp b/none/tests/rlimit_nofile.stderr.exp
new file mode 100644 (file)
index 0000000..139597f
--- /dev/null
+++ b/none/tests/rlimit_nofile.stderr.exp
@@ -0,0 +1,2 @@
+
+

diff --git a/none/tests/rlimit_nofile.stdout.exp b/none/tests/rlimit_nofile.stdout.exp
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/none/tests/rlimit_nofile.vgtest b/none/tests/rlimit_nofile.vgtest
new file mode 100644 (file)
index 0000000..8436808
--- /dev/null
+++ b/none/tests/rlimit_nofile.vgtest
@@ -0,0 +1 @@
+prog: rlimit_nofile