Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 28 Sep 2017 22:33:01 +0000 (UTC)
From:      Rick Macklem <rmacklem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r324090 - in head/sys/fs: nfs nfsclient
Message-ID:  <201709282233.v8SMX2b3092271@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: rmacklem
Date: Thu Sep 28 22:33:01 2017
New Revision: 324090
URL: https://svnweb.freebsd.org/changeset/base/324090

Log:
  Change nfsv4_getipaddr() and nfsrpc_fillsa() to not use sockaddr_storage.
  
  This patch changes nfsv4_getipaddr() and nfsrpc_fillsa() to use
  a sockaddr_in * and sockaddr_in6 * instead of sockaddr_storage, to
  avoid allocating the latter on the stack. It also moves the nfsrpc_fillsa()
  call to after the completion of parsing of the DeviceInfo reply from
  the server. This patch is in preparation for addition of Flex File
  Layout support in a future commit.
  It only affects the "pnfs" NFSv4.1 client mount option and should not
  have changed its semantics.

Modified:
  head/sys/fs/nfs/nfs_commonsubs.c
  head/sys/fs/nfs/nfs_var.h
  head/sys/fs/nfsclient/nfs_clrpcops.c

Modified: head/sys/fs/nfs/nfs_commonsubs.c
==============================================================================
--- head/sys/fs/nfs/nfs_commonsubs.c	Thu Sep 28 19:57:46 2017	(r324089)
+++ head/sys/fs/nfs/nfs_commonsubs.c	Thu Sep 28 22:33:01 2017	(r324090)
@@ -3938,14 +3938,13 @@ newnfs_sndunlock(int *flagp)
 }
 
 APPLESTATIC int
-nfsv4_getipaddr(struct nfsrv_descript *nd, struct sockaddr_storage *sa,
-    int *isudp)
+nfsv4_getipaddr(struct nfsrv_descript *nd, struct sockaddr_in *sin,
+    struct sockaddr_in6 *sin6, sa_family_t *saf, int *isudp)
 {
-	struct sockaddr_in *sad;
-	struct sockaddr_in6 *sad6;
 	struct in_addr saddr;
 	uint32_t portnum, *tl;
-	int af = 0, i, j, k;
+	int i, j, k;
+	sa_family_t af = AF_UNSPEC;
 	char addr[64], protocol[5], *cp;
 	int cantparse = 0, error = 0;
 	uint16_t portv;
@@ -4023,20 +4022,20 @@ nfsv4_getipaddr(struct nfsrv_descript *nd, struct sock
 			cantparse = 1;
 		if (cantparse == 0) {
 			if (af == AF_INET) {
-				sad = (struct sockaddr_in *)sa;
-				if (inet_pton(af, addr, &sad->sin_addr) == 1) {
-					sad->sin_len = sizeof(*sad);
-					sad->sin_family = AF_INET;
-					sad->sin_port = htons(portv);
+				if (inet_pton(af, addr, &sin->sin_addr) == 1) {
+					sin->sin_len = sizeof(*sin);
+					sin->sin_family = AF_INET;
+					sin->sin_port = htons(portv);
+					*saf = af;
 					return (0);
 				}
 			} else {
-				sad6 = (struct sockaddr_in6 *)sa;
-				if (inet_pton(af, addr, &sad6->sin6_addr)
+				if (inet_pton(af, addr, &sin6->sin6_addr)
 				    == 1) {
-					sad6->sin6_len = sizeof(*sad6);
-					sad6->sin6_family = AF_INET6;
-					sad6->sin6_port = htons(portv);
+					sin6->sin6_len = sizeof(*sin6);
+					sin6->sin6_family = AF_INET6;
+					sin6->sin6_port = htons(portv);
+					*saf = af;
 					return (0);
 				}
 			}

Modified: head/sys/fs/nfs/nfs_var.h
==============================================================================
--- head/sys/fs/nfs/nfs_var.h	Thu Sep 28 19:57:46 2017	(r324089)
+++ head/sys/fs/nfs/nfs_var.h	Thu Sep 28 22:33:01 2017	(r324090)
@@ -287,8 +287,8 @@ void nfsrv_cleanusergroup(void);
 int nfsrv_checkutf8(u_int8_t *, int);
 int newnfs_sndlock(int *);
 void newnfs_sndunlock(int *);
-int nfsv4_getipaddr(struct nfsrv_descript *, struct sockaddr_storage *,
-    int *);
+int nfsv4_getipaddr(struct nfsrv_descript *, struct sockaddr_in *,
+    struct sockaddr_in6 *, sa_family_t *, int *);
 int nfsv4_seqsession(uint32_t, uint32_t, uint32_t, struct nfsslot *,
     struct mbuf **, uint16_t);
 void nfsv4_seqsess_cacherep(uint32_t, struct nfsslot *, int, struct mbuf **);

Modified: head/sys/fs/nfsclient/nfs_clrpcops.c
==============================================================================
--- head/sys/fs/nfsclient/nfs_clrpcops.c	Thu Sep 28 19:57:46 2017	(r324089)
+++ head/sys/fs/nfsclient/nfs_clrpcops.c	Thu Sep 28 22:33:01 2017	(r324090)
@@ -109,8 +109,8 @@ static int nfsrpc_setaclrpc(vnode_t, struct ucred *, N
 static int nfsrpc_getlayout(struct nfsmount *, vnode_t, struct nfsfh *, int,
     uint32_t *, nfsv4stateid_t *, uint64_t, struct nfscllayout **,
     struct ucred *, NFSPROC_T *);
-static int nfsrpc_fillsa(struct nfsmount *, struct sockaddr_storage *,
-    struct nfsclds **, NFSPROC_T *);
+static int nfsrpc_fillsa(struct nfsmount *, struct sockaddr_in *,
+    struct sockaddr_in6 *, sa_family_t, int, struct nfsclds **, NFSPROC_T *);
 static void nfscl_initsessionslots(struct nfsclsession *);
 static int nfscl_doflayoutio(vnode_t, struct uio *, int *, int *, int *,
     nfsv4stateid_t *, int, struct nfscldevinfo *, struct nfscllayout *,
@@ -4885,14 +4885,17 @@ nfsrpc_getdeviceinfo(struct nfsmount *nmp, uint8_t *de
 	uint32_t cnt, *tl;
 	struct nfsrv_descript nfsd;
 	struct nfsrv_descript *nd = &nfsd;
-	struct sockaddr_storage ss;
-	struct nfsclds *dsp = NULL, **dspp;
+	struct sockaddr_in sin, ssin;
+	struct sockaddr_in6 sin6, ssin6;
+	struct nfsclds *dsp = NULL, **dspp, **gotdspp;
 	struct nfscldevinfo *ndi;
-	int addrcnt, bitcnt, error, i, isudp, j, pos, safilled, stripecnt;
+	int addrcnt = 0, bitcnt, error, gotvers, i, isudp, j, stripecnt;
 	uint8_t stripeindex;
+	sa_family_t af, safilled;
 
 	*ndip = NULL;
 	ndi = NULL;
+	gotdspp = NULL;
 	nfscl_reqstart(nd, NFSPROC_GETDEVICEINFO, nmp, NULL, 0, NULL, NULL, 0,
 	    0);
 	NFSM_BUILD(tl, uint32_t *, NFSX_V4DEVICEID + 3 * NFSX_UNSIGNED);
@@ -4960,7 +4963,7 @@ nfsrpc_getdeviceinfo(struct nfsmount *nmp, uint8_t *de
 		}
 
 		/* Now, dissect the server address(es). */
-		safilled = 0;
+		safilled = AF_UNSPEC;
 		for (i = 0; i < addrcnt; i++) {
 			NFSM_DISSECT(tl, uint32_t *, NFSX_UNSIGNED);
 			cnt = fxdr_unsigned(uint32_t, *tl);
@@ -4970,61 +4973,65 @@ nfsrpc_getdeviceinfo(struct nfsmount *nmp, uint8_t *de
 				goto nfsmout;
 			}
 			dspp = nfsfldi_addr(ndi, i);
-			pos = arc4random() % cnt;	/* Choose one. */
-			safilled = 0;
+			safilled = AF_UNSPEC;
 			for (j = 0; j < cnt; j++) {
-				error = nfsv4_getipaddr(nd, &ss, &isudp);
+				error = nfsv4_getipaddr(nd, &sin, &sin6, &af,
+				    &isudp);
 				if (error != 0 && error != EPERM) {
 					error = NFSERR_BADXDR;
 					goto nfsmout;
 				}
 				if (error == 0 && isudp == 0) {
 					/*
-					 * The algorithm is:
-					 * - use "pos" entry if it is of the
-					 *   same af_family or none of them
-					 *   is of the same af_family
-					 * else
-					 * - use the first one of the same
-					 *   af_family.
+					 * The priority is:
+					 * - Same address family.
+					 * Save the address and dspp, so that
+					 * the connection can be done after
+					 * parsing is complete.
 					 */
-					if ((safilled == 0 && ss.ss_family ==
-					     nmp->nm_nam->sa_family) ||
-					    (j == pos &&
-					     (safilled == 0 || ss.ss_family ==
-					      nmp->nm_nam->sa_family)) ||
-					    (safilled == 1 && ss.ss_family ==
-					     nmp->nm_nam->sa_family)) {
-						error = nfsrpc_fillsa(nmp, &ss,
-						    &dsp, p);
-						if (error == 0) {
-							*dspp = dsp;
-							if (ss.ss_family ==
-							 nmp->nm_nam->sa_family)
-								safilled = 2;
-							else
-								safilled = 1;
-						}
+					if (safilled == AF_UNSPEC ||
+					    (af == nmp->nm_nam->sa_family &&
+					     safilled != nmp->nm_nam->sa_family)
+					   ) {
+						if (af == AF_INET)
+							ssin = sin;
+						else
+							ssin6 = sin6;
+						safilled = af;
+						gotdspp = dspp;
 					}
 				}
 			}
-			if (safilled == 0)
-				break;
 		}
 
+		gotvers = NFS_VER4;	/* Always NFSv4 for File Layout. */
+
 		/* And the notify bits. */
 		NFSM_DISSECT(tl, uint32_t *, NFSX_UNSIGNED);
-		if (safilled != 0) {
-			bitcnt = fxdr_unsigned(int, *tl);
-			if (bitcnt > 0) {
-				NFSM_DISSECT(tl, uint32_t *, NFSX_UNSIGNED);
-				if (notifybitsp != NULL)
-					*notifybitsp =
-					    fxdr_unsigned(uint32_t, *tl);
-			}
+		bitcnt = fxdr_unsigned(int, *tl);
+		if (bitcnt > 0) {
+			NFSM_DISSECT(tl, uint32_t *, NFSX_UNSIGNED);
+			if (notifybitsp != NULL)
+				*notifybitsp =
+				    fxdr_unsigned(uint32_t, *tl);
+		}
+		if (safilled != AF_UNSPEC) {
+			KASSERT(ndi != NULL, ("ndi is NULL"));
 			*ndip = ndi;
 		} else
 			error = EPERM;
+		if (error == 0) {
+			/*
+			 * Now we can do a TCP connection for the correct
+			 * NFS version and IP address.
+			 */
+			error = nfsrpc_fillsa(nmp, &ssin, &ssin6, safilled,
+			    gotvers, &dsp, p);
+		}
+		if (error == 0) {
+			KASSERT(gotdspp != NULL, ("gotdspp is NULL"));
+			*gotdspp = dsp;
+		}
 	}
 	if (nd->nd_repstat != 0)
 		error = nd->nd_repstat;
@@ -5213,11 +5220,12 @@ nfsrpc_getlayout(struct nfsmount *nmp, vnode_t vp, str
  * mount point and a pointer to it is returned.
  */
 static int
-nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_storage *ssp,
-    struct nfsclds **dspp, NFSPROC_T *p)
+nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_in *sin,
+    struct sockaddr_in6 *sin6, sa_family_t af, int vers, struct nfsclds **dspp,
+    NFSPROC_T *p)
 {
-	struct sockaddr_in *msad, *sad, *ssd;
-	struct sockaddr_in6 *msad6, *sad6, *ssd6;
+	struct sockaddr_in *msad, *sad;
+	struct sockaddr_in6 *msad6, *sad6;
 	struct nfsclclient *clp;
 	struct nfssockreq *nrp;
 	struct nfsclds *dsp, *tdsp;
@@ -5232,10 +5240,8 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_st
 	NFSUNLOCKCLSTATE();
 	if (clp == NULL)
 		return (EPERM);
-	if (ssp->ss_family == AF_INET) {
-		ssd = (struct sockaddr_in *)ssp;
+	if (af == AF_INET) {
 		NFSLOCKMNT(nmp);
-
 		/*
 		 * Check to see if we already have a session for this
 		 * address that is usable for a DS.
@@ -5246,8 +5252,8 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_st
 		tdsp = TAILQ_FIRST(&nmp->nm_sess);
 		while (tdsp != NULL) {
 			if (msad != NULL && msad->sin_family == AF_INET &&
-			    ssd->sin_addr.s_addr == msad->sin_addr.s_addr &&
-			    ssd->sin_port == msad->sin_port &&
+			    sin->sin_addr.s_addr == msad->sin_addr.s_addr &&
+			    sin->sin_port == msad->sin_port &&
 			    (tdsp->nfsclds_flags & NFSCLDS_DS) != 0 &&
 			    tdsp->nfsclds_sess.nfsess_defunct == 0) {
 				*dspp = tdsp;
@@ -5268,14 +5274,12 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_st
 		sad = malloc(sizeof(*sad), M_SONAME, M_WAITOK | M_ZERO);
 		sad->sin_len = sizeof(*sad);
 		sad->sin_family = AF_INET;
-		sad->sin_port = ssd->sin_port;
-		sad->sin_addr.s_addr = ssd->sin_addr.s_addr;
+		sad->sin_port = sin->sin_port;
+		sad->sin_addr.s_addr = sin->sin_addr.s_addr;
 		nrp = malloc(sizeof(*nrp), M_NFSSOCKREQ, M_WAITOK | M_ZERO);
 		nrp->nr_nam = (struct sockaddr *)sad;
-	} else if (ssp->ss_family == AF_INET6) {
-		ssd6 = (struct sockaddr_in6 *)ssp;
+	} else if (af == AF_INET6) {
 		NFSLOCKMNT(nmp);
-
 		/*
 		 * Check to see if we already have a session for this
 		 * address that is usable for a DS.
@@ -5286,9 +5290,9 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_st
 		tdsp = TAILQ_FIRST(&nmp->nm_sess);
 		while (tdsp != NULL) {
 			if (msad6 != NULL && msad6->sin6_family == AF_INET6 &&
-			    IN6_ARE_ADDR_EQUAL(&ssd6->sin6_addr,
+			    IN6_ARE_ADDR_EQUAL(&sin6->sin6_addr,
 			    &msad6->sin6_addr) &&
-			    ssd6->sin6_port == msad6->sin6_port &&
+			    sin6->sin6_port == msad6->sin6_port &&
 			    (tdsp->nfsclds_flags & NFSCLDS_DS) != 0 &&
 			    tdsp->nfsclds_sess.nfsess_defunct == 0) {
 				*dspp = tdsp;
@@ -5308,8 +5312,8 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_st
 		sad6 = malloc(sizeof(*sad6), M_SONAME, M_WAITOK | M_ZERO);
 		sad6->sin6_len = sizeof(*sad6);
 		sad6->sin6_family = AF_INET6;
-		sad6->sin6_port = ssd6->sin6_port;
-		NFSBCOPY(&ssd6->sin6_addr, &sad6->sin6_addr,
+		sad6->sin6_port = sin6->sin6_port;
+		NFSBCOPY(&sin6->sin6_addr, &sad6->sin6_addr,
 		    sizeof(struct in6_addr));
 		nrp = malloc(sizeof(*nrp), M_NFSSOCKREQ, M_WAITOK | M_ZERO);
 		nrp->nr_nam = (struct sockaddr *)sad6;
@@ -5319,7 +5323,7 @@ nfsrpc_fillsa(struct nfsmount *nmp, struct sockaddr_st
 	nrp->nr_sotype = SOCK_STREAM;
 	mtx_init(&nrp->nr_mtx, "nfssock", NULL, MTX_DEF);
 	nrp->nr_prog = NFS_PROG;
-	nrp->nr_vers = NFS_VER4;
+	nrp->nr_vers = vers;
 
 	/*
 	 * Use the credentials that were used for the mount, which are



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201709282233.v8SMX2b3092271>