public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Layton <jlayton@redhat.com>
To: steved@redhat.com
Cc: linux-nfs@vger.kernel.org, chuck.lever@oracle.com,
	trond.myklebust@fys.uio.no
Subject: [PATCH] nfs-utils: fix endptr check in closeall (try #4)
Date: Thu,  4 Jun 2009 07:53:07 -0400	[thread overview]
Message-ID: <1244116387-19827-1-git-send-email-jlayton@redhat.com> (raw)

The closeall function is broken in such a way that it almost never
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>
---
 support/nfs/closeall.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/support/nfs/closeall.c b/support/nfs/closeall.c
index cc7fb3b..38fb162 100644
--- a/support/nfs/closeall.c
+++ b/support/nfs/closeall.c
@@ -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);
-- 
1.6.0.6


             reply	other threads:[~2009-06-04 11:53 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-04 11:53 Jeff Layton [this message]
2009-06-22 16:02 ` [PATCH] nfs-utils: fix endptr check in closeall (try #4) Steve Dickson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1244116387-19827-1-git-send-email-jlayton@redhat.com \
    --to=jlayton@redhat.com \
    --cc=chuck.lever@oracle.com \
    --cc=linux-nfs@vger.kernel.org \
    --cc=steved@redhat.com \
    --cc=trond.myklebust@fys.uio.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox