* [PATCH 1/6] nfsdcltrack: rename the nfsdcld directory and options to nfsdcltrack
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
@ 2012-10-01 11:52 ` Jeff Layton
2012-10-01 11:52 ` [PATCH 2/6] nfsdcltrack: remove pointless sqlite_topdir variable Jeff Layton
` (5 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-10-01 11:52 UTC (permalink / raw)
To: linux-nfs
We'll soon be adding a new nfsdcltrack program, at which point it won't
make much sense to call this directory and the config option "nfsdcld".
Rename it to be a bit more generic.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
configure.ac | 20 ++++++++++----------
utils/Makefile.am | 4 ++--
utils/{nfsdcld => nfsdcltrack}/Makefile.am | 0
utils/{nfsdcld => nfsdcltrack}/nfsdcld.c | 0
utils/{nfsdcld => nfsdcltrack}/nfsdcld.man | 0
utils/{nfsdcld => nfsdcltrack}/sqlite.c | 0
utils/{nfsdcld => nfsdcltrack}/sqlite.h | 0
7 files changed, 12 insertions(+), 12 deletions(-)
rename utils/{nfsdcld => nfsdcltrack}/Makefile.am (100%)
rename utils/{nfsdcld => nfsdcltrack}/nfsdcld.c (100%)
rename utils/{nfsdcld => nfsdcltrack}/nfsdcld.man (100%)
rename utils/{nfsdcld => nfsdcltrack}/sqlite.c (100%)
rename utils/{nfsdcld => nfsdcltrack}/sqlite.h (100%)
diff --git a/configure.ac b/configure.ac
index a174bf4..65d1bea 100644
--- a/configure.ac
+++ b/configure.ac
@@ -185,11 +185,11 @@ else
AM_CONDITIONAL(MOUNT_CONFIG, [test "$enable_mount" = "yes"])
fi
-AC_ARG_ENABLE(nfsdcld,
- [AC_HELP_STRING([--enable-nfsdcld],
- [Create nfsdcld NFSv4 clientid tracking daemon. @<:@default=no@:>@])],
- enable_nfsdcld=$enableval,
- enable_nfsdcld="no")
+AC_ARG_ENABLE(nfsdcltrack,
+ [AC_HELP_STRING([--enable-nfsdcltrack],
+ [enable NFSv4 clientid tracking programs @<:@default=no@:>@])],
+ enable_nfsdctrack=$enableval,
+ enable_nfsdcltrack="no")
dnl Check for TI-RPC library and headers
AC_LIBTIRPC
@@ -269,12 +269,12 @@ if test "$enable_nfsv4" = yes; then
dnl Check for sqlite3
AC_SQLITE3_VERS
- if test "$enable_nfsdcld" = "yes"; then
+ if test "$enable_nfsdcltrack" = "yes"; then
AC_CHECK_HEADERS([libgen.h sys/inotify.h], ,
- AC_MSG_ERROR([Cannot find header needed for nfsdcld]))
+ AC_MSG_ERROR([Cannot find header needed for nfsdcltrack]))
if test "$libsqlite3_cv_is_recent" != "yes" ; then
- AC_MSG_ERROR([nfsdcld requires sqlite3])
+ AC_MSG_ERROR([nfsdcltrack requires sqlite3])
fi
fi
@@ -292,7 +292,7 @@ if test "$enable_nfsv41" = yes; then
fi
dnl enable nfsidmap when its support by libnfsidmap
-AM_CONDITIONAL(CONFIG_NFSDCLD, [test "$enable_nfsdcld" = "yes" ])
+AM_CONDITIONAL(CONFIG_NFSDCLTRACK, [test "$enable_nfsdcltrack" = "yes" ])
AM_CONDITIONAL(CONFIG_NFSIDMAP, [test "$ac_cv_header_keyutils_h$ac_cv_lib_nfsidmap_nfs4_owner_to_uid" = "yesyes"])
@@ -477,7 +477,7 @@ AC_CONFIG_FILES([
tools/nfs-iostat/Makefile
utils/Makefile
utils/blkmapd/Makefile
- utils/nfsdcld/Makefile
+ utils/nfsdcltrack/Makefile
utils/exportfs/Makefile
utils/gssd/Makefile
utils/idmapd/Makefile
diff --git a/utils/Makefile.am b/utils/Makefile.am
index 09045dd..b892dc8 100644
--- a/utils/Makefile.am
+++ b/utils/Makefile.am
@@ -21,8 +21,8 @@ if CONFIG_MOUNT
OPTDIRS += mount
endif
-if CONFIG_NFSDCLD
-OPTDIRS += nfsdcld
+if CONFIG_NFSDCLTRACK
+OPTDIRS += nfsdcltrack
endif
SUBDIRS = \
diff --git a/utils/nfsdcld/Makefile.am b/utils/nfsdcltrack/Makefile.am
similarity index 100%
rename from utils/nfsdcld/Makefile.am
rename to utils/nfsdcltrack/Makefile.am
diff --git a/utils/nfsdcld/nfsdcld.c b/utils/nfsdcltrack/nfsdcld.c
similarity index 100%
rename from utils/nfsdcld/nfsdcld.c
rename to utils/nfsdcltrack/nfsdcld.c
diff --git a/utils/nfsdcld/nfsdcld.man b/utils/nfsdcltrack/nfsdcld.man
similarity index 100%
rename from utils/nfsdcld/nfsdcld.man
rename to utils/nfsdcltrack/nfsdcld.man
diff --git a/utils/nfsdcld/sqlite.c b/utils/nfsdcltrack/sqlite.c
similarity index 100%
rename from utils/nfsdcld/sqlite.c
rename to utils/nfsdcltrack/sqlite.c
diff --git a/utils/nfsdcld/sqlite.h b/utils/nfsdcltrack/sqlite.h
similarity index 100%
rename from utils/nfsdcld/sqlite.h
rename to utils/nfsdcltrack/sqlite.h
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/6] nfsdcltrack: remove pointless sqlite_topdir variable
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
2012-10-01 11:52 ` [PATCH 1/6] nfsdcltrack: rename the nfsdcld directory and options to nfsdcltrack Jeff Layton
@ 2012-10-01 11:52 ` Jeff Layton
2012-10-01 11:52 ` [PATCH 3/6] nfsdcltrack: break out a function to open the database handle Jeff Layton
` (4 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-10-01 11:52 UTC (permalink / raw)
To: linux-nfs
This is holdover from an earlier version of the code and doesn't really
provide any benefit. Also, mark the topdir and dirname arguments const
since they should never be changed.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsdcltrack/sqlite.c | 13 ++++---------
utils/nfsdcltrack/sqlite.h | 2 +-
2 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/utils/nfsdcltrack/sqlite.c b/utils/nfsdcltrack/sqlite.c
index fc882c6..c19af7e 100644
--- a/utils/nfsdcltrack/sqlite.c
+++ b/utils/nfsdcltrack/sqlite.c
@@ -61,9 +61,6 @@
/* global variables */
-/* top level DB directory */
-static char *sqlite_topdir;
-
/* reusable pathname and sql command buffer */
static char buf[PATH_MAX];
@@ -74,7 +71,7 @@ static sqlite3 *dbh;
/* make a directory, ignoring EEXIST errors unless it's not a directory */
static int
-mkdir_if_not_exist(char *dirname)
+mkdir_if_not_exist(const char *dirname)
{
int ret;
struct stat statbuf;
@@ -102,19 +99,17 @@ mkdir_if_not_exist(char *dirname)
* the "clients" table.
*/
int
-sqlite_maindb_init(char *topdir)
+sqlite_maindb_init(const char *topdir)
{
int ret;
char *err = NULL;
sqlite3_stmt *stmt = NULL;
- sqlite_topdir = topdir;
-
- ret = mkdir_if_not_exist(sqlite_topdir);
+ ret = mkdir_if_not_exist(topdir);
if (ret)
return ret;
- ret = snprintf(buf, PATH_MAX - 1, "%s/main.sqlite", sqlite_topdir);
+ ret = snprintf(buf, PATH_MAX - 1, "%s/main.sqlite", topdir);
if (ret < 0)
return ret;
diff --git a/utils/nfsdcltrack/sqlite.h b/utils/nfsdcltrack/sqlite.h
index c85e7d6..8748948 100644
--- a/utils/nfsdcltrack/sqlite.h
+++ b/utils/nfsdcltrack/sqlite.h
@@ -20,7 +20,7 @@
#ifndef _SQLITE_H_
#define _SQLITE_H_
-int sqlite_maindb_init(char *topdir);
+int sqlite_maindb_init(const char *topdir);
int sqlite_insert_client(const unsigned char *clname, const size_t namelen);
int sqlite_remove_client(const unsigned char *clname, const size_t namelen);
int sqlite_check_client(const unsigned char *clname, const size_t namelen);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/6] nfsdcltrack: break out a function to open the database handle
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
2012-10-01 11:52 ` [PATCH 1/6] nfsdcltrack: rename the nfsdcld directory and options to nfsdcltrack Jeff Layton
2012-10-01 11:52 ` [PATCH 2/6] nfsdcltrack: remove pointless sqlite_topdir variable Jeff Layton
@ 2012-10-01 11:52 ` Jeff Layton
2012-10-01 11:52 ` [PATCH 4/6] nfsdcltrack: add a new "one-shot" program for manipulating the client tracking db Jeff Layton
` (3 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-10-01 11:52 UTC (permalink / raw)
To: linux-nfs
When we add a new usermodehelper upcall program to do the database
access, the existing "init" function will be overkill every time
we start up the program.
Break out the database handle establishment routine into a separate
function that we can call from each upcall command in the one-shot
program.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsdcltrack/sqlite.c | 49 ++++++++++++++++++++++++++++++++--------------
utils/nfsdcltrack/sqlite.h | 1 +
2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/utils/nfsdcltrack/sqlite.c b/utils/nfsdcltrack/sqlite.c
index c19af7e..bac6789 100644
--- a/utils/nfsdcltrack/sqlite.c
+++ b/utils/nfsdcltrack/sqlite.c
@@ -90,24 +90,15 @@ mkdir_if_not_exist(const char *dirname)
return ret;
}
-/*
- * Open the "main" database, and attempt to initialize it by creating the
- * parameters table and inserting the schema version into it. Ignore any errors
- * from that, and then attempt to select the version out of it again. If the
- * version appears wrong, then assume that the DB is corrupt or has been
- * upgraded, and return an error. If all of that works, then attempt to create
- * the "clients" table.
- */
+/* Open the database and set up the database handle for it */
int
-sqlite_maindb_init(const char *topdir)
+sqlite_prepare_dbh(const char *topdir)
{
int ret;
- char *err = NULL;
- sqlite3_stmt *stmt = NULL;
- ret = mkdir_if_not_exist(topdir);
- if (ret)
- return ret;
+ /* Do nothing if the database handle is already set up */
+ if (dbh)
+ return 0;
ret = snprintf(buf, PATH_MAX - 1, "%s/main.sqlite", topdir);
if (ret < 0)
@@ -118,15 +109,43 @@ sqlite_maindb_init(const char *topdir)
ret = sqlite3_open(buf, &dbh);
if (ret != SQLITE_OK) {
xlog(L_ERROR, "Unable to open main database: %d", ret);
+ dbh = NULL;
return ret;
}
ret = sqlite3_busy_timeout(dbh, CLD_SQLITE_BUSY_TIMEOUT);
if (ret != SQLITE_OK) {
xlog(L_ERROR, "Unable to set sqlite busy timeout: %d", ret);
- goto out_err;
+ sqlite3_close(dbh);
+ dbh = NULL;
}
+ return ret;
+}
+
+/*
+ * Open the "main" database, and attempt to initialize it by creating the
+ * parameters table and inserting the schema version into it. Ignore any errors
+ * from that, and then attempt to select the version out of it again. If the
+ * version appears wrong, then assume that the DB is corrupt or has been
+ * upgraded, and return an error. If all of that works, then attempt to create
+ * the "clients" table.
+ */
+int
+sqlite_maindb_init(const char *topdir)
+{
+ int ret;
+ char *err = NULL;
+ sqlite3_stmt *stmt = NULL;
+
+ ret = mkdir_if_not_exist(topdir);
+ if (ret)
+ return ret;
+
+ ret = sqlite_prepare_dbh(topdir);
+ if (ret)
+ return ret;
+
/* Try to create table */
ret = sqlite3_exec(dbh, "CREATE TABLE IF NOT EXISTS parameters "
"(key TEXT PRIMARY KEY, value TEXT);",
diff --git a/utils/nfsdcltrack/sqlite.h b/utils/nfsdcltrack/sqlite.h
index 8748948..ebf04c3 100644
--- a/utils/nfsdcltrack/sqlite.h
+++ b/utils/nfsdcltrack/sqlite.h
@@ -20,6 +20,7 @@
#ifndef _SQLITE_H_
#define _SQLITE_H_
+int sqlite_prepare_dbh(const char *topdir);
int sqlite_maindb_init(const char *topdir);
int sqlite_insert_client(const unsigned char *clname, const size_t namelen);
int sqlite_remove_client(const unsigned char *clname, const size_t namelen);
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/6] nfsdcltrack: add a new "one-shot" program for manipulating the client tracking db
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
` (2 preceding siblings ...)
2012-10-01 11:52 ` [PATCH 3/6] nfsdcltrack: break out a function to open the database handle Jeff Layton
@ 2012-10-01 11:52 ` Jeff Layton
2012-10-01 11:52 ` [PATCH 5/6] nfsdcltrack: add a manpage for nfsdcltrack Jeff Layton
` (2 subsequent siblings)
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-10-01 11:52 UTC (permalink / raw)
To: linux-nfs
Usermode helper upcalls are all the rage these days for infrequent
upcalls, since they make it rather idiot-proof to set up an upcall. No
running daemon is required, so there's really no setup beyond ensuring
that the callout exists and is runnable.
This program adds a callout program to nfs-utils for that purpose. The
storage engine on the backend is identical to the one used by nfsdcld.
This just adds a new frontend for it.
For now, building with --enable-nfsdcltrack gives you both nfsdcld and
nfsdcltrack programs. Eventually we may want to put these under separate
configure options or just deprecate nfsdcld if there's no interest in
maintaining it long-term.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsdcltrack/Makefile.am | 6 +-
utils/nfsdcltrack/nfsdcltrack.c | 435 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 439 insertions(+), 2 deletions(-)
create mode 100644 utils/nfsdcltrack/nfsdcltrack.c
diff --git a/utils/nfsdcltrack/Makefile.am b/utils/nfsdcltrack/Makefile.am
index 073a71b..afae455 100644
--- a/utils/nfsdcltrack/Makefile.am
+++ b/utils/nfsdcltrack/Makefile.am
@@ -4,11 +4,13 @@ man8_MANS = nfsdcld.man
EXTRA_DIST = $(man8_MANS)
AM_CFLAGS += -D_LARGEFILE64_SOURCE
-sbin_PROGRAMS = nfsdcld
+sbin_PROGRAMS = nfsdcld nfsdcltrack
nfsdcld_SOURCES = nfsdcld.c sqlite.c
-
nfsdcld_LDADD = ../../support/nfs/libnfs.a $(LIBEVENT) $(LIBSQLITE) $(LIBCAP)
+nfsdcltrack_SOURCES = nfsdcltrack.c sqlite.c
+nfsdcltrack_LDADD = ../../support/nfs/libnfs.a $(LIBSQLITE) $(LIBCAP)
+
MAINTAINERCLEANFILES = Makefile.in
diff --git a/utils/nfsdcltrack/nfsdcltrack.c b/utils/nfsdcltrack/nfsdcltrack.c
new file mode 100644
index 0000000..db7e534
--- /dev/null
+++ b/utils/nfsdcltrack/nfsdcltrack.c
@@ -0,0 +1,435 @@
+/*
+ * nfsdcltrack.c -- NFSv4 client name tracking program
+ *
+ * Copyright (C) 2012 Jeff Layton <jlayton@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor,
+ * Boston, MA 02110-1301, USA.
+ */
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif /* HAVE_CONFIG_H */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <ctype.h>
+#include <errno.h>
+#include <stdbool.h>
+#include <getopt.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <libgen.h>
+#include <sys/inotify.h>
+#ifdef HAVE_SYS_CAPABILITY_H
+#include <sys/prctl.h>
+#include <sys/capability.h>
+#endif
+
+#include "xlog.h"
+#include "sqlite.h"
+
+#ifndef CLD_DEFAULT_STORAGEDIR
+#define CLD_DEFAULT_STORAGEDIR NFS_STATEDIR "/nfsdcltrack"
+#endif
+
+/* defined by RFC 3530 */
+#define NFS4_OPAQUE_LIMIT 1024
+
+/* private data structures */
+struct cltrack_cmd {
+ char *name;
+ bool needs_arg;
+ int (*func)(const char *arg);
+};
+
+/* forward declarations */
+static int cltrack_init(const char *unused);
+static int cltrack_create(const char *id);
+static int cltrack_remove(const char *id);
+static int cltrack_check(const char *id);
+static int cltrack_gracedone(const char *gracetime);
+
+/* global variables */
+static struct option longopts[] =
+{
+ { "help", 0, NULL, 'h' },
+ { "debug", 0, NULL, 'd' },
+ { "foreground", 0, NULL, 'f' },
+ { "storagedir", 1, NULL, 's' },
+ { NULL, 0, 0, 0 },
+};
+
+static struct cltrack_cmd commands[] =
+{
+ { "init", false, cltrack_init },
+ { "create", true, cltrack_create },
+ { "remove", true, cltrack_remove },
+ { "check", true, cltrack_check },
+ { "gracedone", true, cltrack_gracedone },
+ { NULL, false, NULL },
+};
+
+static char *storagedir = CLD_DEFAULT_STORAGEDIR;
+
+/* common buffer for holding id4 blobs */
+static unsigned char blob[NFS4_OPAQUE_LIMIT];
+
+static void
+usage(char *progname)
+{
+ printf("%s [ -hfd ] [ -s dir ] < cmd > < arg >\n", progname);
+ printf("Where < cmd > is one of the following and takes the following < arg >:\n");
+ printf(" init\n");
+ printf(" create <nfs_client_id4>\n");
+ printf(" remove <nfs_client_id4>\n");
+ printf(" check <nfs_client_id4>\n");
+ printf(" gracedone <epoch time>\n");
+}
+
+
+/**
+ * hex_to_bin - convert a hex digit to its real value
+ * @ch: ascii character represents hex digit
+ *
+ * hex_to_bin() converts one hex digit to its actual value or -1 in case of bad
+ * input.
+ *
+ * Note: borrowed from lib/hexdump.c in the Linux kernel sources.
+ */
+static int
+hex_to_bin(char ch)
+{
+ if ((ch >= '0') && (ch <= '9'))
+ return ch - '0';
+ ch = tolower(ch);
+ if ((ch >= 'a') && (ch <= 'f'))
+ return ch - 'a' + 10;
+ return -1;
+}
+
+/**
+ * hex_str_to_bin - convert a hexidecimal string into a binary blob
+ *
+ * @src: string of hex digit pairs
+ * @dst: destination buffer to hold binary data
+ * @dstsize: size of the destination buffer
+ *
+ * Walk a string of hex digit pairs and convert them into binary data. Returns
+ * the resulting length of the binary data or a negative error code. If the
+ * data will not fit in the buffer, it returns -ENOBUFS (but will likely have
+ * clobbered the dst buffer in the process of determining that). If there are
+ * non-hexidecimal characters in the src, or an odd number of them then it
+ * returns -EINVAL.
+ */
+static ssize_t
+hex_str_to_bin(const char *src, unsigned char *dst, ssize_t dstsize)
+{
+ unsigned char *tmpdst = dst;
+
+ while (*src) {
+ int hi, lo;
+
+ /* make sure we don't overrun the dst buffer */
+ if ((tmpdst - dst) >= dstsize)
+ return -ENOBUFS;
+
+ hi = hex_to_bin(*src++);
+
+ /* did we get an odd number of characters? */
+ if (!*src)
+ return -EINVAL;
+ lo = hex_to_bin(*src++);
+
+ /* one of the characters isn't a hex digit */
+ if (hi < 0 || lo < 0)
+ return -EINVAL;
+
+ /* now place it in the dst buffer */
+ *tmpdst++ = (hi << 4) | lo;
+ }
+
+ return (ssize_t)(tmpdst - dst);
+}
+
+/*
+ * This program will almost always be run with root privileges since the
+ * kernel will call out to run it. Drop all capabilities prior to doing
+ * anything important to limit the exposure to potential compromise.
+ *
+ * FIXME: should we setuid to a different user early on instead?
+ */
+static int
+cltrack_set_caps(void)
+{
+ int ret = 0;
+#ifdef HAVE_SYS_CAPABILITY_H
+ unsigned long i;
+ cap_t caps;
+
+ /* prune the bounding set to nothing */
+ for (i = 0; prctl(PR_CAPBSET_READ, i, 0, 0, 0) >= 0 ; ++i) {
+ ret = prctl(PR_CAPBSET_DROP, i, 0, 0, 0);
+ if (ret) {
+ xlog(L_ERROR, "Unable to prune capability %lu from "
+ "bounding set: %m", i);
+ return -errno;
+ }
+ }
+
+ /* get a blank capset */
+ caps = cap_init();
+ if (caps == NULL) {
+ xlog(L_ERROR, "Unable to get blank capability set: %m");
+ return -errno;
+ }
+
+ /* reset the process capabilities */
+ if (cap_set_proc(caps) != 0) {
+ xlog(L_ERROR, "Unable to set process capabilities: %m");
+ ret = -errno;
+ }
+ cap_free(caps);
+#endif
+ return ret;
+}
+
+static int
+cltrack_init(const char __attribute__((unused)) *unused)
+{
+ int ret;
+
+ /*
+ * see if the storagedir is writable by root w/o CAP_DAC_OVERRIDE.
+ * If it isn't then give the user a warning but proceed as if
+ * everything is OK. If the DB has already been created, then
+ * everything might still work. If it doesn't exist at all, then
+ * assume that the maindb init will be able to create it. Fail on
+ * anything else.
+ */
+ if (access(storagedir, W_OK) == -1) {
+ switch (errno) {
+ case EACCES:
+ xlog(L_WARNING, "Storage directory %s is not writable. "
+ "Should be owned by root and writable "
+ "by owner!", storagedir);
+ break;
+ case ENOENT:
+ /* ignore and assume that we can create dir as root */
+ break;
+ default:
+ xlog(L_ERROR, "Unexpected error when checking access "
+ "on %s: %m", storagedir);
+ return -errno;
+ }
+ }
+
+ /* set up storage db */
+ ret = sqlite_maindb_init(storagedir);
+ if (ret) {
+ xlog(L_ERROR, "Failed to init database: %d", ret);
+ /*
+ * Convert any error here into -EACCES. It's not truly
+ * accurate in all cases, but it should cause the kernel to
+ * stop upcalling until the problem is resolved.
+ */
+ ret = -EACCES;
+ }
+ return ret;
+}
+
+static int
+cltrack_create(const char *id)
+{
+ int ret;
+ ssize_t len;
+
+ xlog(D_GENERAL, "%s: create client record.", __func__);
+
+ ret = sqlite_prepare_dbh(storagedir);
+ if (ret)
+ return ret;
+
+ len = hex_str_to_bin(id, blob, sizeof(blob));
+ if (len < 0)
+ return (int)len;
+
+ ret = sqlite_insert_client(blob, len);
+
+ return ret ? -EREMOTEIO : ret;
+}
+
+static int
+cltrack_remove(const char *id)
+{
+ int ret;
+ ssize_t len;
+
+ xlog(D_GENERAL, "%s: remove client record.", __func__);
+
+ ret = sqlite_prepare_dbh(storagedir);
+ if (ret)
+ return ret;
+
+ len = hex_str_to_bin(id, blob, sizeof(blob));
+ if (len < 0)
+ return (int)len;
+
+ ret = sqlite_remove_client(blob, len);
+
+ return ret ? -EREMOTEIO : ret;
+}
+
+static int
+cltrack_check(const char *id)
+{
+ int ret;
+ ssize_t len;
+
+ xlog(D_GENERAL, "%s: check client record", __func__);
+
+ ret = sqlite_prepare_dbh(storagedir);
+ if (ret)
+ return ret;
+
+ len = hex_str_to_bin(id, blob, sizeof(blob));
+ if (len < 0)
+ return (int)len;
+
+ ret = sqlite_check_client(blob, len);
+
+ return ret ? -EPERM : ret;
+}
+
+static int
+cltrack_gracedone(const char *timestr)
+{
+ int ret;
+ char *tail;
+ time_t gracetime;
+
+
+ ret = sqlite_prepare_dbh(storagedir);
+ if (ret)
+ return ret;
+
+ errno = 0;
+ gracetime = strtol(timestr, &tail, 0);
+
+ /* did the resulting value overflow? (Probably -ERANGE here) */
+ if (errno)
+ return -errno;
+
+ /* string wasn't fully converted */
+ if (*tail)
+ return -EINVAL;
+
+ xlog(D_GENERAL, "%s: grace done. gracetime=%ld", __func__, gracetime);
+
+ ret = sqlite_remove_unreclaimed(gracetime);
+
+ return ret ? -EREMOTEIO : ret;
+}
+
+static struct cltrack_cmd *
+find_cmd(char *cmdname)
+{
+ struct cltrack_cmd *current = &commands[0];
+
+ while (current->name) {
+ if (!strcmp(cmdname, current->name))
+ return current;
+ ++current;
+ }
+
+ xlog(L_ERROR, "%s: '%s' doesn't match any known command",
+ __func__, cmdname);
+ return NULL;
+}
+
+int
+main(int argc, char **argv)
+{
+ char arg;
+ int rc = 0;
+ char *progname, *cmdarg = NULL;
+ struct cltrack_cmd *cmd;
+
+ progname = strdup(basename(argv[0]));
+ if (!progname) {
+ fprintf(stderr, "%s: unable to allocate memory.\n", argv[0]);
+ return 1;
+ }
+
+ xlog_syslog(1);
+ xlog_stderr(0);
+
+ /* process command-line options */
+ while ((arg = getopt_long(argc, argv, "hdfs:", longopts,
+ NULL)) != EOF) {
+ switch (arg) {
+ case 'd':
+ xlog_config(D_ALL, 1);
+ case 'f':
+ xlog_syslog(0);
+ xlog_stderr(1);
+ break;
+ case 's':
+ storagedir = optarg;
+ break;
+ default:
+ usage(progname);
+ return 0;
+ }
+ }
+
+ xlog_open(progname);
+
+ /* we expect a command, at least */
+ if (optind >= argc) {
+ xlog(L_ERROR, "Missing command name\n");
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* drop all capabilities */
+ rc = cltrack_set_caps();
+ if (rc)
+ goto out;
+
+ cmd = find_cmd(argv[optind]);
+ if (!cmd) {
+ rc = -EINVAL;
+ goto out;
+ }
+
+ /* populate arg var if command needs it */
+ if (cmd->needs_arg) {
+ if (optind + 1 >= argc) {
+ xlog(L_ERROR, "Command %s requires an argument\n",
+ cmd->name);
+ rc = -EINVAL;
+ goto out;
+ }
+ cmdarg = argv[optind + 1];
+ }
+ rc = cmd->func(cmdarg);
+out:
+ free(progname);
+ return rc;
+}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/6] nfsdcltrack: add a manpage for nfsdcltrack
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
` (3 preceding siblings ...)
2012-10-01 11:52 ` [PATCH 4/6] nfsdcltrack: add a new "one-shot" program for manipulating the client tracking db Jeff Layton
@ 2012-10-01 11:52 ` Jeff Layton
2012-10-01 11:52 ` [PATCH 6/6] nfsdcltrack: update the README about server startup order Jeff Layton
2012-10-03 12:42 ` [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism Jeff Layton
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-10-01 11:52 UTC (permalink / raw)
To: linux-nfs
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsdcltrack/Makefile.am | 2 +-
utils/nfsdcltrack/nfsdcltrack.man | 194 ++++++++++++++++++++++++++++++++++++++
2 files changed, 195 insertions(+), 1 deletion(-)
create mode 100644 utils/nfsdcltrack/nfsdcltrack.man
diff --git a/utils/nfsdcltrack/Makefile.am b/utils/nfsdcltrack/Makefile.am
index afae455..1746a25 100644
--- a/utils/nfsdcltrack/Makefile.am
+++ b/utils/nfsdcltrack/Makefile.am
@@ -1,6 +1,6 @@
## Process this file with automake to produce Makefile.in
-man8_MANS = nfsdcld.man
+man8_MANS = nfsdcld.man nfsdcltrack.man
EXTRA_DIST = $(man8_MANS)
AM_CFLAGS += -D_LARGEFILE64_SOURCE
diff --git a/utils/nfsdcltrack/nfsdcltrack.man b/utils/nfsdcltrack/nfsdcltrack.man
new file mode 100644
index 0000000..bf64dc6
--- /dev/null
+++ b/utils/nfsdcltrack/nfsdcltrack.man
@@ -0,0 +1,194 @@
+.\" Automatically generated by Pod::Man 2.25 (Pod::Simple 3.16)
+.\"
+.\" Standard preamble:
+.\" ========================================================================
+.de Sp \" Vertical space (when we can't use .PP)
+.if t .sp .5v
+.if n .sp
+..
+.de Vb \" Begin verbatim text
+.ft CW
+.nf
+.ne \\$1
+..
+.de Ve \" End verbatim text
+.ft R
+.fi
+..
+.\" Set up some character translations and predefined strings. \*(-- will
+.\" give an unbreakable dash, \*(PI will give pi, \*(L" will give a left
+.\" double quote, and \*(R" will give a right double quote. \*(C+ will
+.\" give a nicer C++. Capital omega is used to do unbreakable dashes and
+.\" therefore won't be available. \*(C` and \*(C' expand to `' in nroff,
+.\" nothing in troff, for use with C<>.
+.tr \(*W-
+.ds C+ C\v'-.1v'\h'-1p'\s-2+\h'-1p'+\s0\v'.1v'\h'-1p'
+.ie n \{\
+. ds -- \(*W-
+. ds PI pi
+. if (\n(.H=4u)&(1m=24u) .ds -- \(*W\h'-12u'\(*W\h'-12u'-\" diablo 10 pitch
+. if (\n(.H=4u)&(1m=20u) .ds -- \(*W\h'-12u'\(*W\h'-8u'-\" diablo 12 pitch
+. ds L" ""
+. ds R" ""
+. ds C` ""
+. ds C' ""
+'br\}
+.el\{\
+. ds -- \|\(em\|
+. ds PI \(*p
+. ds L" ``
+. ds R" ''
+'br\}
+.\"
+.\" Escape single quotes in literal strings from groff's Unicode transform.
+.ie \n(.g .ds Aq \(aq
+.el .ds Aq '
+.\"
+.\" If the F register is turned on, we'll generate index entries on stderr for
+.\" titles (.TH), headers (.SH), subsections (.SS), items (.Ip), and index
+.\" entries marked with X<> in POD. Of course, you'll have to process the
+.\" output yourself in some meaningful fashion.
+.ie \nF \{\
+. de IX
+. tm Index:\\$1\t\\n%\t"\\$2"
+..
+. nr % 0
+. rr F
+.\}
+.el \{\
+. de IX
+..
+.\}
+.\"
+.\" Accent mark definitions (@(#)ms.acc 1.5 88/02/08 SMI; from UCB 4.2).
+.\" Fear. Run. Save yourself. No user-serviceable parts.
+. \" fudge factors for nroff and troff
+.if n \{\
+. ds #H 0
+. ds #V .8m
+. ds #F .3m
+. ds #[ \f1
+. ds #] \fP
+.\}
+.if t \{\
+. ds #H ((1u-(\\\\n(.fu%2u))*.13m)
+. ds #V .6m
+. ds #F 0
+. ds #[ \&
+. ds #] \&
+.\}
+. \" simple accents for nroff and troff
+.if n \{\
+. ds ' \&
+. ds ` \&
+. ds ^ \&
+. ds , \&
+. ds ~ ~
+. ds /
+.\}
+.if t \{\
+. ds ' \\k:\h'-(\\n(.wu*8/10-\*(#H)'\'\h"|\\n:u"
+. ds ` \\k:\h'-(\\n(.wu*8/10-\*(#H)'\`\h'|\\n:u'
+. ds ^ \\k:\h'-(\\n(.wu*10/11-\*(#H)'^\h'|\\n:u'
+. ds , \\k:\h'-(\\n(.wu*8/10)',\h'|\\n:u'
+. ds ~ \\k:\h'-(\\n(.wu-\*(#H-.1m)'~\h'|\\n:u'
+. ds / \\k:\h'-(\\n(.wu*8/10-\*(#H)'\z\(sl\h'|\\n:u'
+.\}
+. \" troff and (daisy-wheel) nroff accents
+.ds : \\k:\h'-(\\n(.wu*8/10-\*(#H+.1m+\*(#F)'\v'-\*(#V'\z.\h'.2m+\*(#F'.\h'|\\n:u'\v'\*(#V'
+.ds 8 \h'\*(#H'\(*b\h'-\*(#H'
+.ds o \\k:\h'-(\\n(.wu+\w'\(de'u-\*(#H)/2u'\v'-.3n'\*(#[\z\(de\v'.3n'\h'|\\n:u'\*(#]
+.ds d- \h'\*(#H'\(pd\h'-\w'~'u'\v'-.25m'\f2\(hy\fP\v'.25m'\h'-\*(#H'
+.ds D- D\\k:\h'-\w'D'u'\v'-.11m'\z\(hy\v'.11m'\h'|\\n:u'
+.ds th \*(#[\v'.3m'\s+1I\s-1\v'-.3m'\h'-(\w'I'u*2/3)'\s-1o\s+1\*(#]
+.ds Th \*(#[\s+2I\s-2\h'-\w'I'u*3/5'\v'-.3m'o\v'.3m'\*(#]
+.ds ae a\h'-(\w'a'u*4/10)'e
+.ds Ae A\h'-(\w'A'u*4/10)'E
+. \" corrections for vroff
+.if v .ds ~ \\k:\h'-(\\n(.wu*9/10-\*(#H)'\s-2\u~\d\s+2\h'|\\n:u'
+.if v .ds ^ \\k:\h'-(\\n(.wu*10/11-\*(#H)'\v'-.4m'^\v'.4m'\h'|\\n:u'
+. \" for low resolution devices (crt and lpr)
+.if \n(.H>23 .if \n(.V>19 \
+\{\
+. ds : e
+. ds 8 ss
+. ds o a
+. ds d- d\h'-1'\(ga
+. ds D- D\h'-1'\(hy
+. ds th \o'bp'
+. ds Th \o'LP'
+. ds ae ae
+. ds Ae AE
+.\}
+.rm #[ #] #H #V #F C
+.\" ========================================================================
+.\"
+.IX Title "NFSDCLTRACK 8"
+.TH NFSDCLTRACK 8 "2012-10-01" "" ""
+.\" For nroff, turn off justification. Always turn off hyphenation; it makes
+.\" way too many mistakes in technical documents.
+.if n .ad l
+.nh
+.SH "NAME"
+nfsdcltrack \- NFSv4 Client Tracking Callout Program
+.SH "SYNOPSIS"
+.IX Header "SYNOPSIS"
+nfsdcltrack [\-d] [\-f] [\-s stable storage dir] <command> <args...>
+.SH "DESCRIPTION"
+.IX Header "DESCRIPTION"
+nfsdcltack is the NFSv4 client tracking callout program. It is not necessary
+to install this daemon on machines that are not acting as NFSv4 servers.
+.PP
+When a network partition is combined with a server reboot, there are
+edge conditions that can cause the server to grant lock reclaims when
+other clients have taken conflicting locks in the interim. A more detailed
+explanation of this issue is described in \s-1RFC\s0 3530, section 8.6.3.
+.PP
+In order to prevent these problems, the server must track a small amount
+of per-client information on stable storage. This program provides the
+userspace piece of that functionality. When the kernel needs to manipulate
+the database that stores this info, it will execute this program to handle
+it.
+.SH "OPTIONS"
+.IX Header "OPTIONS"
+.IP "\fB\-d\fR, \fB\-\-debug\fR" 4
+.IX Item "-d, --debug"
+Enable debug level logging.
+.IP "\fB\-f\fR, \fB\-\-foreground\fR" 4
+.IX Item "-f, --foreground"
+Log to stderr instead of syslog.
+.IP "\fB\-s\fR \fIstoragedir\fR, \fB\-\-storagedir\fR=\fIstorage_dir\fR" 4
+.IX Item "-s storagedir, --storagedir=storage_dir"
+Directory where stable storage information should be kept. The default
+value is \fI/var/lib/nfs/nfsdcltrack\fR.
+.SH "COMMANDS"
+.IX Header "COMMANDS"
+nfsdcltrack requires a command for each invocation. Supported commands
+are:
+.IP "\fBinit\fR" 4
+.IX Item "init"
+Initialize the database. This command requires no argument.
+.IP "\fBcreate\fR" 4
+.IX Item "create"
+Create a new client record (or update the timestamp on an existing one). This command requires a hex-encoded nfs_client_id4 as an argument.
+.IP "\fBremove\fR" 4
+.IX Item "remove"
+Remove a client record from the database. This command requires a hex-encoded nfs_client_id4 as an argument.
+.IP "\fBcheck\fR" 4
+.IX Item "check"
+Check to see if a nfs_client_id4 is allowed to reclaim. This command requires a hex-encoded nfs_client_id4 as an argument.
+.IP "\fBgracedone\fR" 4
+.IX Item "gracedone"
+Remove any unreclaimed client records from the database. This command requires a epoch boot time as an argument.
+.SH "NOTES"
+.IX Header "NOTES"
+The Linux kernel NFSv4 server has historically tracked this information
+on stable storage by manipulating information on the filesystem
+directly, in the directory to which \fI/proc/fs/nfsd/nfsv4recoverydir\fR
+points.
+.PP
+This program requires a kernel that supports the nfsdcltrack usermodehelper
+upcall. This support was first added in mainline kernel 3.7.
+.SH "AUTHORS"
+.IX Header "AUTHORS"
+nfsdcltrack was developed by Jeff Layton <jlayton@redhat.com>.
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/6] nfsdcltrack: update the README about server startup order
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
` (4 preceding siblings ...)
2012-10-01 11:52 ` [PATCH 5/6] nfsdcltrack: add a manpage for nfsdcltrack Jeff Layton
@ 2012-10-01 11:52 ` Jeff Layton
2012-10-03 12:42 ` [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism Jeff Layton
6 siblings, 0 replies; 13+ messages in thread
From: Jeff Layton @ 2012-10-01 11:52 UTC (permalink / raw)
To: linux-nfs
If we eventually deprecate nfsdcld, then we can just remove this
section I think.
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
README | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/README b/README
index e55b2dd..f997783 100644
--- a/README
+++ b/README
@@ -110,8 +110,9 @@ scripts can be written to work correctly.
E/ nfsdcld
This daemon is only needed on kernels that support the nfsdcld
- upcall, and only if the legacy client ID tracking isn't used. It
- is also not needed if the server does not support NFSv4.
+ upcall, and only if the legacy client ID tracking and nfsdcltrack
+ aren't used. It is also not needed if the server does not support
+ NFSv4.
To determine whether you need this or not, do the following:
@@ -124,7 +125,12 @@ scripts can be written to work correctly.
# cat /proc/fs/nfsd/nfsv4recoverydir
If that file is not present, or the directory that the above command
- outputs is not present, then this daemon is required in order to
+ outputs is not present, then check this:
+
+ # modinfo nfsd | grep cltrack_prog
+
+ to see whether the kernel supports the nfsdcltrack upcall. If
+ none of those are configured then this daemon is required in order to
support lock recovery by the clients when the server reboots.
F/ rpc.nfsd
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism
2012-10-01 11:52 [PATCH 0/6] nfsdcltrack: create a new usermodehelper upcall program for tracking clients Jeff Layton
` (5 preceding siblings ...)
2012-10-01 11:52 ` [PATCH 6/6] nfsdcltrack: update the README about server startup order Jeff Layton
@ 2012-10-03 12:42 ` Jeff Layton
2012-10-03 13:34 ` Scott Lovenberg
6 siblings, 1 reply; 13+ messages in thread
From: Jeff Layton @ 2012-10-03 12:42 UTC (permalink / raw)
To: steved; +Cc: linux-nfs
If the kernel passes the legacy recdir path in the environment, then we
can use that to transition from the old legacy tracker to the new one.
On a "check" operation, if there is no record of the client in the
database, check to see if there is a matching recoverydir. If there
isn't then just refuse the reclaim. If there is, then insert a new
record for this client into the db, and remove the legacy recoverydir.
If either of those operations fail, then refuse the reclaim.
On a "gracedone" operation, clean out the entire legacy recoverydir
after purging any unreclaimed records from the db. There's not much
we can do if this fails, so just log a warning if it does.
Note that this is a one-way conversion. If the user later boots back
into an older kernel, it will have no knowledge of the new database.
In principle, we could create a tool that would walk the clients
table, md5 hash the clientids and create directories in the
v4recovery dir. Trying to do that automatically would be a mess
though...
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
utils/nfsdcltrack/nfsdcltrack.c | 87 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/utils/nfsdcltrack/nfsdcltrack.c b/utils/nfsdcltrack/nfsdcltrack.c
index db7e534..7c2625a 100644
--- a/utils/nfsdcltrack/nfsdcltrack.c
+++ b/utils/nfsdcltrack/nfsdcltrack.c
@@ -36,6 +36,7 @@
#include <unistd.h>
#include <libgen.h>
#include <sys/inotify.h>
+#include <dirent.h>
#ifdef HAVE_SYS_CAPABILITY_H
#include <sys/prctl.h>
#include <sys/capability.h>
@@ -296,6 +297,50 @@ cltrack_remove(const char *id)
}
static int
+cltrack_check_legacy(const unsigned char *blob, const ssize_t len)
+{
+ int ret;
+ char *recdir;
+ struct stat st;
+
+ /* see if the legacy recdir exists */
+ recdir = getenv("NFSDCLTRACK_LEGACY_RECDIR");
+ if (!recdir) {
+ xlog(D_GENERAL, "No NFSDCLTRACK_LEGACY_RECDIR env var");
+ return -EOPNOTSUPP;
+ }
+
+ /* fail recovery on any stat failure */
+ ret = stat(recdir, &st);
+ if (ret) {
+ xlog(D_GENERAL, "Unable to stat %s: %d", recdir, errno);
+ return -errno;
+ }
+
+ /* fail if it isn't a directory */
+ if (!S_ISDIR(st.st_mode)) {
+ xlog(D_GENERAL, "%s is not a directory: mode=0%o", recdir
+ , st.st_mode);
+ return -ENOTDIR;
+ }
+
+ /* Dir exists, try to insert record into db */
+ ret = sqlite_insert_client(blob, len);
+ if (ret) {
+ xlog(D_GENERAL, "Failed to insert client: %d", ret);
+ return -EREMOTEIO;
+ }
+
+ /* remove the legacy recoverydir */
+ ret = rmdir(recdir);
+ if (ret) {
+ xlog(D_GENERAL, "Failed to rmdir %s: %d", recdir, errno);
+ return -errno;
+ }
+ return 0;
+}
+
+static int
cltrack_check(const char *id)
{
int ret;
@@ -312,10 +357,50 @@ cltrack_check(const char *id)
return (int)len;
ret = sqlite_check_client(blob, len);
+ if (ret)
+ ret = cltrack_check_legacy(blob, len);
return ret ? -EPERM : ret;
}
+/* Clean out the v4recoverydir -- best effort here */
+static void
+cltrack_legacy_gracedone(void)
+{
+ char *dirname = getenv("NFSDCLTRACK_LEGACY_TOPDIR");
+ DIR *v4recovery;
+ struct dirent *entry;
+
+ if (!dirname)
+ return;
+
+ v4recovery = opendir(dirname);
+ if (!v4recovery)
+ return;
+
+ while ((entry = readdir(v4recovery))) {
+ int len;
+
+ /* borrow the clientid blob for this */
+ len = snprintf((char *)blob, sizeof(blob), "%s/%s", dirname,
+ entry->d_name);
+
+ /* if there's a problem, then skip this entry */
+ if (len < 0 || (size_t)len >= sizeof(blob)) {
+ xlog(L_WARNING, "%s: unable to build filename for %s!",
+ __func__, entry->d_name);
+ continue;
+ }
+
+ len = rmdir((char *)blob);
+ if (len)
+ xlog(L_WARNING, "%s: unable to rmdir %s: %d", __func__,
+ (char *)blob, len);
+ }
+
+ closedir(v4recovery);
+}
+
static int
cltrack_gracedone(const char *timestr)
{
@@ -343,6 +428,8 @@ cltrack_gracedone(const char *timestr)
ret = sqlite_remove_unreclaimed(gracetime);
+ cltrack_legacy_gracedone();
+
return ret ? -EREMOTEIO : ret;
}
--
1.7.11.4
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism
2012-10-03 12:42 ` [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism Jeff Layton
@ 2012-10-03 13:34 ` Scott Lovenberg
2012-10-03 13:40 ` Jeff Layton
0 siblings, 1 reply; 13+ messages in thread
From: Scott Lovenberg @ 2012-10-03 13:34 UTC (permalink / raw)
To: Jeff Layton; +Cc: steved, linux-nfs
On Wed, Oct 3, 2012 at 8:42 AM, Jeff Layton <jlayton@redhat.com> wrote:
[snip]
> + /* if there's a problem, then skip this entry */
> + if (len < 0 || (size_t)len >= sizeof(blob)) {
> + xlog(L_WARNING, "%s: unable to build filename for %s!",
> + __func__, entry->d_name);
> + continue;
> + }
Is a len == 0 not a problem?
--
Peace and Blessings,
-Scott.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism
2012-10-03 13:34 ` Scott Lovenberg
@ 2012-10-03 13:40 ` Jeff Layton
2012-10-03 13:52 ` Scott Lovenberg
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Layton @ 2012-10-03 13:40 UTC (permalink / raw)
To: Scott Lovenberg; +Cc: steved, linux-nfs
On Wed, 3 Oct 2012 09:34:36 -0400
Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
> On Wed, Oct 3, 2012 at 8:42 AM, Jeff Layton <jlayton@redhat.com> wrote:
> [snip]
> > + /* if there's a problem, then skip this entry */
> > + if (len < 0 || (size_t)len >= sizeof(blob)) {
> > + xlog(L_WARNING, "%s: unable to build filename for %s!",
> > + __func__, entry->d_name);
> > + continue;
> > + }
>
> Is a len == 0 not a problem?
>
I'm not sure -- under what circumstances would it return 0?
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism
2012-10-03 13:40 ` Jeff Layton
@ 2012-10-03 13:52 ` Scott Lovenberg
2012-10-03 13:54 ` Jeff Layton
0 siblings, 1 reply; 13+ messages in thread
From: Scott Lovenberg @ 2012-10-03 13:52 UTC (permalink / raw)
To: Jeff Layton; +Cc: steved, linux-nfs
On Wed, Oct 3, 2012 at 9:40 AM, Jeff Layton <jlayton@redhat.com> wrote:
> On Wed, 3 Oct 2012 09:34:36 -0400
> Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
>
>> On Wed, Oct 3, 2012 at 8:42 AM, Jeff Layton <jlayton@redhat.com> wrote:
>> [snip]
>> > + /* if there's a problem, then skip this entry */
>> > + if (len < 0 || (size_t)len >= sizeof(blob)) {
>> > + xlog(L_WARNING, "%s: unable to build filename for %s!",
>> > + __func__, entry->d_name);
>> > + continue;
>> > + }
>>
>> Is a len == 0 not a problem?
>>
>
> I'm not sure -- under what circumstances would it return 0?
>
> --
> Jeff Layton <jlayton@redhat.com>
I guess the only case would be if sizeof(blob) was 0, then snprintf
would return a value < 1 according to the SUSv2. Apparently C99 says
the return value will be > 0. I guess this is a non-issue?
--
Peace and Blessings,
-Scott.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism
2012-10-03 13:52 ` Scott Lovenberg
@ 2012-10-03 13:54 ` Jeff Layton
2012-10-03 13:55 ` Scott Lovenberg
0 siblings, 1 reply; 13+ messages in thread
From: Jeff Layton @ 2012-10-03 13:54 UTC (permalink / raw)
To: Scott Lovenberg; +Cc: steved, linux-nfs
On Wed, 3 Oct 2012 09:52:02 -0400
Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
> On Wed, Oct 3, 2012 at 9:40 AM, Jeff Layton <jlayton@redhat.com> wrote:
> > On Wed, 3 Oct 2012 09:34:36 -0400
> > Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
> >
> >> On Wed, Oct 3, 2012 at 8:42 AM, Jeff Layton <jlayton@redhat.com> wrote:
> >> [snip]
> >> > + /* if there's a problem, then skip this entry */
> >> > + if (len < 0 || (size_t)len >= sizeof(blob)) {
> >> > + xlog(L_WARNING, "%s: unable to build filename for %s!",
> >> > + __func__, entry->d_name);
> >> > + continue;
> >> > + }
> >>
> >> Is a len == 0 not a problem?
> >>
> >
> > I'm not sure -- under what circumstances would it return 0?
> >
> > --
> > Jeff Layton <jlayton@redhat.com>
>
> I guess the only case would be if sizeof(blob) was 0, then snprintf
> would return a value < 1 according to the SUSv2. Apparently C99 says
> the return value will be > 0. I guess this is a non-issue?
>
I think so, yes. The main worry here is overrunning the buffer and I
think the error checking I'm doing has that covered.
--
Jeff Layton <jlayton@redhat.com>
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 7/6] nfsdcltrack: add a legacy transition mechanism
2012-10-03 13:54 ` Jeff Layton
@ 2012-10-03 13:55 ` Scott Lovenberg
0 siblings, 0 replies; 13+ messages in thread
From: Scott Lovenberg @ 2012-10-03 13:55 UTC (permalink / raw)
To: Jeff Layton; +Cc: steved, linux-nfs
On Wed, Oct 3, 2012 at 9:54 AM, Jeff Layton <jlayton@redhat.com> wrote:
> On Wed, 3 Oct 2012 09:52:02 -0400
> Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
>
>> On Wed, Oct 3, 2012 at 9:40 AM, Jeff Layton <jlayton@redhat.com> wrote:
>> > On Wed, 3 Oct 2012 09:34:36 -0400
>> > Scott Lovenberg <scott.lovenberg@gmail.com> wrote:
>> >
>> >> On Wed, Oct 3, 2012 at 8:42 AM, Jeff Layton <jlayton@redhat.com> wrote:
>> >> [snip]
>> >> > + /* if there's a problem, then skip this entry */
>> >> > + if (len < 0 || (size_t)len >= sizeof(blob)) {
>> >> > + xlog(L_WARNING, "%s: unable to build filename for %s!",
>> >> > + __func__, entry->d_name);
>> >> > + continue;
>> >> > + }
>> >>
>> >> Is a len == 0 not a problem?
>> >>
>> >
>> > I'm not sure -- under what circumstances would it return 0?
>> >
>> > --
>> > Jeff Layton <jlayton@redhat.com>
>>
>> I guess the only case would be if sizeof(blob) was 0, then snprintf
>> would return a value < 1 according to the SUSv2. Apparently C99 says
>> the return value will be > 0. I guess this is a non-issue?
>>
>
> I think so, yes. The main worry here is overrunning the buffer and I
> think the error checking I'm doing has that covered.
>
> --
> Jeff Layton <jlayton@redhat.com>
Agreed. Sorry for the noise. :)
--
Peace and Blessings,
-Scott.
^ permalink raw reply [flat|nested] 13+ messages in thread