The closeall function is broken in such a way that it almost never nfs-utils-1-2-1-rc1
authorSteve Dickson <steved@redhat.com>
Mon, 22 Jun 2009 14:05:44 +0000 (10:05 -0400)
committerSteve Dickson <steved@redhat.com>
Mon, 22 Jun 2009 14:05:44 +0000 (10:05 -0400)
closes any file descriptors. It's calling strtol on the text
representation of the file descriptor, and then checking to see if the
value of *endptr is not '\0' before trying to close the file. This check
is wrong.

When strtol returns an endptr that points to a NULL byte, that indicates
that the conversion was completely successful. I believe this check
should instead be requiring that endptr is pointing to '\0' before
closing the fd.

Also, fix up the function to check for conversion errors from strtol. If
one occurs, just skip the close on that entry.

Finally, as Trond pointed out, it's unlikely that readdir will return a
blank string in d_name but that situation wouldn't be detected by the
current code. This patch adds such a check and skips the close if it
occurs.

Reported-by: Chuck Lever <chuck.lever@oracle.com>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
Signed-off-by: Steve Dickson <steved@redhat.com>
support/nfs/closeall.c

index cc7fb3b..38fb162 100644 (file)
@@ -7,19 +7,24 @@
 #include <unistd.h>
 #include <stdlib.h>
 #include <dirent.h>
+#include <errno.h>
 
 void
 closeall(int min)
 {
+       char *endp;
+       long n;
        DIR *dir = opendir("/proc/self/fd");
+
        if (dir != NULL) {
                int dfd = dirfd(dir);
                struct dirent *d;
 
                while ((d = readdir(dir)) != NULL) {
-                       char *endp;
-                       long n = strtol(d->d_name, &endp, 10);
-                       if (*endp != '\0' && n >= min && n != dfd)
+                       errno = 0;
+                       n = strtol(d->d_name, &endp, 10);
+                       if (!errno && *endp == '\0' && endp != d->d_name &&
+                           n >= min && n != dfd)
                                (void) close(n);
                }
                closedir(dir);