* [PATCH 00/19] REPOST (with addn'l patches) nfs-utils cleanups
@ 2008-09-05 21:06 Chuck Lever
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
0 siblings, 1 reply; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:06 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Hi Steve-
Here are a group of minor clean-ups to nfs-utils culled from my IPv6
patch set. It would be nice to get these sorted before we start into
the meat of the client-side IPv6-related changes to nfs-utils. Please
consider these for the next release of nfs-utils.
In addition to the RPC_ANYSOCK change suggested by Neil, there are
three additional small patches (8, 9, and 18 in this repost) that I
found yesterday.
---
Chuck Lever (19):
rpc.statd: Stop overloading sockfd in utils/statd/rmtcall.c
rpc.statd: Use __func__ in dprintf
rpc.statd: Clean up: replace "if (!(foo = rtnl))".
nfs-utils: whitespace clean ups in support/nfs/rpcmisc.c
nfs-utils: Remove excess log reporting
nfs-utils: make makesock() static
nfs-utils: Clean up support/nfs/rpcmisc.c:closedown()
nfs-utils: remove disabled code from support/nfs/rpcmisc.c
nfs-utils: Remove unused function rpc_svcrun()
nfs-utils: remove unused function rpc_logcall()
sm-notify command: use static function definitions
sm-notify command: replace nsm_address typedef
sm-notify command: clean up error logging
sm-notify command: getaddrinfo(3) addrinfo leak
sm-notify command: include <config.h>
showmount command: clean up error returns from connect_nb()
showmount: destroy RPC client when finished
rpc.statd: eliminate --secure_statd
rpc.statd: refactor check to see if call is from loopback address
configure.ac | 9 --
support/include/rpcmisc.h | 3 -
support/nfs/rpcdispatch.c | 51 ----------
support/nfs/rpcmisc.c | 225 ++++++++++++++-----------------------------
utils/showmount/showmount.c | 27 +++--
utils/statd/callback.c | 6 -
utils/statd/monitor.c | 100 ++++++-------------
utils/statd/rmtcall.c | 78 ++++++++-------
utils/statd/simu.c | 35 ++-----
utils/statd/sm-notify.c | 206 +++++++++++++++++++++------------------
utils/statd/statd.c | 4 -
utils/statd/statd.h | 1
12 files changed, 290 insertions(+), 455 deletions(-)
--
Chuck Lever
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 01/19] rpc.statd: refactor check to see if call is from loopback address
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 02/19] rpc.statd: eliminate --secure_statd Chuck Lever
` (18 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Refactor common logic to check if SM_FOO request is from loopback
address.
We'll have to do something about this for IPv6. On IPv6-capable
systems, there will be only one AF_INET6 listener. The loopback caller
will get either an IPv6 loopback address, or a mapped IPv4 loopback --
either way this will be an AF_INET6 address.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Acked-by: Neil Brown <neilb@suse.de>
---
utils/statd/monitor.c | 82 ++++++++++++++++++++++---------------------------
1 files changed, 36 insertions(+), 46 deletions(-)
diff --git a/utils/statd/monitor.c b/utils/statd/monitor.c
index eadc434..5d4aa49 100644
--- a/utils/statd/monitor.c
+++ b/utils/statd/monitor.c
@@ -29,6 +29,36 @@ notify_list * rtnl = NULL; /* Run-time notify list. */
#define LINELEN (4*(8+1)+SM_PRIV_SIZE*2+1)
+#ifdef RESTRICTED_STATD
+/*
+ * Reject requests from non-loopback addresses in order
+ * to prevent attack described in CERT CA-99.05.
+ */
+static int
+caller_is_localhost(struct svc_req *rqstp)
+{
+ struct in_addr caller;
+
+ caller = svc_getcaller(rqstp->rq_xprt)->sin_addr;
+ if (caller.s_addr != htonl(INADDR_LOOPBACK)) {
+ note(N_WARNING,
+ "Call to statd from non-local host %s",
+ inet_ntoa(caller));
+ return 0;
+ }
+ return 1;
+}
+#else /* RESTRICTED_STATD */
+/*
+ * No restrictions for remote callers.
+ */
+static int
+caller_is_localhost(struct svc_req *rqstp)
+{
+ return 1;
+}
+#endif /* RESTRICTED_STATD */
+
/*
* Services SM_MON requests.
*/
@@ -45,31 +75,19 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
notify_list *clnt;
struct in_addr my_addr;
char *dnsname;
-#ifdef RESTRICTED_STATD
- struct in_addr caller;
-#endif
struct hostent *hostinfo = NULL;
/* Assume that we'll fail. */
result.res_stat = STAT_FAIL;
result.state = -1; /* State is undefined for STAT_FAIL. */
- /* Restrict access to statd.
- * In the light of CERT CA-99.05, we tighten access to
- * statd. --okir
- */
#ifdef RESTRICTED_STATD
- /* 1. Reject anyone not calling from 127.0.0.1.
+ /* 1. Reject any remote callers.
* Ignore the my_name specified by the caller, and
* use "127.0.0.1" instead.
*/
- caller = svc_getcaller(rqstp->rq_xprt)->sin_addr;
- if (caller.s_addr != htonl(INADDR_LOOPBACK)) {
- note(N_WARNING,
- "Call to statd from non-local host %s",
- inet_ntoa(caller));
+ if (!caller_is_localhost(rqstp))
goto failure;
- }
my_addr.s_addr = htonl(INADDR_LOOPBACK);
/* 2. Reject any registrations for non-lockd services.
@@ -329,25 +347,12 @@ sm_unmon_1_svc(struct mon_id *argp, struct svc_req *rqstp)
*my_name = argp->my_id.my_name;
struct my_id *id = &argp->my_id;
char *cp;
-#ifdef RESTRICTED_STATD
- struct in_addr caller;
-#endif
result.state = MY_STATE;
-#ifdef RESTRICTED_STATD
- /* 1. Reject anyone not calling from 127.0.0.1.
- * Ignore the my_name specified by the caller, and
- * use "127.0.0.1" instead.
- */
- caller = svc_getcaller(rqstp->rq_xprt)->sin_addr;
- if (caller.s_addr != htonl(INADDR_LOOPBACK)) {
- note(N_WARNING,
- "Call to statd from non-local host %s",
- inet_ntoa(caller));
+ if (!caller_is_localhost(rqstp))
goto failure;
- }
-#endif
+
/* my_name must not have white space */
for (cp=my_name ; *cp ; cp++)
if (*cp == ' ' || *cp == '\t' || *cp == '\r' || *cp == '\n')
@@ -388,9 +393,7 @@ sm_unmon_1_svc(struct mon_id *argp, struct svc_req *rqstp)
clnt = NL_NEXT(clnt);
}
-#ifdef RESTRICTED_STATD
failure:
-#endif
note(N_WARNING, "Received erroneous SM_UNMON request from %s for %s",
my_name, mon_name);
return (&result);
@@ -404,21 +407,9 @@ sm_unmon_all_1_svc(struct my_id *argp, struct svc_req *rqstp)
static sm_stat result;
notify_list *clnt;
char *my_name = argp->my_name;
-#ifdef RESTRICTED_STATD
- struct in_addr caller;
- /* 1. Reject anyone not calling from 127.0.0.1.
- * Ignore the my_name specified by the caller, and
- * use "127.0.0.1" instead.
- */
- caller = svc_getcaller(rqstp->rq_xprt)->sin_addr;
- if (caller.s_addr != htonl(INADDR_LOOPBACK)) {
- note(N_WARNING,
- "Call to statd from non-local host %s",
- inet_ntoa(caller));
+ if (!caller_is_localhost(rqstp))
goto failure;
- }
-#endif
result.state = MY_STATE;
@@ -457,8 +448,7 @@ sm_unmon_all_1_svc(struct my_id *argp, struct svc_req *rqstp)
dprintf(N_DEBUG, "SM_UNMON_ALL request from %s with no "
"SM_MON requests from it.", my_name);
}
-#ifdef RESTRICTED_STATD
+
failure:
-#endif
return (&result);
}
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 02/19] rpc.statd: eliminate --secure_statd
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
2008-09-05 21:07 ` [PATCH 01/19] rpc.statd: refactor check to see if call is from loopback address Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 03/19] showmount: destroy RPC client when finished Chuck Lever
` (17 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: Remove RESTRICTED_STATD to help make IPv6 changes simpler.
We keep the code behind RESTRICTED_STATD, and toss anything that is
compiled out when it is set.
RESTRICTED_STATD was added almost 10 years ago in response to CERT
CERT CA-99.05, which addresses exposures in rpc.statd that might allow
an attacker to take advantage of buffer overflows in rpc.statd while it
is running in privileged mode.
These days, I can't think of a reason why anyone would want to run
rpc.statd without setting RESTRICTED_STATD. In addition, I don't
think rpc.statd is ever tested without it.
Removing RESTRICTED_STATD will get rid of some address storage and
comparison issues that will make IPv6 support simpler. Plus it will
make our test matrix smaller!
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Acked-by: Neil Brown <neilb@suse.de>
---
configure.ac | 9 ---------
utils/statd/monitor.c | 34 ----------------------------------
utils/statd/simu.c | 35 +++++++++++++----------------------
3 files changed, 13 insertions(+), 65 deletions(-)
diff --git a/configure.ac b/configure.ac
index 6ae6c6d..1ab89db 100644
--- a/configure.ac
+++ b/configure.ac
@@ -95,15 +95,6 @@ AC_ARG_ENABLE(kprefix,
test "$enableval" = "yes" && kprefix=k,
kprefix=)
AC_SUBST(kprefix)
-AC_ARG_ENABLE(secure-statd,
- [AC_HELP_STRING([--enable-secure-statd],
- [Only lockd can use statd (security)])],
- test "$enableval" = "yes" && secure_statd=yes,
- secure_statd=yes)
- if test "$secure_statd" = yes; then
- AC_DEFINE(RESTRICTED_STATD, 1, [Define this if you want to enable various security checks in statd. These checks basically keep anyone but lockd from using this service.])
- fi
- AC_SUBST(secure_statd)
AC_ARG_WITH(rpcgen,
[AC_HELP_STRING([--with-rpcgen=internal], [use internal rpcgen instead of system one])],
rpcgen_path=$withval,
diff --git a/utils/statd/monitor.c b/utils/statd/monitor.c
index 5d4aa49..d300338 100644
--- a/utils/statd/monitor.c
+++ b/utils/statd/monitor.c
@@ -29,7 +29,6 @@ notify_list * rtnl = NULL; /* Run-time notify list. */
#define LINELEN (4*(8+1)+SM_PRIV_SIZE*2+1)
-#ifdef RESTRICTED_STATD
/*
* Reject requests from non-loopback addresses in order
* to prevent attack described in CERT CA-99.05.
@@ -48,16 +47,6 @@ caller_is_localhost(struct svc_req *rqstp)
}
return 1;
}
-#else /* RESTRICTED_STATD */
-/*
- * No restrictions for remote callers.
- */
-static int
-caller_is_localhost(struct svc_req *rqstp)
-{
- return 1;
-}
-#endif /* RESTRICTED_STATD */
/*
* Services SM_MON requests.
@@ -81,7 +70,6 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
result.res_stat = STAT_FAIL;
result.state = -1; /* State is undefined for STAT_FAIL. */
-#ifdef RESTRICTED_STATD
/* 1. Reject any remote callers.
* Ignore the my_name specified by the caller, and
* use "127.0.0.1" instead.
@@ -107,28 +95,6 @@ sm_mon_1_svc(struct mon *argp, struct svc_req *rqstp)
goto failure;
}
-#if 0
- This is not usable anymore. Linux-kernel can be configured to use
- host names with NSM so that multi-homed hosts are handled properly.
- NeilBrown 15mar2007
-
- /* 3. mon_name must be an address in dotted quad.
- * Again, specific to the linux kernel lockd.
- */
- if (!inet_aton(mon_name, &mon_addr)) {
- note(N_WARNING,
- "Attempt to register host %s (not a dotted quad)",
- mon_name);
- goto failure;
- }
-#endif
-#else
- if (!(hostinfo = gethostbyname(my_name))) {
- note(N_WARNING, "gethostbyname error for %s", my_name);
- goto failure;
- } else
- my_addr = *(struct in_addr *) hostinfo->h_addr;
-#endif
/*
* Check hostnames. If I can't look them up, I won't monitor. This
* might not be legal, but it adds a little bit of safety and sanity.
diff --git a/utils/statd/simu.c b/utils/statd/simu.c
index 82d794e..25e8bad 100644
--- a/utils/statd/simu.c
+++ b/utils/statd/simu.c
@@ -22,35 +22,26 @@ void *
sm_simu_crash_1_svc (void *argp, struct svc_req *rqstp)
{
static char *result = NULL;
+ struct in_addr caller;
+
+ caller = svc_getcaller(rqstp->rq_xprt)->sin_addr;
+ if (caller.s_addr != htonl(INADDR_LOOPBACK)) {
+ note(N_WARNING, "Call to statd from non-local host %s",
+ inet_ntoa(caller));
+ goto failure;
+ }
+
+ if (ntohs(svc_getcaller(rqstp->rq_xprt)->sin_port) >= 1024) {
+ note(N_WARNING, "Call to statd-simu-crash from unprivileged port");
+ goto failure;
+ }
-#ifdef RESTRICTED_STATD
- struct in_addr caller;
-
- /* 1. Reject anyone not calling from 127.0.0.1.
- * Ignore the my_name specified by the caller, and
- * use "127.0.0.1" instead.
- */
- caller = svc_getcaller(rqstp->rq_xprt)->sin_addr;
- if (caller.s_addr != htonl(INADDR_LOOPBACK)) {
- note(N_WARNING,
- "Call to statd from non-local host %s",
- inet_ntoa(caller));
- goto failure;
- }
- if (ntohs(svc_getcaller(rqstp->rq_xprt)->sin_port) >= 1024) {
- note(N_WARNING,
- "Call to statd-simu-crash from unprivileged port\n");
- goto failure;
- }
-#endif
note (N_WARNING, "*** SIMULATING CRASH! ***");
my_svc_exit ();
if (rtnl)
nlist_kill (&rtnl);
-#ifdef RESTRICTED_STATD
failure:
-#endif
return ((void *)&result);
}
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 03/19] showmount: destroy RPC client when finished
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
2008-09-05 21:07 ` [PATCH 01/19] rpc.statd: refactor check to see if call is from loopback address Chuck Lever
2008-09-05 21:07 ` [PATCH 02/19] rpc.statd: eliminate --secure_statd Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 04/19] showmount command: clean up error returns from connect_nb() Chuck Lever
` (16 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: call clnt_destroy() in the showmount command as needed to
destroy the RPC client properly (and close the associated socket) before
the program exits.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/showmount/showmount.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
index 213a573..b6cea92 100644
--- a/utils/showmount/showmount.c
+++ b/utils/showmount/showmount.c
@@ -393,6 +393,7 @@ int main(int argc, char **argv)
total_timeout);
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount export");
+ clnt_destroy(mclient);
exit(1);
}
if (headers)
@@ -416,6 +417,7 @@ int main(int argc, char **argv)
printf("\n");
exportlist = exportlist->ex_next;
}
+ clnt_destroy(mclient);
exit(0);
}
@@ -426,8 +428,10 @@ int main(int argc, char **argv)
total_timeout);
if (clnt_stat != RPC_SUCCESS) {
clnt_perror(mclient, "rpc mount dump");
+ clnt_destroy(mclient);
exit(1);
}
+ clnt_destroy(mclient);
n = 0;
for (list = dumplist; list; list = list->ml_next)
@@ -481,4 +485,3 @@ int main(int argc, char **argv)
}
exit(0);
}
-
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 04/19] showmount command: clean up error returns from connect_nb()
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (2 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 03/19] showmount: destroy RPC client when finished Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 05/19] sm-notify command: include <config.h> Chuck Lever
` (15 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up connect_nb() in the showmount command.
Sometimes it returns -1 on error, and sometimes a negative errno. On error,
it should always return one of these or the other, not both.
Similar functions in other parts of nfs-utils return -1 on error, and set
errno; so let's do that here too.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/showmount/showmount.c | 22 ++++++++++++----------
1 files changed, 12 insertions(+), 10 deletions(-)
diff --git a/utils/showmount/showmount.c b/utils/showmount/showmount.c
index b6cea92..eafcf4c 100644
--- a/utils/showmount/showmount.c
+++ b/utils/showmount/showmount.c
@@ -83,7 +83,8 @@ static void usage(FILE *fp, int n)
* tout contains the timeout. It will be modified to contain the time
* remaining (i.e. time provided - time elasped).
*
- * Returns 0 for success
+ * Returns zero on success; otherwise, -1 is returned and errno is set
+ * to reflect the nature of the error.
*/
static int connect_nb(int fd, struct sockaddr_in *addr, struct timeval *tout)
{
@@ -107,7 +108,7 @@ static int connect_nb(int fd, struct sockaddr_in *addr, struct timeval *tout)
len = sizeof(struct sockaddr);
ret = connect(fd, (struct sockaddr *)addr, len);
if (ret < 0 && errno != EINPROGRESS) {
- ret = -errno;
+ ret = -1;
goto done;
}
@@ -121,9 +122,8 @@ static int connect_nb(int fd, struct sockaddr_in *addr, struct timeval *tout)
ret = select(fd + 1, NULL, &rset, NULL, tout);
if (ret <= 0) {
if (ret == 0)
- ret = -ETIMEDOUT;
- else
- ret = -errno;
+ errno = ETIMEDOUT;
+ ret = -1;
goto done;
}
@@ -133,13 +133,15 @@ static int connect_nb(int fd, struct sockaddr_in *addr, struct timeval *tout)
len = sizeof(ret);
status = getsockopt(fd, SOL_SOCKET, SO_ERROR, &ret, &len);
if (status < 0) {
- ret = -errno;
+ ret = -1;
goto done;
}
/* Oops - something wrong with connect */
- if (ret)
- ret = -ret;
+ if (ret != 0) {
+ errno = ret;
+ ret = -1;
+ }
}
done:
@@ -179,10 +181,10 @@ static unsigned short getport(struct sockaddr_in *addr,
tout.tv_sec = TIMEOUT_TCP;
ret = connect_nb(sock, &saddr, &tout);
- if (ret < 0) {
- close(sock);
+ if (ret != 0) {
rpc_createerr.cf_stat = RPC_SYSTEMERROR;
rpc_createerr.cf_error.re_errno = errno;
+ close(sock);
return 0;
}
client = clnttcp_create(&saddr,
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 05/19] sm-notify command: include <config.h>
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (3 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 04/19] showmount command: clean up error returns from connect_nb() Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 06/19] sm-notify command: getaddrinfo(3) addrinfo leak Chuck Lever
` (14 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: Include config.h as other source files do; instead of using
"config.h" use the HAVE_CONFIG_H macro and include <config.h>.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/sm-notify.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index b75f11b..5a02394 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -4,6 +4,10 @@
* Copyright (C) 2004-2006 Olaf Kirch <okir@suse.de>
*/
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/stat.h>
@@ -24,8 +28,6 @@
#include <errno.h>
#include <grp.h>
-#include "config.h"
-
#ifndef BASEDIR
# ifdef NFS_STATEDIR
# define BASEDIR NFS_STATEDIR
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 06/19] sm-notify command: getaddrinfo(3) addrinfo leak
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (4 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 05/19] sm-notify command: include <config.h> Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 07/19] sm-notify command: clean up error logging Chuck Lever
` (13 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Make sure the results of getaddrinfo(3) are properly freed in notify().
Note this is a one-time addrinfo allocation that would be automatically
freed when sm-notify exits anyway, so this is more of a nit than a real
bug fix.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/sm-notify.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 5a02394..b69f4cc 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -241,8 +241,11 @@ notify(void)
opt_srcaddr);
exit(1);
}
- memcpy(&local_addr, ai->ai_addr, ai->ai_addrlen);
+
/* We know it's IPv4 at this point */
+ memcpy(&local_addr, ai->ai_addr, ai->ai_addrlen);
+
+ freeaddrinfo(ai);
}
/* Use source port if provided on the command line,
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 07/19] sm-notify command: clean up error logging
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (5 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 06/19] sm-notify command: getaddrinfo(3) addrinfo leak Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 08/19] sm-notify command: replace nsm_address typedef Chuck Lever
` (12 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up a few issues with logging in sm-notify.c.
Sometimes in sm-notify, when a system call fails the problem is reported
to stderr but not logged, and then usually sm-notify exits. In cases like
this, there are probably more hosts to notify, but sm-notify dies silently.
Make sure these errors are logged, and that the log messages explain the
nature of the problem.
Also, if sm-notify exits prematurely, make sure this is always reported at
the LOG_ERR level, not at the LOG_WARNING level.
Remove a couple of unnecessary '\n' in the arguments of nsm_log() calls --
nsm_log() already appends an '\n' to the message.
Finally, use exit() consistently in main().
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/sm-notify.c | 48 ++++++++++++++++++++++++++++-------------------
1 files changed, 29 insertions(+), 19 deletions(-)
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index b69f4cc..e5ca904 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -134,7 +134,7 @@ main(int argc, char **argv)
_SM_STATE_PATH == NULL ||
_SM_DIR_PATH == NULL ||
_SM_BAK_PATH == NULL) {
- nsm_log(LOG_WARNING, "unable to allocate memory");
+ nsm_log(LOG_ERR, "unable to allocate memory");
exit(1);
}
strcat(strcpy(_SM_STATE_PATH, _SM_BASE_PATH), "/state");
@@ -151,7 +151,7 @@ main(int argc, char **argv)
usage: fprintf(stderr,
"Usage: sm-notify [-dfq] [-m max-retry-minutes] [-p srcport]\n"
" [-P /path/to/state/directory] [-v my_host_name]\n");
- return 1;
+ exit(1);
}
if (strcmp(_SM_BASE_PATH, BASEDIR) == 0) {
@@ -164,8 +164,9 @@ usage: fprintf(stderr,
strncpy(nsm_hostname, opt_srcaddr, sizeof(nsm_hostname)-1);
} else
if (gethostname(nsm_hostname, sizeof(nsm_hostname)) < 0) {
- perror("gethostname");
- return 1;
+ nsm_log(LOG_ERR, "Failed to obtain name of local host: %s",
+ strerror(errno));
+ exit(1);
}
backup_hosts(_SM_DIR_PATH, _SM_BAK_PATH);
@@ -183,9 +184,9 @@ usage: fprintf(stderr,
log_syslog = 1;
if (daemon(0, 0) < 0) {
- nsm_log(LOG_WARNING, "unable to background: %s",
+ nsm_log(LOG_ERR, "unable to background: %s",
strerror(errno));
- return 1;
+ exit(1);
}
close(0);
@@ -204,10 +205,10 @@ usage: fprintf(stderr,
"Unable to notify %s, giving up",
hp->name);
}
- return 1;
+ exit(1);
}
- return 0;
+ exit(0);
}
/*
@@ -224,7 +225,8 @@ notify(void)
retry:
sock = socket(AF_INET, SOCK_DGRAM, 0);
if (sock < 0) {
- perror("socket");
+ nsm_log(LOG_ERR, "Failed to create RPC socket: %s",
+ strerror(errno));
exit(1);
}
fcntl(sock, F_SETFL, O_NONBLOCK);
@@ -236,8 +238,8 @@ notify(void)
if (opt_srcaddr) {
struct addrinfo *ai = host_lookup(AF_INET, opt_srcaddr);
if (!ai) {
- nsm_log(LOG_WARNING,
- "Not a valid hostname or address: \"%s\"\n",
+ nsm_log(LOG_ERR,
+ "Not a valid hostname or address: \"%s\"",
opt_srcaddr);
exit(1);
}
@@ -253,7 +255,8 @@ notify(void)
if (opt_srcport) {
addr_set_port(&local_addr, opt_srcport);
if (bind(sock, (struct sockaddr *) &local_addr, sizeof(local_addr)) < 0) {
- perror("bind");
+ nsm_log(LOG_ERR, "Failed to bind RPC socket: %s",
+ strerror(errno));
exit(1);
}
} else {
@@ -492,7 +495,8 @@ recv_reply(int sock)
* packet)
*/
if (p <= end) {
- nsm_log(LOG_DEBUG, "Host %s notified successfully", hp->name);
+ nsm_log(LOG_DEBUG, "Host %s notified successfully",
+ hp->name);
unlink(hp->path);
free(hp->name);
free(hp->path);
@@ -516,7 +520,8 @@ backup_hosts(const char *dirname, const char *bakname)
DIR *dir;
if (!(dir = opendir(dirname))) {
- perror(dirname);
+ nsm_log(LOG_WARNING,
+ "Failed to open %s: %s", dirname, strerror(errno));
return;
}
@@ -548,7 +553,8 @@ get_hosts(const char *dirname)
DIR *dir;
if (!(dir = opendir(dirname))) {
- perror(dirname);
+ nsm_log(LOG_WARNING,
+ "Failed to open %s: %s", dirname, strerror(errno));
return;
}
@@ -561,6 +567,10 @@ get_hosts(const char *dirname)
continue;
if (host == NULL)
host = calloc(1, sizeof(*host));
+ if (host == NULL) {
+ nsm_log(LOG_WARNING, "Unable to allocate memory");
+ return;
+ }
snprintf(path, sizeof(path), "%s/%s", dirname, de->d_name);
if (stat(path, &stb) < 0)
@@ -665,17 +675,17 @@ nsm_get_state(int update)
snprintf(newfile, sizeof(newfile),
"%s.new", _SM_STATE_PATH);
if ((fd = open(newfile, O_CREAT|O_WRONLY, 0644)) < 0) {
- nsm_log(LOG_WARNING, "Cannot create %s: %m", newfile);
+ nsm_log(LOG_ERR, "Cannot create %s: %m", newfile);
exit(1);
}
if (write(fd, &state, sizeof(state)) != sizeof(state)) {
- nsm_log(LOG_WARNING,
+ nsm_log(LOG_ERR,
"Failed to write state to %s", newfile);
exit(1);
}
close(fd);
if (rename(newfile, _SM_STATE_PATH) < 0) {
- nsm_log(LOG_WARNING,
+ nsm_log(LOG_ERR,
"Cannot create %s: %m", _SM_STATE_PATH);
exit(1);
}
@@ -784,7 +794,7 @@ static void drop_privs(void)
if (st.st_uid == 0) {
nsm_log(LOG_WARNING,
- "sm-notify running as root. chown %s to choose different user\n",
+ "sm-notify running as root. chown %s to choose different user",
_SM_DIR_PATH);
return;
}
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 08/19] sm-notify command: replace nsm_address typedef
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (6 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 07/19] sm-notify command: clean up error logging Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 09/19] sm-notify command: use static function definitions Chuck Lever
` (11 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: replace "typedef struct sockaddr_storage nsm_address" with
standard socket address types. This makes sm-notify.c consistent with other
parts of nfs-utils, and with typical network application coding conventions.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/sm-notify.c | 129 +++++++++++++++++++++++------------------------
1 files changed, 63 insertions(+), 66 deletions(-)
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index e5ca904..8108765 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -53,13 +53,11 @@ char *_SM_BAK_PATH = DEFAULT_SM_BAK_PATH;
#define NSM_MAX_TIMEOUT 120 /* don't make this too big */
#define MAXMSGSIZE 256
-typedef struct sockaddr_storage nsm_address;
-
struct nsm_host {
struct nsm_host * next;
char * name;
char * path;
- nsm_address addr;
+ struct sockaddr_storage addr;
struct addrinfo *ai;
time_t last_used;
time_t send_next;
@@ -86,9 +84,6 @@ static void backup_hosts(const char *, const char *);
static void get_hosts(const char *);
static void insert_host(struct nsm_host *);
struct nsm_host * find_host(uint32_t);
-static int addr_get_port(nsm_address *);
-static void addr_set_port(nsm_address *, int);
-static struct addrinfo *host_lookup(int, const char *);
void nsm_log(int fac, const char *fmt, ...);
static int record_pid(void);
static void drop_privs(void);
@@ -96,6 +91,46 @@ static void set_kernel_nsm_state(int state);
static struct nsm_host * hosts = NULL;
+/*
+ * Address handling utilities
+ */
+
+static unsigned short smn_get_port(const struct sockaddr *sap)
+{
+ switch (sap->sa_family) {
+ case AF_INET:
+ return ntohs(((struct sockaddr_in *)sap)->sin_port);
+ case AF_INET6:
+ return ntohs(((struct sockaddr_in6 *)sap)->sin6_port);
+ }
+ return 0;
+}
+
+static void smn_set_port(struct sockaddr *sap, const unsigned short port)
+{
+ switch (sap->sa_family) {
+ case AF_INET:
+ ((struct sockaddr_in *)sap)->sin_port = htons(port);
+ break;
+ case AF_INET6:
+ ((struct sockaddr_in6 *)sap)->sin6_port = htons(port);
+ break;
+ }
+}
+
+static struct addrinfo *smn_lookup(const sa_family_t family, const char *name)
+{
+ struct addrinfo *ai, hint = {
+ .ai_family = family,
+ .ai_protocol = IPPROTO_UDP,
+ };
+
+ if (getaddrinfo(name, NULL, &hint, &ai) != 0)
+ return NULL;
+
+ return ai;
+}
+
int
main(int argc, char **argv)
{
@@ -217,7 +252,8 @@ usage: fprintf(stderr,
void
notify(void)
{
- nsm_address local_addr;
+ struct sockaddr_storage address;
+ struct sockaddr *local_addr = (struct sockaddr *)&address;
time_t failtime = 0;
int sock = -1;
int retry_cnt = 0;
@@ -231,12 +267,12 @@ notify(void)
}
fcntl(sock, F_SETFL, O_NONBLOCK);
- memset(&local_addr, 0, sizeof(local_addr));
- local_addr.ss_family = AF_INET; /* Default to IPv4 */
+ memset(&address, 0, sizeof(address));
+ local_addr->sa_family = AF_INET; /* Default to IPv4 */
/* Bind source IP if provided on command line */
if (opt_srcaddr) {
- struct addrinfo *ai = host_lookup(AF_INET, opt_srcaddr);
+ struct addrinfo *ai = smn_lookup(AF_INET, opt_srcaddr);
if (!ai) {
nsm_log(LOG_ERR,
"Not a valid hostname or address: \"%s\"",
@@ -245,7 +281,7 @@ notify(void)
}
/* We know it's IPv4 at this point */
- memcpy(&local_addr, ai->ai_addr, ai->ai_addrlen);
+ memcpy(local_addr, ai->ai_addr, ai->ai_addrlen);
freeaddrinfo(ai);
}
@@ -253,15 +289,15 @@ notify(void)
/* Use source port if provided on the command line,
* otherwise use bindresvport */
if (opt_srcport) {
- addr_set_port(&local_addr, opt_srcport);
- if (bind(sock, (struct sockaddr *) &local_addr, sizeof(local_addr)) < 0) {
+ smn_set_port(local_addr, opt_srcport);
+ if (bind(sock, local_addr, sizeof(struct sockaddr_in)) < 0) {
nsm_log(LOG_ERR, "Failed to bind RPC socket: %s",
strerror(errno));
exit(1);
}
} else {
struct servent *se;
- struct sockaddr_in *sin = (struct sockaddr_in *)&local_addr;
+ struct sockaddr_in *sin = (struct sockaddr_in *)local_addr;
(void) bindresvport(sock, sin);
/* try to avoid known ports */
se = getservbyport(sin->sin_port, "udp");
@@ -339,8 +375,10 @@ notify(void)
int
notify_host(int sock, struct nsm_host *host)
{
+ struct sockaddr_storage address;
+ struct sockaddr *dest = (struct sockaddr *)&address;
+ socklen_t destlen = sizeof(address);
static unsigned int xid = 0;
- nsm_address dest;
uint32_t msgbuf[MAXMSGSIZE], *p;
unsigned int len;
@@ -350,7 +388,7 @@ notify_host(int sock, struct nsm_host *host)
host->xid = xid++;
if (host->ai == NULL) {
- host->ai = host_lookup(AF_UNSPEC, host->name);
+ host->ai = smn_lookup(AF_UNSPEC, host->name);
if (host->ai == NULL) {
nsm_log(LOG_WARNING,
"%s doesn't seem to be a valid address,"
@@ -384,16 +422,16 @@ notify_host(int sock, struct nsm_host *host)
/* put first entry at end */
*next = first;
memcpy(&host->addr, first->ai_addr, first->ai_addrlen);
- addr_set_port(&host->addr, 0);
+ smn_set_port((struct sockaddr *)&host->addr, 0);
host->retries = 0;
}
- dest = host->addr;
- if (addr_get_port(&dest) == 0) {
+ memcpy(dest, &host->addr, destlen);
+ if (smn_get_port(dest) == 0) {
/* Build a PMAP packet */
nsm_log(LOG_DEBUG, "Sending portmap query to %s", host->name);
- addr_set_port(&dest, 111);
+ smn_set_port(dest, 111);
*p++ = htonl(100000);
*p++ = htonl(2);
*p++ = htonl(3);
@@ -427,7 +465,7 @@ notify_host(int sock, struct nsm_host *host)
}
len = (p - msgbuf) << 2;
- if (sendto(sock, msgbuf, len, 0, (struct sockaddr *) &dest, sizeof(dest)) < 0)
+ if (sendto(sock, msgbuf, len, 0, dest, destlen) < 0)
nsm_log(LOG_WARNING, "Sending Reboot Notification to "
"'%s' failed: errno %d (%s)", host->name, errno, strerror(errno));
@@ -441,6 +479,7 @@ void
recv_reply(int sock)
{
struct nsm_host *hp;
+ struct sockaddr *sap;
uint32_t msgbuf[MAXMSGSIZE], *p, *end;
uint32_t xid;
int res;
@@ -466,8 +505,9 @@ recv_reply(int sock)
this reply */
if ((hp = find_host(xid)) == NULL)
return;
+ sap = (struct sockaddr *)&hp->addr;
- if (addr_get_port(&hp->addr) == 0) {
+ if (smn_get_port(sap) == 0) {
/* This was a portmap request */
unsigned int port;
@@ -483,7 +523,7 @@ recv_reply(int sock)
hp->timeout = NSM_MAX_TIMEOUT;
hp->send_next += NSM_MAX_TIMEOUT;
} else {
- addr_set_port(&hp->addr, port);
+ smn_set_port(sap, port);
if (hp->timeout >= NSM_MAX_TIMEOUT / 4)
hp->timeout = NSM_MAX_TIMEOUT / 4;
}
@@ -696,49 +736,6 @@ nsm_get_state(int update)
}
/*
- * Address handling utilities
- */
-
-int
-addr_get_port(nsm_address *addr)
-{
- switch (((struct sockaddr *) addr)->sa_family) {
- case AF_INET:
- return ntohs(((struct sockaddr_in *) addr)->sin_port);
- case AF_INET6:
- return ntohs(((struct sockaddr_in6 *) addr)->sin6_port);
- }
- return 0;
-}
-
-static void
-addr_set_port(nsm_address *addr, int port)
-{
- switch (((struct sockaddr *) addr)->sa_family) {
- case AF_INET:
- ((struct sockaddr_in *) addr)->sin_port = htons(port);
- break;
- case AF_INET6:
- ((struct sockaddr_in6 *) addr)->sin6_port = htons(port);
- }
-}
-
-static struct addrinfo *
-host_lookup(int af, const char *name)
-{
- struct addrinfo hints, *ai;
-
- memset(&hints, 0, sizeof(hints));
- hints.ai_family = af;
- hints.ai_protocol = IPPROTO_UDP;
-
- if (getaddrinfo(name, NULL, &hints, &ai) != 0)
- return NULL;
-
- return ai;
-}
-
-/*
* Log a message
*/
void
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 09/19] sm-notify command: use static function definitions
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (7 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 08/19] sm-notify command: replace nsm_address typedef Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 10/19] nfs-utils: remove unused function rpc_logcall() Chuck Lever
` (10 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up.
The sm-notify command is built from a single source file.
Some of its internal functions are appropriately defined as static.
However, some are declared static, but defined as global. Some are
declared and defined as global. None of them are used outside of
utils/statd/sm-notify.c.
Make all the internal functions in utils/statd/sm-notify.cstatic.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/sm-notify.c | 20 ++++++++++----------
1 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/utils/statd/sm-notify.c b/utils/statd/sm-notify.c
index 8108765..76a378c 100644
--- a/utils/statd/sm-notify.c
+++ b/utils/statd/sm-notify.c
@@ -83,11 +83,11 @@ static void recv_reply(int);
static void backup_hosts(const char *, const char *);
static void get_hosts(const char *);
static void insert_host(struct nsm_host *);
-struct nsm_host * find_host(uint32_t);
-void nsm_log(int fac, const char *fmt, ...);
+static struct nsm_host *find_host(uint32_t);
+static void nsm_log(int fac, const char *fmt, ...);
static int record_pid(void);
static void drop_privs(void);
-static void set_kernel_nsm_state(int state);
+static void set_kernel_nsm_state(int state);
static struct nsm_host * hosts = NULL;
@@ -249,7 +249,7 @@ usage: fprintf(stderr,
/*
* Notify hosts
*/
-void
+static void
notify(void)
{
struct sockaddr_storage address;
@@ -372,7 +372,7 @@ notify(void)
/*
* Send notification to a single host
*/
-int
+static int
notify_host(int sock, struct nsm_host *host)
{
struct sockaddr_storage address;
@@ -475,7 +475,7 @@ notify_host(int sock, struct nsm_host *host)
/*
* Receive reply from remote host
*/
-void
+static void
recv_reply(int sock)
{
struct nsm_host *hp;
@@ -634,7 +634,7 @@ get_hosts(const char *dirname)
/*
* Insert host into sorted list
*/
-void
+static void
insert_host(struct nsm_host *host)
{
struct nsm_host **where, *p;
@@ -662,7 +662,7 @@ insert_host(struct nsm_host *host)
/*
* Find host given the XID
*/
-struct nsm_host *
+static struct nsm_host *
find_host(uint32_t xid)
{
struct nsm_host **where, *p;
@@ -682,7 +682,7 @@ find_host(uint32_t xid)
/*
* Retrieve the current NSM state
*/
-unsigned int
+static unsigned int
nsm_get_state(int update)
{
char newfile[PATH_MAX];
@@ -738,7 +738,7 @@ nsm_get_state(int update)
/*
* Log a message
*/
-void
+static void
nsm_log(int fac, const char *fmt, ...)
{
va_list ap;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 10/19] nfs-utils: remove unused function rpc_logcall()
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (8 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 09/19] sm-notify command: use static function definitions Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:07 ` [PATCH 11/19] nfs-utils: Remove unused function rpc_svcrun() Chuck Lever
` (9 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: Eliminate rpc_logcall(), which has no callers.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/include/rpcmisc.h | 1 -
support/nfs/rpcmisc.c | 60 ---------------------------------------------
2 files changed, 0 insertions(+), 61 deletions(-)
diff --git a/support/include/rpcmisc.h b/support/include/rpcmisc.h
index a2b2b5a..665df8e 100644
--- a/support/include/rpcmisc.h
+++ b/support/include/rpcmisc.h
@@ -50,7 +50,6 @@ void rpc_svcrun(void);
void rpc_dispatch(struct svc_req *rq, SVCXPRT *xprt,
struct rpc_dtable *dtable, int nvers,
void *argp, void *resp);
-void rpc_logcall(struct svc_req *, char *xname, char *args);
extern int _rpcpmstart;
extern int _rpcfdtype;
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index fcc6433..bd1a2db 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -215,63 +215,3 @@ int makesock(int port, int proto)
}
return (s);
}
-
-
-/* Log an incoming call. */
-void
-rpc_logcall(struct svc_req *rqstp, char *xname, char *arg)
-{
- char buff[1024];
- int buflen=sizeof(buff);
- int len;
- char *sp;
- int i;
-
- if (!xlog_enabled(D_CALL))
- return;
-
- sp = buff;
- switch (rqstp->rq_cred.oa_flavor) {
- case AUTH_NULL:
- sprintf(sp, "NULL");
- break;
- case AUTH_UNIX: {
- struct authunix_parms *unix_cred;
- time_t time;
- struct tm *tm;
-
- unix_cred = (struct authunix_parms *) rqstp->rq_clntcred;
- time = unix_cred->aup_time;
- tm = localtime(&time);
- snprintf(sp, buflen, "UNIX %d/%d/%d %02d:%02d:%02d %s %d.%d",
- tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
- tm->tm_hour, tm->tm_min, tm->tm_sec,
- unix_cred->aup_machname,
- unix_cred->aup_uid,
- unix_cred->aup_gid);
- sp[buflen-1] = 0;
- len = strlen(sp);
- sp += buflen;
- buflen -= len;
- if ((int) unix_cred->aup_len > 0) {
- snprintf(sp, buflen, "+%d", unix_cred->aup_gids[0]);
- sp[buflen-1] = 0;
- len = strlen(sp);
- sp += buflen;
- buflen -= len;
- for (i = 1; i < unix_cred->aup_len; i++) {
- snprintf(sp, buflen, ",%d",
- unix_cred->aup_gids[i]);
- sp[buflen-1] = 0;
- len = strlen(sp);
- sp += buflen;
- buflen -= len;
- }
- }
- }
- break;
- default:
- sprintf(sp, "CRED %d", rqstp->rq_cred.oa_flavor);
- }
- xlog(D_CALL, "%s [%s]\n\t%s\n", xname, buff, arg);
-}
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 11/19] nfs-utils: Remove unused function rpc_svcrun()
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (9 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 10/19] nfs-utils: remove unused function rpc_logcall() Chuck Lever
@ 2008-09-05 21:07 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 12/19] nfs-utils: remove disabled code from support/nfs/rpcmisc.c Chuck Lever
` (8 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:07 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: remove function that has been disabled (via #if 0) for almost a
decade.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/include/rpcmisc.h | 1 -
support/nfs/rpcdispatch.c | 51 ---------------------------------------------
2 files changed, 0 insertions(+), 52 deletions(-)
diff --git a/support/include/rpcmisc.h b/support/include/rpcmisc.h
index 665df8e..35c5011 100644
--- a/support/include/rpcmisc.h
+++ b/support/include/rpcmisc.h
@@ -46,7 +46,6 @@ int makesock(int port, int proto);
void rpc_init(char *name, int prog, int vers,
void (*dispatch)(struct svc_req *, SVCXPRT *),
int defport);
-void rpc_svcrun(void);
void rpc_dispatch(struct svc_req *rq, SVCXPRT *xprt,
struct rpc_dtable *dtable, int nvers,
void *argp, void *resp);
diff --git a/support/nfs/rpcdispatch.c b/support/nfs/rpcdispatch.c
index 3d34774..502fc5f 100644
--- a/support/nfs/rpcdispatch.c
+++ b/support/nfs/rpcdispatch.c
@@ -61,54 +61,3 @@ rpc_dispatch(struct svc_req *rqstp, SVCXPRT *transp,
exit (2);
}
}
-
-#if 0
-/*
- * This is our replacement for svc_run. It turns off some signals while
- * executing the server procedures to avoid nasty race conditions.
- */
-void
-rpc_svcrun(fd_set *morefds, void (*func)(int fd))
-{
- sigset_t block, current;
- fd_set readfds;
-
- for (;;) {
- readfds = svc_fdset;
- if (morefds) {
- int i;
-
- /* most efficient */
- for (i = 0; i < FD_SETSIZE; i++)
- if (FD_ISSET(i, morefds))
- FD_SET(i, &readfs);
- }
- switch (select(FD_SETSIZE, &readfds, NULL, NULL, NULL)) {
- case -1:
- if (errno == EINTR)
- continue;
- xlog(L_ERROR, "svc_run: - select failed");
- break;
- case 0:
- continue;
- default:
- if (morefds) {
- int i;
-
- /* most efficient */
- for (i = 0; i < FD_SETSIZE; i++)
- if (FD_ISSET(i, morefds) &&
- FD_ISSET(i, &readfds))
- func(i);
- }
- sigemptyset(&block);
- sigaddset(&block, SIGALRM);
- sigaddset(&block, SIGVTALRM);
- sigaddset(&block, SIGIO);
- sigprocmask(SIG_BLOCK, &block, ¤t);
- svc_getreqset(&readfds);
- sigprocmask(SIG_SETMASK, ¤t, NULL);
- }
- }
-}
-#endif
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 12/19] nfs-utils: remove disabled code from support/nfs/rpcmisc.c
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (10 preceding siblings ...)
2008-09-05 21:07 ` [PATCH 11/19] nfs-utils: Remove unused function rpc_svcrun() Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 13/19] nfs-utils: Clean up support/nfs/rpcmisc.c:closedown() Chuck Lever
` (7 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
After some recent discussions, we want to rely on the kernel's network
layer to autotune socket buffers. Since this code is already disabled in
support/nfs/rpcmisc.c (and has been for some time), let's just remove it.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/nfs/rpcmisc.c | 16 ----------------
1 files changed, 0 insertions(+), 16 deletions(-)
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index bd1a2db..40c2102 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -192,22 +192,6 @@ int makesock(int port, int proto)
xlog(L_ERROR, "setsockopt failed: %s\n",
strerror(errno));
-#if 0
- /* I was told it didn't work with gigabit ethernet.
- Don't bothet with it. H.J. */
-#ifdef SO_SNDBUF
- {
- int sblen, rblen;
-
- /* 1024 for rpc & transport overheads */
- sblen = rblen = socksz + 1024;
- if (setsockopt(s, SOL_SOCKET, SO_SNDBUF, &sblen, sizeof sblen) < 0 ||
- setsockopt(s, SOL_SOCKET, SO_RCVBUF, &rblen, sizeof rblen) < 0)
- xlog(L_ERROR, "setsockopt failed: %s\n", strerror(errno));
- }
-#endif /* SO_SNDBUF */
-#endif
-
if (bind(s, (struct sockaddr *) &sin, sizeof(sin)) == -1) {
xlog(L_FATAL, "Could not bind name to socket: %s\n",
strerror(errno));
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 13/19] nfs-utils: Clean up support/nfs/rpcmisc.c:closedown()
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (11 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 12/19] nfs-utils: remove disabled code from support/nfs/rpcmisc.c Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 14/19] nfs-utils: make makesock() static Chuck Lever
` (6 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: update closedown()'s synopsis to modern C style, and move the
function so we can remove the forward declaration.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/nfs/rpcmisc.c | 48 +++++++++++++++++++++++++-----------------------
1 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index 40c2102..ad53a43 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -38,7 +38,6 @@
#define socklen_t int
#endif
-static void closedown(int sig);
int makesock(int port, int proto);
#define _RPCSVC_CLOSEDOWN 120
@@ -46,6 +45,31 @@ int _rpcpmstart = 0;
int _rpcfdtype = 0;
int _rpcsvcdirty = 0;
+static void
+closedown(int sig)
+{
+ (void) signal(sig, closedown);
+
+ if (_rpcsvcdirty == 0) {
+ static int size;
+ int i, openfd;
+
+ if (_rpcfdtype == SOCK_DGRAM)
+ exit(0);
+
+ if (size == 0)
+ size = getdtablesize();
+
+ for (i = 0, openfd = 0; i < size && openfd < 2; i++)
+ if (FD_ISSET(i, &svc_fdset))
+ openfd++;
+ if (openfd <= 1)
+ exit(0);
+ }
+
+ (void) alarm(_RPCSVC_CLOSEDOWN);
+}
+
void
rpc_init(char *name, int prog, int vers,
void (*dispatch)(struct svc_req *, register SVCXPRT *),
@@ -144,28 +168,6 @@ rpc_init(char *name, int prog, int vers,
}
}
-static void closedown(sig)
-int sig;
-{
- (void) signal(sig, closedown);
- if (_rpcsvcdirty == 0) {
- static int size;
- int i, openfd;
-
- if (_rpcfdtype == SOCK_DGRAM)
- exit(0);
- if (size == 0) {
- size = getdtablesize();
- }
- for (i = 0, openfd = 0; i < size && openfd < 2; i++)
- if (FD_ISSET(i, &svc_fdset))
- openfd++;
- if (openfd <= 1)
- exit(0);
- }
- (void) alarm(_RPCSVC_CLOSEDOWN);
-}
-
int makesock(int port, int proto)
{
struct sockaddr_in sin;
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 14/19] nfs-utils: make makesock() static
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (12 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 13/19] nfs-utils: Clean up support/nfs/rpcmisc.c:closedown() Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 15/19] nfs-utils: Remove excess log reporting Chuck Lever
` (5 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: The makesock() function can become static since it is only used in
rpcmisc.c, where it is defined. Fix some minor nits while we're in the area:
o Move it so we can remove it's forward declaration.
o Get rid of unneeded newlines in the xlog() format strings.
o Use htonl(INADDR_ANY) instead of INADDR_ANY to initialize sin_addr.
Should make no run-time difference, but is slightly more proper,
as the standard definition of INADDR_ANY is in host byte-order.
o Remove the parentheses in the "return" statements.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/include/rpcmisc.h | 1 -
support/nfs/rpcmisc.c | 76 ++++++++++++++++++++++++---------------------
2 files changed, 40 insertions(+), 37 deletions(-)
diff --git a/support/include/rpcmisc.h b/support/include/rpcmisc.h
index 35c5011..5814a63 100644
--- a/support/include/rpcmisc.h
+++ b/support/include/rpcmisc.h
@@ -42,7 +42,6 @@ struct rpc_dtable {
}
-int makesock(int port, int proto);
void rpc_init(char *name, int prog, int vers,
void (*dispatch)(struct svc_req *, SVCXPRT *),
int defport);
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index ad53a43..9d76e5d 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -38,8 +38,6 @@
#define socklen_t int
#endif
-int makesock(int port, int proto);
-
#define _RPCSVC_CLOSEDOWN 120
int _rpcpmstart = 0;
int _rpcfdtype = 0;
@@ -70,6 +68,46 @@ closedown(int sig)
(void) alarm(_RPCSVC_CLOSEDOWN);
}
+/*
+ * Create listener socket for a given port
+ *
+ * Return an open network socket on success; otherwise return -1
+ * if some error occurs.
+ */
+static int
+makesock(int port, int proto)
+{
+ struct sockaddr_in sin;
+ int sock, sock_type, val;
+
+ sock_type = (proto == IPPROTO_UDP) ? SOCK_DGRAM : SOCK_STREAM;
+ sock = socket(AF_INET, sock_type, proto);
+ if (sock < 0) {
+ xlog(L_FATAL, "Could not make a socket: %s",
+ strerror(errno));
+ return -1;
+ }
+ memset((char *) &sin, 0, sizeof(sin));
+ sin.sin_family = AF_INET;
+ sin.sin_addr.s_addr = htonl(INADDR_ANY);
+ sin.sin_port = htons(port);
+
+ val = 1;
+ if (proto == IPPROTO_TCP)
+ if (setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
+ &val, sizeof(val)) < 0)
+ xlog(L_ERROR, "setsockopt failed: %s",
+ strerror(errno));
+
+ if (bind(sock, (struct sockaddr *) &sin, sizeof(sin)) == -1) {
+ xlog(L_FATAL, "Could not bind name to socket: %s",
+ strerror(errno));
+ return -1;
+ }
+
+ return sock;
+}
+
void
rpc_init(char *name, int prog, int vers,
void (*dispatch)(struct svc_req *, register SVCXPRT *),
@@ -167,37 +205,3 @@ rpc_init(char *name, int prog, int vers,
alarm (_RPCSVC_CLOSEDOWN);
}
}
-
-int makesock(int port, int proto)
-{
- struct sockaddr_in sin;
- int s;
- int sock_type;
- int val;
-
- sock_type = (proto == IPPROTO_UDP) ? SOCK_DGRAM : SOCK_STREAM;
- s = socket(AF_INET, sock_type, proto);
- if (s < 0) {
- xlog(L_FATAL, "Could not make a socket: %s\n",
- strerror(errno));
- return (-1);
- }
- memset((char *) &sin, 0, sizeof(sin));
- sin.sin_family = AF_INET;
- sin.sin_addr.s_addr = INADDR_ANY;
- sin.sin_port = htons(port);
-
- val = 1;
- if (proto == IPPROTO_TCP)
- if (setsockopt(s, SOL_SOCKET, SO_REUSEADDR,
- &val, sizeof(val)) < 0)
- xlog(L_ERROR, "setsockopt failed: %s\n",
- strerror(errno));
-
- if (bind(s, (struct sockaddr *) &sin, sizeof(sin)) == -1) {
- xlog(L_FATAL, "Could not bind name to socket: %s\n",
- strerror(errno));
- return (-1);
- }
- return (s);
-}
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 15/19] nfs-utils: Remove excess log reporting
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (13 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 14/19] nfs-utils: make makesock() static Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 16/19] nfs-utils: whitespace clean ups in support/nfs/rpcmisc.c Chuck Lever
` (4 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: The makesock() function already reports an error if it can't
create a socket. Remove the redundant error check and logging done in
rpc_init() after a makesock() call.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/nfs/rpcmisc.c | 12 ++++--------
1 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index 9d76e5d..506b595 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -151,10 +151,8 @@ rpc_init(char *name, int prog, int vers,
}
if (defport == 0)
sock = RPC_ANYSOCK;
- else if ((sock = makesock(defport, IPPROTO_UDP)) < 0) {
- xlog(L_FATAL, "%s: cannot make a UDP socket\n",
- name);
- }
+ else
+ sock = makesock(defport, IPPROTO_UDP);
}
if (sock == RPC_ANYSOCK)
sock = svcudp_socket (prog, 1);
@@ -181,10 +179,8 @@ rpc_init(char *name, int prog, int vers,
}
if (defport == 0)
sock = RPC_ANYSOCK;
- else if ((sock = makesock(defport, IPPROTO_TCP)) < 0) {
- xlog(L_FATAL, "%s: cannot make a TCP socket\n",
- name);
- }
+ else
+ sock = makesock(defport, IPPROTO_TCP);
}
if (sock == RPC_ANYSOCK)
sock = svctcp_socket (prog, 1);
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 16/19] nfs-utils: whitespace clean ups in support/nfs/rpcmisc.c
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (14 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 15/19] nfs-utils: Remove excess log reporting Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 17/19] rpc.statd: Clean up: replace "if (!(foo = rtnl))" Chuck Lever
` (3 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: fix a few spurious white space issues in support/nfs/rpcmisc.c.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
support/nfs/rpcmisc.c | 13 ++++++-------
1 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/support/nfs/rpcmisc.c b/support/nfs/rpcmisc.c
index 506b595..a0854e7 100644
--- a/support/nfs/rpcmisc.c
+++ b/support/nfs/rpcmisc.c
@@ -1,9 +1,8 @@
/*
- * support/nfs/rpcmisc.c
+ * Miscellaneous functions for RPC service startup and shutdown.
*
- * Miscellaneous functions for RPC startup and shutdown.
* This code is partially snarfed from rpcgen -s tcp -s udp,
- * partly written by Mark Shand, Donald Becker, and Rick
+ * partly written by Mark Shand, Donald Becker, and Rick
* Sladkey. It was tweaked slightly by Olaf Kirch to be
* usable by both unfsd and mountd.
*
@@ -122,7 +121,7 @@ rpc_init(char *name, int prog, int vers,
sock = 0;
if (getsockname(0, (struct sockaddr *) &saddr, &asize) == 0
&& saddr.sin_family == AF_INET) {
- socklen_t ssize = sizeof (int);
+ socklen_t ssize = sizeof(int);
int fdtype = 0;
if (getsockopt(0, SOL_SOCKET, SO_TYPE,
(char *)&fdtype, &ssize) == -1)
@@ -142,7 +141,7 @@ rpc_init(char *name, int prog, int vers,
if ((_rpcfdtype == 0) || (_rpcfdtype == SOCK_DGRAM)) {
static SVCXPRT *last_transp = NULL;
-
+
if (_rpcpmstart == 0) {
if (last_transp
&& (!defport || defport == last_transp->xp_port)) {
@@ -197,7 +196,7 @@ rpc_init(char *name, int prog, int vers,
}
if (_rpcpmstart) {
- signal (SIGALRM, closedown);
- alarm (_RPCSVC_CLOSEDOWN);
+ signal(SIGALRM, closedown);
+ alarm(_RPCSVC_CLOSEDOWN);
}
}
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 17/19] rpc.statd: Clean up: replace "if (!(foo = rtnl))".
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (15 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 16/19] nfs-utils: whitespace clean ups in support/nfs/rpcmisc.c Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 18/19] rpc.statd: Use __func__ in dprintf Chuck Lever
` (2 subsequent siblings)
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Static code checkers flag this kind of thing because it's easy to
confuse with "if (!(foo == rtnl))". In one of these cases, the
combination of evaluation and assignment isn't even necessary.
While we are in the neighborhood, remove an extra argument to note() that is
not called for in the passed-in format string.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/callback.c | 6 ++----
utils/statd/monitor.c | 6 ++++--
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/utils/statd/callback.c b/utils/statd/callback.c
index d5d036e..505fdb3 100644
--- a/utils/statd/callback.c
+++ b/utils/statd/callback.c
@@ -36,11 +36,9 @@ sm_notify_1_svc(struct stat_chge *argp, struct svc_req *rqstp)
argp->mon_name, argp->state);
/* quick check - don't bother if we're not monitoring anyone */
- /* LH - this was != MULL, meaning that if anyone _was_ in our RTNL,
- * we'd never pass this point. */
- if (!(lp = rtnl)) {
+ if (rtnl == NULL) {
note(N_WARNING, "SM_NOTIFY from %s while not monitoring any hosts.",
- argp->mon_name, argp->state);
+ argp->mon_name);
return ((void *) &result);
}
diff --git a/utils/statd/monitor.c b/utils/statd/monitor.c
index d300338..a6a1d9c 100644
--- a/utils/statd/monitor.c
+++ b/utils/statd/monitor.c
@@ -326,12 +326,13 @@ sm_unmon_1_svc(struct mon_id *argp, struct svc_req *rqstp)
/* Check if we're monitoring anyone. */
- if (!(clnt = rtnl)) {
+ if (rtnl == NULL) {
note(N_WARNING,
"Received SM_UNMON request from %s for %s while not "
"monitoring any hosts.", my_name, argp->mon_name);
return (&result);
}
+ clnt = rtnl;
/*
* OK, we are. Now look for appropriate entry in run-time list.
@@ -379,11 +380,12 @@ sm_unmon_all_1_svc(struct my_id *argp, struct svc_req *rqstp)
result.state = MY_STATE;
- if (!(clnt = rtnl)) {
+ if (rtnl == NULL) {
note(N_WARNING, "Received SM_UNMON_ALL request from %s "
"while not monitoring any hosts", my_name);
return (&result);
}
+ clnt = rtnl;
while ((clnt = nlist_gethost(clnt, my_name, 1))) {
if (NL_MY_PROC(clnt) == argp->my_proc &&
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 18/19] rpc.statd: Use __func__ in dprintf
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (16 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 17/19] rpc.statd: Clean up: replace "if (!(foo = rtnl))" Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-05 21:08 ` [PATCH 19/19] rpc.statd: Stop overloading sockfd in utils/statd/rmtcall.c Chuck Lever
2008-09-29 11:07 ` [PATCH 00/19] REPOST (with addn'l patches) nfs-utils cleanups Steve Dickson
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
Clean up: The named function in many of the debugging messages in
utils/statd/rmtcall.c is out of date. To prevent this from happening
in the future, replace these with __func__.
Also, note() and dprintf() do not require a terminating '\n' in their
format string. So make all invocations consistent.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/rmtcall.c | 50 ++++++++++++++++++++++++++-----------------------
1 files changed, 27 insertions(+), 23 deletions(-)
diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
index eb1919a..8240529 100644
--- a/utils/statd/rmtcall.c
+++ b/utils/statd/rmtcall.c
@@ -73,7 +73,7 @@ statd_get_socket(void)
if (sockfd >= 0) close(sockfd);
if ((sockfd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP)) < 0) {
- note(N_CRIT, "Can't create socket: %m");
+ note(N_CRIT, "%s: Can't create socket: %m", __func__);
return -1;
}
@@ -83,8 +83,8 @@ statd_get_socket(void)
sin.sin_addr.s_addr = INADDR_ANY;
if (bindresvport(sockfd, &sin) < 0) {
- dprintf(N_WARNING,
- "process_hosts: can't bind to reserved port\n");
+ dprintf(N_WARNING, "%s: can't bind to reserved port",
+ __func__);
break;
}
se = getservbyport(sin.sin_port, "udp");
@@ -142,7 +142,7 @@ xmit_call(int sockfd, struct sockaddr_in *sin,
/* Encode the RPC header part and payload */
if (!xdr_callmsg(xdrs, &mesg) || !func(xdrs, obj)) {
- dprintf(N_WARNING, "xmit_mesg: can't encode RPC message!\n");
+ dprintf(N_WARNING, "%s: can't encode RPC message!", __func__);
xdr_destroy(xdrs);
return 0;
}
@@ -152,9 +152,9 @@ xmit_call(int sockfd, struct sockaddr_in *sin,
if ((err = sendto(sockfd, msgbuf, msglen, 0,
(struct sockaddr *) sin, sizeof(*sin))) < 0) {
- dprintf(N_WARNING, "xmit_mesg: sendto failed: %m");
+ dprintf(N_WARNING, "%s: sendto failed: %m", __func__);
} else if (err != msglen) {
- dprintf(N_WARNING, "xmit_mesg: short write: %m\n");
+ dprintf(N_WARNING, "%s: short write: %m", __func__);
}
xdr_destroy(xdrs);
@@ -174,7 +174,7 @@ recv_rply(int sockfd, struct sockaddr_in *sin, u_long *portp)
/* Receive message */
if ((msglen = recvfrom(sockfd, msgbuf, sizeof(msgbuf), 0,
(struct sockaddr *) sin, &alen)) < 0) {
- dprintf(N_WARNING, "recv_rply: recvfrom failed: %m");
+ dprintf(N_WARNING, "%s: recvfrom failed: %m", __func__);
return NULL;
}
@@ -186,18 +186,20 @@ recv_rply(int sockfd, struct sockaddr_in *sin, u_long *portp)
mesg.rm_reply.rp_acpt.ar_results.proc = (xdrproc_t) xdr_void;
if (!xdr_replymsg(xdrs, &mesg)) {
- note(N_WARNING, "recv_rply: can't decode RPC message!\n");
+ note(N_WARNING, "%s: can't decode RPC message!", __func__);
goto done;
}
if (mesg.rm_reply.rp_stat != 0) {
- note(N_WARNING, "recv_rply: [%s] RPC status %d\n",
+ note(N_WARNING, "%s: [%s] RPC status %d",
+ __func__,
inet_ntoa(sin->sin_addr),
mesg.rm_reply.rp_stat);
goto done;
}
if (mesg.rm_reply.rp_acpt.ar_stat != 0) {
- note(N_WARNING, "recv_rply: [%s] RPC status %d\n",
+ note(N_WARNING, "%s: [%s] RPC status %d",
+ __func__,
inet_ntoa(sin->sin_addr),
mesg.rm_reply.rp_acpt.ar_stat);
goto done;
@@ -214,14 +216,15 @@ recv_rply(int sockfd, struct sockaddr_in *sin, u_long *portp)
strncpy (addr, inet_ntoa(lp->addr),
sizeof (addr) - 1);
addr [sizeof (addr) - 1] = '\0';
- dprintf(N_WARNING, "address mismatch: "
- "expected %s, got %s\n",
+ dprintf(N_WARNING, "%s: address mismatch: "
+ "expected %s, got %s", __func__,
addr, inet_ntoa(sin->sin_addr));
}
if (lp->port == 0) {
if (!xdr_u_long(xdrs, portp)) {
- note(N_WARNING, "recv_rply: [%s] "
- "can't decode reply body!\n",
+ note(N_WARNING,
+ "%s: [%s] can't decode reply body!",
+ __func__,
inet_ntoa(sin->sin_addr));
lp = NULL;
goto done;
@@ -249,8 +252,8 @@ process_entry(int sockfd, notify_list *lp)
/* __u32 proc, vers, prog; */
if (NL_TIMES(lp) == 0) {
- note(N_DEBUG, "Cannot notify %s, giving up.\n",
- inet_ntoa(NL_ADDR(lp)));
+ note(N_DEBUG, "%s: Cannot notify %s, giving up.",
+ __func__, inet_ntoa(NL_ADDR(lp)));
return 0;
}
@@ -275,8 +278,8 @@ process_entry(int sockfd, notify_list *lp)
lp->xid = xmit_call(sockfd, &sin, prog, vers, proc, func, objp);
if (!lp->xid) {
- note(N_WARNING, "notify_host: failed to notify port %d\n",
- ntohs(lp->port));
+ note(N_WARNING, "%s: failed to notify port %d",
+ __func__, ntohs(lp->port));
}
NL_TIMES(lp) -= 1;
@@ -308,11 +311,11 @@ process_reply(FD_SET_TYPE *rfds)
nlist_insert_timer(¬ify, lp);
return 1;
}
- note(N_WARNING, "recv_rply: [%s] service %d not registered",
- inet_ntoa(lp->addr), NL_MY_PROG(lp));
+ note(N_WARNING, "%s: [%s] service %d not registered",
+ __func__, inet_ntoa(lp->addr), NL_MY_PROG(lp));
} else {
- dprintf(N_DEBUG, "Callback to %s (for %d) succeeded.",
- NL_MY_NAME(lp), NL_MON_NAME(lp));
+ dprintf(N_DEBUG, "%s: Callback to %s (for %d) succeeded.",
+ __func__, NL_MY_NAME(lp), NL_MON_NAME(lp));
}
nlist_free(¬ify, lp);
return 1;
@@ -340,7 +343,8 @@ process_notify_list(void)
nlist_insert_timer(¬ify, entry);
} else {
note(N_ERROR,
- "Can't callback %s (%d,%d), giving up.",
+ "%s: Can't callback %s (%d,%d), giving up.",
+ __func__,
NL_MY_NAME(entry),
NL_MY_PROG(entry),
NL_MY_VERS(entry));
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 19/19] rpc.statd: Stop overloading sockfd in utils/statd/rmtcall.c
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (17 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 18/19] rpc.statd: Use __func__ in dprintf Chuck Lever
@ 2008-09-05 21:08 ` Chuck Lever
2008-09-29 11:07 ` [PATCH 00/19] REPOST (with addn'l patches) nfs-utils cleanups Steve Dickson
19 siblings, 0 replies; 21+ messages in thread
From: Chuck Lever @ 2008-09-05 21:08 UTC (permalink / raw)
To: steved; +Cc: neilb, linux-nfs
The Linux kernel's lockd requires that rpc.statd perform notification
callbacks from a privileged source port. To guarantee rpc.statd gets a
privileged source port but runs unprivileged, it calls
statd_get_socket() then drops root privileges before starting it's svc
request processing loop.
Statd's svc request loop is the only caller of the process_foo()
functions in utils/statd/rmtcall.c, but one of them,
process_notify_list() attempts to invoke statd_get_socket() again.
In today's code, this is unneeded because statd_get_socket() is always
invoked before my_svc_run(). However, if it ever succeeded, it would
get an unprivileged source port anyway, causing the kernel to reject
all subsequent requests from statd.
Thus the process_notify_list() function should not ever call
statd_get_socket() because root privileges have been dropped by this
point, and statd_get_socket() wouldn't get a privileged source port,
causing the kernel to reject all subsequent SM_NOTIFY requests.
So all of the process_foo functions in utils/statd/rmtcall.c should use
the global sockfd instead of a local copy, as it already has a
privileged source port.
I've seen some unexplained behavior where statd starts making calls to
the kernel via an unprivileged port. This could be one way that might
occur.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
utils/statd/rmtcall.c | 28 ++++++++++++++++------------
utils/statd/statd.c | 4 ++--
utils/statd/statd.h | 1 +
3 files changed, 19 insertions(+), 14 deletions(-)
diff --git a/utils/statd/rmtcall.c b/utils/statd/rmtcall.c
index 8240529..cc1a4a4 100644
--- a/utils/statd/rmtcall.c
+++ b/utils/statd/rmtcall.c
@@ -56,7 +56,15 @@ static unsigned long xid = 0; /* RPC XID counter */
static int sockfd = -1; /* notify socket */
/*
- * Initialize callback socket
+ * Initialize socket used to notify lockd of peer reboots.
+ *
+ * Returns the file descriptor of the new socket if successful;
+ * otherwise returns -1 and logs an error.
+ *
+ * Lockd rejects such requests if the source port is not privileged.
+ * statd_get_socket() must be invoked while statd still holds root
+ * privileges in order for the socket to acquire a privileged source
+ * port.
*/
int
statd_get_socket(void)
@@ -97,7 +105,7 @@ statd_get_socket(void)
}
static unsigned long
-xmit_call(int sockfd, struct sockaddr_in *sin,
+xmit_call(struct sockaddr_in *sin,
u_int32_t prog, u_int32_t vers, u_int32_t proc,
xdrproc_t func, void *obj)
/* __u32 prog, __u32 vers, __u32 proc, xdrproc_t func, void *obj) */
@@ -163,7 +171,7 @@ xmit_call(int sockfd, struct sockaddr_in *sin,
}
static notify_list *
-recv_rply(int sockfd, struct sockaddr_in *sin, u_long *portp)
+recv_rply(struct sockaddr_in *sin, u_long *portp)
{
unsigned int msgbuf[MAXMSGSIZE], msglen;
struct rpc_msg mesg;
@@ -242,7 +250,7 @@ done:
* Notify operation for a single list entry
*/
static int
-process_entry(int sockfd, notify_list *lp)
+process_entry(notify_list *lp)
{
struct sockaddr_in sin;
struct status new_status;
@@ -276,7 +284,7 @@ process_entry(int sockfd, notify_list *lp)
new_status.state = NL_STATE(lp);
memcpy(new_status.priv, NL_PRIV(lp), SM_PRIV_SIZE);
- lp->xid = xmit_call(sockfd, &sin, prog, vers, proc, func, objp);
+ lp->xid = xmit_call(&sin, prog, vers, proc, func, objp);
if (!lp->xid) {
note(N_WARNING, "%s: failed to notify port %d",
__func__, ntohs(lp->port));
@@ -299,13 +307,13 @@ process_reply(FD_SET_TYPE *rfds)
if (sockfd == -1 || !FD_ISSET(sockfd, rfds))
return 0;
- if (!(lp = recv_rply(sockfd, &sin, &port)))
+ if (!(lp = recv_rply(&sin, &port)))
return 1;
if (lp->port == 0) {
if (port != 0) {
lp->port = htons((unsigned short) port);
- process_entry(sockfd, lp);
+ process_entry(lp);
NL_WHEN(lp) = time(NULL) + NOTIFY_TIMEOUT;
nlist_remove(¬ify, lp);
nlist_insert_timer(¬ify, lp);
@@ -331,13 +339,9 @@ process_notify_list(void)
{
notify_list *entry;
time_t now;
- int fd;
-
- if ((fd = statd_get_socket()) < 0)
- return 0;
while ((entry = notify) != NULL && NL_WHEN(entry) < time(&now)) {
- if (process_entry(fd, entry)) {
+ if (process_entry(entry)) {
NL_WHEN(entry) = time(NULL) + NOTIFY_TIMEOUT;
nlist_remove(¬ify, entry);
nlist_insert_timer(¬ify, entry);
diff --git a/utils/statd/statd.c b/utils/statd/statd.c
index ea985e6..321f7a9 100644
--- a/utils/statd/statd.c
+++ b/utils/statd/statd.c
@@ -75,7 +75,6 @@ static struct option longopts[] =
};
extern void sm_prog_1 (struct svc_req *, register SVCXPRT *);
-extern int statd_get_socket(void);
static void load_state_number(void);
#ifdef SIMULATIONS
@@ -477,7 +476,8 @@ int main (int argc, char **argv)
}
/* Make sure we have a privilege port for calling into the kernel */
- statd_get_socket();
+ if (statd_get_socket() < 0)
+ exit(1);
/* If sm-notify didn't take all the state files, load
* state information into our notify-list so we can
diff --git a/utils/statd/statd.h b/utils/statd/statd.h
index 5d06d88..5a6e289 100644
--- a/utils/statd/statd.h
+++ b/utils/statd/statd.h
@@ -48,6 +48,7 @@ extern void change_state(void);
extern void my_svc_run(void);
extern void notify_hosts(void);
extern void shuffle_dirs(void);
+extern int statd_get_socket(void);
extern int process_notify_list(void);
extern int process_reply(FD_SET_TYPE *);
extern char * xstrdup(const char *);
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 00/19] REPOST (with addn'l patches) nfs-utils cleanups
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
` (18 preceding siblings ...)
2008-09-05 21:08 ` [PATCH 19/19] rpc.statd: Stop overloading sockfd in utils/statd/rmtcall.c Chuck Lever
@ 2008-09-29 11:07 ` Steve Dickson
19 siblings, 0 replies; 21+ messages in thread
From: Steve Dickson @ 2008-09-29 11:07 UTC (permalink / raw)
To: Chuck Lever; +Cc: neilb, linux-nfs
Chuck Lever wrote:
> Hi Steve-
>
> Here are a group of minor clean-ups to nfs-utils culled from my IPv6
> patch set. It would be nice to get these sorted before we start into
> the meat of the client-side IPv6-related changes to nfs-utils. Please
> consider these for the next release of nfs-utils.
>
> In addition to the RPC_ANYSOCK change suggested by Neil, there are
> three additional small patches (8, 9, and 18 in this repost) that I
> found yesterday.
>
> ---
Committed...
steved.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2008-09-29 11:09 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-09-05 21:06 [PATCH 00/19] REPOST (with addn'l patches) nfs-utils cleanups Chuck Lever
[not found] ` <20080905210054.10001.8518.stgit-meopP2rzCrTwdl/1UfZZQIVfYA8g3rJ/@public.gmane.org>
2008-09-05 21:07 ` [PATCH 01/19] rpc.statd: refactor check to see if call is from loopback address Chuck Lever
2008-09-05 21:07 ` [PATCH 02/19] rpc.statd: eliminate --secure_statd Chuck Lever
2008-09-05 21:07 ` [PATCH 03/19] showmount: destroy RPC client when finished Chuck Lever
2008-09-05 21:07 ` [PATCH 04/19] showmount command: clean up error returns from connect_nb() Chuck Lever
2008-09-05 21:07 ` [PATCH 05/19] sm-notify command: include <config.h> Chuck Lever
2008-09-05 21:07 ` [PATCH 06/19] sm-notify command: getaddrinfo(3) addrinfo leak Chuck Lever
2008-09-05 21:07 ` [PATCH 07/19] sm-notify command: clean up error logging Chuck Lever
2008-09-05 21:07 ` [PATCH 08/19] sm-notify command: replace nsm_address typedef Chuck Lever
2008-09-05 21:07 ` [PATCH 09/19] sm-notify command: use static function definitions Chuck Lever
2008-09-05 21:07 ` [PATCH 10/19] nfs-utils: remove unused function rpc_logcall() Chuck Lever
2008-09-05 21:07 ` [PATCH 11/19] nfs-utils: Remove unused function rpc_svcrun() Chuck Lever
2008-09-05 21:08 ` [PATCH 12/19] nfs-utils: remove disabled code from support/nfs/rpcmisc.c Chuck Lever
2008-09-05 21:08 ` [PATCH 13/19] nfs-utils: Clean up support/nfs/rpcmisc.c:closedown() Chuck Lever
2008-09-05 21:08 ` [PATCH 14/19] nfs-utils: make makesock() static Chuck Lever
2008-09-05 21:08 ` [PATCH 15/19] nfs-utils: Remove excess log reporting Chuck Lever
2008-09-05 21:08 ` [PATCH 16/19] nfs-utils: whitespace clean ups in support/nfs/rpcmisc.c Chuck Lever
2008-09-05 21:08 ` [PATCH 17/19] rpc.statd: Clean up: replace "if (!(foo = rtnl))" Chuck Lever
2008-09-05 21:08 ` [PATCH 18/19] rpc.statd: Use __func__ in dprintf Chuck Lever
2008-09-05 21:08 ` [PATCH 19/19] rpc.statd: Stop overloading sockfd in utils/statd/rmtcall.c Chuck Lever
2008-09-29 11:07 ` [PATCH 00/19] REPOST (with addn'l patches) nfs-utils cleanups Steve Dickson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.