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

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);

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);

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;
 }
 

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));

none/tests/.cvsignore

@@ -42,6 +42,7 @@ rcl_assert
 rcrl
 readline1
 resolv
+rlimit_nofile
 seg_override
 sem
 semlimit

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

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);
+}

none/tests/rlimit_nofile.stderr.exp

@@ -0,0 +1,2 @@
+
+

none/tests/rlimit_nofile.vgtest

@@ -0,0 +1 @@
+prog: rlimit_nofile