From 33732342e62a8dc91d983819a5a98901789cd129 Mon Sep 17 00:00:00 2001 From: Erez Zadok Date: Wed, 2 Mar 2005 03:00:09 +0000 Subject: [PATCH] * amd/srvr_nfs.c (start_nfs_pings): move code from elsewhere (update_nfs_pingval) that initializes the pinger, as well as turns it on/off as needed, and handles changing its value. This is to avoid races and other infinite-loop conditions that could result in ping storms. * amd/srvr_amfs_auto.c (amfs_generic_find_srvr): when creating a new file server structure, default the ping value to AM_PINGER (30sec) and set the FSF_PING_UNINIT flag. * amd/amd.h (FSF_PING_UNINIT): new flag to tell whether the NFS pinger had been initialized for a given file server. * scripts/ctl-amd.in (stop): no need to check if /var/lock/subsys/amd file exists if you do an rm -f afterward. Ensure that proper return value is returned from script. * NEWS, doc/am-utils.texi (opts Option): update meaning of ping=N so that if N=-1, pings are off; if N=0, pings are set to the default value (currently 30 seconds). * amd/nfs_prot_svc.c (nfs_program_2): on TLI system, try to call __rpc_get_local_uid to verify if the RPC call through the local host interface came from UID 0. * configure.in: look for internal libnsl function __rpc_get_local_uid (seems to be available on all known TLI systems, Solaris and HP-UX 11). * conf/transp/transp_tli.c (amu_svc_getcaller): unnecessary function for TLI systems (and it violated a array's bounds, discovered with libumem.so). (bind_resv_port, bind_resv_port_only_udp, get_autofs_address): just to be on the safe side, set struct t_bind's qlen field to non zero (64 by default). This value cannot be zero for TCP connections, and it's unclear if it's good to have it zero for UDP connections, so setting it to 64 is safer. --- ChangeLog | 42 ++++++++++++++++++++++++++++++++ NEWS | 14 ++++++----- amd/amd.h | 3 ++- amd/nfs_prot_svc.c | 34 +++++++++++++++++++++----- amd/nfs_start.c | 4 ++-- amd/srvr_amfs_auto.c | 6 ++--- amd/srvr_nfs.c | 46 ++++++++++++++++++------------------ conf/transp/transp_sockets.c | 4 ++-- conf/transp/transp_tli.c | 33 +++++++++++++++----------- configure.in | 3 ++- doc/am-utils.texi | 19 ++++++++------- include/am_utils.h | 4 +++- scripts/ctl-amd.in | 2 +- 13 files changed, 145 insertions(+), 69 deletions(-) diff --git a/ChangeLog b/ChangeLog index 68c7cd2..311661a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,45 @@ +2005-03-01 Erez Zadok + + * amd/srvr_nfs.c (start_nfs_pings): move code from elsewhere + (update_nfs_pingval) that initializes the pinger, as well as turns + it on/off as needed, and handles changing its value. This is to + avoid races and other infinite-loop conditions that could result + in ping storms. + + * amd/srvr_amfs_auto.c (amfs_generic_find_srvr): when creating a + new file server structure, default the ping value to AM_PINGER + (30sec) and set the FSF_PING_UNINIT flag. + + * amd/amd.h (FSF_PING_UNINIT): new flag to tell whether the NFS + pinger had been initialized for a given file server. + + * scripts/ctl-amd.in (stop): no need to check if + /var/lock/subsys/amd file exists if you do an rm -f afterward. + Ensure that proper return value is returned from script. + + * NEWS, doc/am-utils.texi (opts Option): update meaning of ping=N + so that if N=-1, pings are off; if N=0, pings are set to the + default value (currently 30 seconds). + +2005-02-28 Erez Zadok + + * amd/nfs_prot_svc.c (nfs_program_2): on TLI system, try to call + __rpc_get_local_uid to verify if the RPC call through the + local host interface came from UID 0. + + * configure.in: look for internal libnsl function + __rpc_get_local_uid (seems to be available on all known TLI + systems, Solaris and HP-UX 11). + + * conf/transp/transp_tli.c (amu_svc_getcaller): unnecessary + function for TLI systems (and it violated a array's bounds, + discovered with libumem.so). + (bind_resv_port, bind_resv_port_only_udp, get_autofs_address): + just to be on the safe side, set struct t_bind's qlen field to non + zero (64 by default). This value cannot be zero for TCP + connections, and it's unclear if it's good to have it zero for UDP + connections, so setting it to 64 is safer. + 2005-02-27 Erez Zadok * doc/am-utils.texi (opts Option, Keep-alives): update text on diff --git a/NEWS b/NEWS index 4b02d10..ce09a6c 100644 --- a/NEWS +++ b/NEWS @@ -93,12 +93,14 @@ servers. It can now be set to any value for each server separately. Setting it to a large value can reduce the amount of NFS_NULL chatter on your network considerably, especially in large sites. Setting this to -1 - turns off pings for that server (useful in NFS-HA setups). Note that if - you have multiple Amd entries using the same file server, and each entry - sets a different value of N, then each time Amd mounts a new entry, the - ping value will be re-evaluated (and updated, turned off, or turned back - on as needed). Note that NFS_NULL pings are sent for both UDP and TCP - mounts, because even a hung TCP mount can cause user processes to hang. + will turn off pings for that server (useful in NFS-HA setups). Setting N + to 0 will pick the default ping value in Amd (currently 30 seconds). Note + that if you have multiple Amd entries using the same file server, and each + entry sets a different value of N, then each time Amd mounts a new entry, + the ping value will be re-evaluated (and updated, turned off, or turned + back on as needed). Note that NFS_NULL pings are sent for both UDP and + TCP mounts, because even a hung TCP mount can cause user processes to + hang. - bugs fixed: * various memory management problems (leaks, etc) diff --git a/amd/amd.h b/amd/amd.h index 0a07ac7..1364f18 100644 --- a/amd/amd.h +++ b/amd/amd.h @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: amd.h,v 1.58 2005/02/23 03:59:08 ezk Exp $ + * $Id: amd.h,v 1.59 2005/03/02 03:00:09 ezk Exp $ * */ @@ -127,6 +127,7 @@ #define FSF_WANT 0x0008 /* Want a wakeup call */ #define FSF_PINGING 0x0010 /* Already doing pings */ #define FSF_WEBNFS 0x0020 /* Don't try to contact portmapper */ +#define FSF_PING_UNINIT 0x0040 /* ping values have not been initilized */ #define FSRV_ERROR(fs) ((fs) && (((fs)->fs_flags & FSF_ERROR) == FSF_ERROR)) #define FSRV_ISDOWN(fs) ((fs) && (((fs)->fs_flags & (FSF_DOWN|FSF_VALID)) == (FSF_DOWN|FSF_VALID))) #define FSRV_ISUP(fs) (!(fs) || (((fs)->fs_flags & (FSF_DOWN|FSF_VALID)) == (FSF_VALID))) diff --git a/amd/nfs_prot_svc.c b/amd/nfs_prot_svc.c index 86eafe9..ddc27bd 100644 --- a/amd/nfs_prot_svc.c +++ b/amd/nfs_prot_svc.c @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: nfs_prot_svc.c,v 1.14 2005/01/03 20:56:45 ezk Exp $ + * $Id: nfs_prot_svc.c,v 1.15 2005/03/02 03:00:09 ezk Exp $ * */ @@ -97,28 +97,50 @@ nfs_program_2(struct svc_req *rqstp, SVCXPRT *transp) char *result; xdrproc_t xdr_argument, xdr_result; nfssvcproc_t local; + +#ifdef HAVE_TRANSPORT_TYPE_TLI + /* + * On TLI systems we don't use an INET network type, but a "ticlts" (see + * /etc/netconfig and conf/transp_tli.c:create_nfs_service). This means + * that packets could only come from the loopback interface, and we don't + * need to check them and filter possibly spoofed packets. Therefore we + * only need to check if the UID caller is correct. + */ +# ifdef HAVE___RPC_GET_LOCAL_UID + uid_t u; + /* extern definition for an internal libnsl function */ + extern int __rpc_get_local_uid(SVCXPRT *transp, uid_t *uid); + if (__rpc_get_local_uid(transp, &u) >= 0 && u != 0) { + plog(XLOG_WARNING, "ignoring request from UID %ld, must be 0", (long) u); + return; + } +# else /* not HAVE___RPC_GET_LOCAL_UID */ + dlog("cannot verify local uid for rpc request"); +# endif /* HAVE___RPC_GET_LOCAL_UID */ +#else /* not HAVE_TRANPORT_TYPE_TLI */ struct sockaddr_in *sinp; char dq[20], dq2[28]; - sinp = amu_svc_getcaller(rqstp->rq_xprt); -#ifdef MNT2_NFS_OPT_RESVPORT +# ifdef MNT2_NFS_OPT_RESVPORT /* Verify that the request comes from a reserved port */ - if (ntohs(sinp->sin_port) >= IPPORT_RESERVED && + if (sinp && + ntohs(sinp->sin_port) >= IPPORT_RESERVED && !(gopt.flags & CFM_NFS_INSECURE_PORT)) { plog(XLOG_WARNING, "ignoring request from %s:%u, port not reserved", inet_dquad(dq, sinp->sin_addr.s_addr), ntohs(sinp->sin_port)); return; } -#endif /* MNT2_NFS_OPT_RESVPORT */ +# endif /* MNT2_NFS_OPT_RESVPORT */ /* if the address does not match, ignore the request */ - if (sinp->sin_addr.s_addr && sinp->sin_addr.s_addr != myipaddr.s_addr) { + if (sinp && sinp->sin_addr.s_addr != myipaddr.s_addr) { plog(XLOG_WARNING, "ignoring request from %s:%u, expected %s", inet_dquad(dq, sinp->sin_addr.s_addr), ntohs(sinp->sin_port), inet_dquad(dq2, myipaddr.s_addr)); return; } +#endif /* not HAVE_TRANPORT_TYPE_TLI */ current_transp = NULL; diff --git a/amd/nfs_start.c b/amd/nfs_start.c index 7af406c..f4e169c 100644 --- a/amd/nfs_start.c +++ b/amd/nfs_start.c @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: nfs_start.c,v 1.24 2005/02/23 03:59:08 ezk Exp $ + * $Id: nfs_start.c,v 1.25 2005/03/02 03:00:09 ezk Exp $ * */ @@ -51,7 +51,7 @@ # define SELECT_MAXWAIT 16 #endif /* not SELECT_MAXWAIT */ -SVCXPRT *nfsxprt; +SVCXPRT *nfsxprt = NULL; u_short nfs_port = 0; #ifndef HAVE_SIGACTION diff --git a/amd/srvr_amfs_auto.c b/amd/srvr_amfs_auto.c index c0befb7..adf9141 100644 --- a/amd/srvr_amfs_auto.c +++ b/amd/srvr_amfs_auto.c @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: srvr_amfs_auto.c,v 1.14 2005/01/03 20:56:45 ezk Exp $ + * $Id: srvr_amfs_auto.c,v 1.15 2005/03/02 03:00:09 ezk Exp $ * */ @@ -72,8 +72,8 @@ amfs_generic_find_srvr(mntfs *mf) fs->fs_host = strdup("localhost"); fs->fs_ip = 0; fs->fs_cid = 0; - fs->fs_pinger = 0; - fs->fs_flags = FSF_VALID; + fs->fs_pinger = AM_PINGER; + fs->fs_flags = FSF_VALID | FSF_PING_UNINIT; fs->fs_type = "local"; fs->fs_private = 0; fs->fs_prfree = 0; diff --git a/amd/srvr_nfs.c b/amd/srvr_nfs.c index e7df0a0..6821ea4 100644 --- a/amd/srvr_nfs.c +++ b/amd/srvr_nfs.c @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: srvr_nfs.c,v 1.38 2005/02/28 01:38:28 ezk Exp $ + * $Id: srvr_nfs.c,v 1.39 2005/03/02 03:00:09 ezk Exp $ * */ @@ -113,7 +113,6 @@ static char *protocols[] = { "tcp", "udp", NULL }; static void nfs_keepalive(voidp); - /* * Flush any cached data */ @@ -564,7 +563,7 @@ nfs_keepalive(voidp v) /* * Back off the ping interval if we are not getting replies and - * the remote system is know to be down. + * the remote system is known to be down. */ switch (fs->fs_flags & (FSF_DOWN | FSF_VALID)) { case FSF_VALID: /* Up */ @@ -590,11 +589,28 @@ nfs_keepalive(voidp v) static void start_nfs_pings(fserver *fs, int pingval) { - if (fs->fs_flags & FSF_PINGING) { - dlog("Already running pings to %s", fs->fs_host); + if (pingval == 0) /* could be because ping mnt option not found */ + pingval = AM_PINGER; + /* if pings haven't been initalized, then init them for first time */ + if (fs->fs_flags & FSF_PING_UNINIT) { + fs->fs_flags &= ~FSF_PING_UNINIT; + plog(XLOG_INFO, "initializing %s's pinger to %d sec", fs->fs_host, pingval); + goto do_pings; + } + + if ((fs->fs_flags & FSF_PINGING) && fs->fs_pinger == pingval) { + dlog("already running pings to %s", fs->fs_host); return; } + /* if got here, then we need to update the ping value */ + plog(XLOG_INFO, "changing %s's ping value from %d%s to %d%s", + fs->fs_host, + fs->fs_pinger, (fs->fs_pinger < 0 ? " (off)" : ""), + pingval, (pingval < 0 ? " (off)" : "")); + do_pings: + fs->fs_pinger = pingval; + if (fs->fs_cid) untimeout(fs->fs_cid); if (pingval < 0) { @@ -608,22 +624,6 @@ start_nfs_pings(fserver *fs, int pingval) } -/* check and update fserver pingval as needed */ -static void -update_nfs_pingval(fserver *fs, int pingval) -{ - if (fs->fs_pinger != pingval) { - plog(XLOG_INFO, "changing %s's ping value from %d%s to %d%s", - fs->fs_host, - fs->fs_pinger, (fs->fs_pinger < 0 ? " (off)" : ""), - pingval, (pingval < 0 ? " (off)" : "")); - fs->fs_flags &= ~FSF_PINGING; /* so start_nfs_pings() will actually do some work */ - fs->fs_pinger = pingval; - start_nfs_pings(fs, pingval); /* start_nfs_pings() does actual work */ - } -} - - /* * Find an nfs server for a host. */ @@ -895,7 +895,7 @@ no_dns: fs->fs_flags &= ~FSF_WEBNFS; /* check if pingval needs to be updated/set/reset */ - update_nfs_pingval(fs, pingval); + start_nfs_pings(fs, pingval); /* * Following if statement from Mike Mitchell @@ -953,6 +953,7 @@ no_dns: fs->fs_proto = nfs_proto; fs->fs_type = MNTTAB_TYPE_NFS; fs->fs_pinger = AM_PINGER; + fs->fs_flags |= FSF_PING_UNINIT; /* pinger hasn't been initialized */ np = ALLOC(struct nfs_private); memset((voidp) np, 0, sizeof(*np)); np->np_mountd_inval = TRUE; @@ -969,7 +970,6 @@ no_dns: if (!FSRV_ERROR(fs)) { /* start of keepalive timer, first updating pingval */ - update_nfs_pingval(fs, pingval); start_nfs_pings(fs, pingval); if (fserver_is_down) fs->fs_flags |= FSF_VALID | FSF_DOWN; diff --git a/conf/transp/transp_sockets.c b/conf/transp/transp_sockets.c index 09885e3..a8c78e4 100644 --- a/conf/transp/transp_sockets.c +++ b/conf/transp/transp_sockets.c @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: transp_sockets.c,v 1.32 2005/02/23 03:59:08 ezk Exp $ + * $Id: transp_sockets.c,v 1.33 2005/03/02 03:00:09 ezk Exp $ * * Socket specific utilities. * -Erez Zadok @@ -230,7 +230,7 @@ struct sockaddr_in * amu_svc_getcaller(SVCXPRT *xprt) { /* glibc 2.2 returns a sockaddr_storage ??? */ - return (struct sockaddr_in *)svc_getcaller(xprt); + return (struct sockaddr_in *) svc_getcaller(xprt); } diff --git a/conf/transp/transp_tli.c b/conf/transp/transp_tli.c index d1a1b45..37d785c 100644 --- a/conf/transp/transp_tli.c +++ b/conf/transp/transp_tli.c @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: transp_tli.c,v 1.28 2005/02/23 03:59:08 ezk Exp $ + * $Id: transp_tli.c,v 1.29 2005/03/02 03:00:09 ezk Exp $ * * TLI specific utilities. * -Erez Zadok @@ -119,7 +119,7 @@ bind_resv_port(int td, u_short *pp) memset((char *) treq->addr.buf, 0, treq->addr.len); sin = (struct sockaddr_in *) treq->addr.buf; sin->sin_family = AF_INET; - treq->qlen = 0; /* 0 is ok for udp, for tcp you need qlen>0 */ + treq->qlen = 64; /* 0 is ok for udp, for tcp you need qlen>0 */ treq->addr.len = treq->addr.maxlen; errno = EADDRINUSE; port = IPPORT_RESERVED; @@ -274,19 +274,23 @@ badout: } +#ifdef NOT_NEEDED_ON_TLI_SYSTEMS /* * find the address of the caller of an RPC procedure. */ struct sockaddr_in * amu_svc_getcaller(SVCXPRT *xprt) { - struct netbuf *nbp = (struct netbuf *) NULL; - - if ((nbp = svc_getrpccaller(xprt)) != NULL) - return (struct sockaddr_in *) nbp->buf; /* all OK */ - - return NULL; /* failed */ + /* + * On TLI systems we don't use an INET network type, but a "ticlts" (see + * /etc/netconfig). This means that packets could only come from the + * loopback interface, and we don't need to check them and filter possibly + * spoofed packets. Therefore we simply return NULL here, and the caller + * will ignore the result. + */ + return NULL; /* tell called to ignore check */ } +#endif /* NOT_NEEDED_ON_TLI_SYSTEMS */ /* @@ -362,7 +366,7 @@ bind_resv_port_only_udp(u_short *pp) memset((char *) treq->addr.buf, 0, treq->addr.len); sin = (struct sockaddr_in *) treq->addr.buf; sin->sin_family = AF_INET; - treq->qlen = 0; /* 0 is ok for udp, for tcp you need qlen>0 */ + treq->qlen = 64; /* 0 is ok for udp, for tcp you need qlen>0 */ treq->addr.len = treq->addr.maxlen; errno = EADDRINUSE; @@ -813,11 +817,12 @@ get_autofs_address(struct netconfig *ncp, struct t_bind *tbp) tbp->addr.len = addrs->n_addrs->len; tbp->addr.maxlen = addrs->n_addrs->len; memcpy(tbp->addr.buf, addrs->n_addrs->buf, addrs->n_addrs->len); - tbp->qlen = 8; /* arbitrary? who cares really. -ion - Actually, Ion, I found out that if - qlen==0, then TCP onnections fail, but UDP - works. -Erez. - */ + /* + * qlen should not be zero for TCP connections. It's not clear what it + * should be for UDP connections, but setting it to something like 64 seems + * to be the safe value that works. + */ + tbp->qlen = 64; /* all OK */ netdir_free((voidp) addrs, ND_ADDRLIST); diff --git a/configure.in b/configure.in index 191e4d5..6a5d53c 100644 --- a/configure.in +++ b/configure.in @@ -53,7 +53,7 @@ AH_BOTTOM([ dnl dnl AC_CONFIG_AUX_DIR(m4) AC_PREREQ(2.52) -AC_REVISION($Revision: 1.76 $) +AC_REVISION($Revision: 1.77 $) AC_COPYRIGHT([Copyright (c) 1997-2005 Erez Zadok]) dnl find out system type AC_MSG_NOTICE(*** SYSTEM TYPES ***) @@ -255,6 +255,7 @@ dnl ====================================================================== dnl Generic Function Checks AC_MSG_NOTICE(*** GENERIC LIBRARY FUNCTIONS ***) AC_CHECK_FUNCS( \ + __rpc_get_local_uid \ __seterr_reply \ _seterr_reply \ bcmp \ diff --git a/doc/am-utils.texi b/doc/am-utils.texi index 1357ab1..688af5c 100644 --- a/doc/am-utils.texi +++ b/doc/am-utils.texi @@ -38,7 +38,7 @@ @c @c %W% (Berkeley) %G% @c -@c $Id: am-utils.texi,v 1.95 2005/02/28 01:38:28 ezk Exp $ +@c $Id: am-utils.texi,v 1.96 2005/03/02 03:00:09 ezk Exp $ @c @setfilename am-utils.info @@ -2292,14 +2292,15 @@ mounting local disks, floppies, and CD-ROMs). See also the related @cindex Mount flags; ping The interval, in seconds, between keep-alive pings. When four consecutive pings have failed the mount point is marked as hung. This -interval defaults to 30 seconds. If the ping interval is less than -zero, no pings are sent and the host is assumed to be always up, which -can cause unmounts to hang See the @i{softlookup} option for a better -alternative. Turning pings off can be useful in NFS-HA -(High-Availability) sites where the NFS service rarely goes down. -Setting the ping value to a large value can reduce the amount of -NFS_NULL chatter on your network considerably, especially in large -sites. +interval defaults to 30 seconds; if the ping interval is set to zero, +@i{Amd} will use the default 30-second interval. If the interval is +set to -1 (or any other negative value), no pings are sent and the +host is assumed to be always up, which can cause unmounts to hang See +the @i{softlookup} option for a better alternative. Turning pings off +can be useful in NFS-HA (High-Availability) sites where the NFS +service rarely goes down. Setting the ping value to a large value can +reduce the amount of NFS_NULL chatter on your network considerably, +especially in large sites. Note that if you have multiple @i{Amd} entries using the same file server, and each entry sets a different value of N, then each time Amd diff --git a/include/am_utils.h b/include/am_utils.h index f6a2b30..08699b6 100644 --- a/include/am_utils.h +++ b/include/am_utils.h @@ -37,7 +37,7 @@ * SUCH DAMAGE. * * - * $Id: am_utils.h,v 1.60 2005/02/23 03:59:09 ezk Exp $ + * $Id: am_utils.h,v 1.61 2005/03/02 03:00:09 ezk Exp $ * */ @@ -299,7 +299,9 @@ extern int pickup_rpc_reply(voidp, int, voidp, XDRPROC_T_TYPE); extern int switch_option(char *); extern int switch_to_logfile(char *logfile, int orig_umask); extern mntlist *read_mtab(char *, const char *); +#ifndef HAVE_TRANSPORT_TYPE_TLI extern struct sockaddr_in *amu_svc_getcaller(SVCXPRT *xprt); +#endif /* not HAVE_TRANSPORT_TYPE_TLI */ extern time_t time(time_t *); extern void amu_get_myaddress(struct in_addr *iap, const char *preferred_localhost); extern void amu_release_controlling_tty(void); diff --git a/scripts/ctl-amd.in b/scripts/ctl-amd.in index 2523f41..694b31a 100755 --- a/scripts/ctl-amd.in +++ b/scripts/ctl-amd.in @@ -103,7 +103,7 @@ case "$1" in echo "killing amd..." killproc " amd" wait4amd2die - test -f /var/lock/subsys/amd && rm -f /var/lock/subsys/amd + rm -f /var/lock/subsys/amd ;; 'restart') -- 2.43.0