changeset 11380:73d454719071

6900751 Corrupt call_table / callist structure leads to networking hang
author Marcel Telka <Marcel.Telka@Sun.COM>
date Tue, 22 Dec 2009 17:10:31 +0100
parents 752a9bf31c52
children c77e4d2b2e75
files usr/src/uts/common/rpc/clnt.h usr/src/uts/common/rpc/clnt_clts.c usr/src/uts/common/rpc/clnt_cots.c
diffstat 3 files changed, 54 insertions(+), 110 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/uts/common/rpc/clnt.h	Tue Dec 22 10:52:46 2009 +0100
+++ b/usr/src/uts/common/rpc/clnt.h	Tue Dec 22 17:10:31 2009 +0100
@@ -19,7 +19,7 @@
  * CDDL HEADER END
  */
 /*
- * Copyright 2008 Sun Microsystems, Inc.  All rights reserved.
+ * Copyright 2009 Sun Microsystems, Inc.  All rights reserved.
  * Use is subject to license terms.
  */
 /* Copyright (c) 1983, 1984, 1985, 1986, 1987, 1988, 1989 AT&T */
@@ -291,36 +291,36 @@
 
 #define	call_table_enter(e)				\
 {							\
-	call_table_t *ctp = e->call_bucket;		\
+	call_table_t *ctp = (e)->call_bucket;		\
 	mutex_enter(&ctp->ct_lock);			\
 	ctp->ct_len++;					\
-	e->call_next = (calllist_t *)ctp->ct_call_next;	\
-	e->call_prev = (calllist_t *)ctp;		\
-	((call_table_t *)ctp->ct_call_next)->ct_call_prev = e;		\
-	ctp->ct_call_next = e;				\
-	mutex_exit(&e->call_bucket->ct_lock);		\
+	(e)->call_next = ctp->ct_call_next;		\
+	(e)->call_prev = (calllist_t *)ctp;		\
+	ctp->ct_call_next->call_prev = (e);		\
+	ctp->ct_call_next = (e);			\
+	mutex_exit(&ctp->ct_lock);			\
 }
 
 #define	call_table_remove(e)				\
 {							\
-	call_table_t *ctp = e->call_bucket;		\
+	call_table_t *ctp = (e)->call_bucket;		\
 	mutex_enter(&ctp->ct_lock);			\
 	ctp->ct_len--;					\
-	((call_table_t *)e->call_prev)->ct_call_next = e->call_next;	\
-	((call_table_t *)e->call_next)->ct_call_prev = e->call_prev;	\
+	(e)->call_prev->call_next = (e)->call_next;	\
+	(e)->call_next->call_prev = (e)->call_prev;	\
 	mutex_exit(&ctp->ct_lock);			\
 }
 
 #define	call_table_find(ctp, xid, ele)			\
 {							\
 	calllist_t *cp;					\
-	ele = NULL;					\
+	(ele) = NULL;					\
 	mutex_enter(&(ctp)->ct_lock);			\
 	for (cp = (ctp)->ct_call_next;			\
-		cp != (calllist_t *)ctp;		\
+		cp != (calllist_t *)(ctp);		\
 		cp = cp->call_next) {			\
-		if (cp->call_xid == xid)		\
-			ele = cp;			\
+		if (cp->call_xid == (xid))		\
+			(ele) = cp;			\
 	}						\
 }
 
--- a/usr/src/uts/common/rpc/clnt_clts.c	Tue Dec 22 10:52:46 2009 +0100
+++ b/usr/src/uts/common/rpc/clnt_clts.c	Tue Dec 22 17:10:31 2009 +0100
@@ -463,7 +463,6 @@
 	int refreshes = REFRESHES;	/* number of times to refresh cred */
 	int round_trip;			/* time the RPC */
 	int error;
-	int hdrsz;
 	mblk_t *mp;
 	mblk_t *mpdup;
 	mblk_t *resp = NULL;
@@ -661,6 +660,7 @@
 	 * will set this to true again.
 	 */
 	call->call_notified = FALSE;
+	call->call_status = RPC_TIMEDOUT;
 	mutex_exit(&call->call_lock);
 
 	if (status == RPC_TIMEDOUT) {
@@ -672,25 +672,6 @@
 			p->cku_err.re_errno = EINTR;
 			goto done1;
 		} else {
-			/*
-			 * It's possible that our response arrived
-			 * right after we timed out.  Check to see
-			 * if it has arrived before we remove the
-			 * calllist from the dispatch queue.
-			 */
-			mutex_enter(&call->call_lock);
-			if (call->call_notified == TRUE) {
-				resp = call->call_reply;
-				call->call_reply = NULL;
-				mutex_exit(&call->call_lock);
-				RPCLOG(8, "clnt_clts_kcallit_addr: "
-				    "response received for request "
-				    "w/xid 0x%x after timeout\n",
-				    p->cku_xid);
-				goto getresponse;
-			}
-			mutex_exit(&call->call_lock);
-
 			RPCLOG(8, "clnt_clts_kcallit_addr: "
 			    "request w/xid 0x%x timedout "
 			    "waiting for reply\n", p->cku_xid);
@@ -714,20 +695,7 @@
 		}
 	}
 
-getresponse:
-	/*
-	 * Check to see if a response arrived.  If it one is
-	 * present then proceed to process the reponse.  Otherwise
-	 * fall through to retry or retransmit the request.  This
-	 * is probably not the optimal thing to do, but since we
-	 * are most likely dealing with a unrealiable transport it
-	 * is the safe thing to so.
-	 */
-	if (resp == NULL) {
-		p->cku_err.re_status = RPC_CANTRECV;
-		p->cku_err.re_errno = EIO;
-		goto done1;
-	}
+	ASSERT(resp != NULL);
 
 	/*
 	 * Prepare the message for further processing.  We need to remove
@@ -748,28 +716,12 @@
 
 	/*
 	 * Pop off the datagram header.
+	 * It was retained in rpcmodrput().
 	 */
-	hdrsz = resp->b_wptr - resp->b_rptr;
-	if ((resp->b_wptr - (resp->b_rptr + hdrsz)) == 0) {
-		tmp = resp;
-		resp = resp->b_cont;
-		tmp->b_cont = NULL;
-		freeb(tmp);
-	} else {
-		unsigned char *ud_off = resp->b_rptr;
-		resp->b_rptr += hdrsz;
-		tmp = dupb(resp);
-		if (tmp == NULL) {
-			p->cku_err.re_status = RPC_SYSTEMERROR;
-			p->cku_err.re_errno = ENOSR;
-			freemsg(resp);
-			goto done1;
-		}
-		tmp->b_cont = resp->b_cont;
-		resp->b_rptr = ud_off;
-		freeb(resp);
-		resp = tmp;
-	}
+	tmp = resp;
+	resp = resp->b_cont;
+	tmp->b_cont = NULL;
+	freeb(tmp);
 
 	round_trip = ddi_get_lbolt() - round_trip;
 	/*
@@ -871,6 +823,14 @@
 		goto tryread;
 	}
 	if (re_status == RPC_AUTHERROR) {
+
+		(void) xdr_rpc_free_verifier(xdrs, &reply_msg);
+		call_table_remove(call);
+		if (call->call_reply != NULL) {
+			freemsg(call->call_reply);
+			call->call_reply = NULL;
+		}
+
 		/*
 		 * Maybe our credential need to be refreshed
 		 */
@@ -886,18 +846,10 @@
 			RCSTAT_INCR(p->cku_stats, rcbadcalls);
 			RCSTAT_INCR(p->cku_stats, rcnewcreds);
 
-			(void) xdr_rpc_free_verifier(xdrs, &reply_msg);
 			freemsg(mpdup);
-			call_table_remove(call);
-			mutex_enter(&call->call_lock);
-			if (call->call_reply != NULL) {
-				freemsg(call->call_reply);
-				call->call_reply = NULL;
-			}
-			mutex_exit(&call->call_lock);
-
+			mpdup = NULL;
 			freemsg(resp);
-			mpdup = NULL;
+			resp = NULL;
 			goto call_again;
 		}
 		/*
@@ -919,19 +871,12 @@
 			 */
 			if (p->cku_useresvport != 1) {
 				p->cku_useresvport = 1;
-				(void) xdr_rpc_free_verifier(xdrs, &reply_msg);
-				freemsg(mpdup);
 
-				call_table_remove(call);
-				mutex_enter(&call->call_lock);
-				if (call->call_reply != NULL) {
-					freemsg(call->call_reply);
-					call->call_reply = NULL;
-				}
-				mutex_exit(&call->call_lock);
+				freemsg(mpdup);
+				mpdup = NULL;
+				freemsg(resp);
+				resp = NULL;
 
-				freemsg(resp);
-				mpdup = NULL;
 				endpt = p->cku_endpnt;
 				if (endpt->e_tiptr != NULL) {
 					mutex_enter(&endpt->e_lock);
@@ -965,18 +910,17 @@
 		RPCLOG(1, "clnt_clts_kcallit : authentication failed "
 		    "with RPC_AUTHERROR of type %d\n",
 		    p->cku_err.re_why);
+		goto done;
 	}
 
 	(void) xdr_rpc_free_verifier(xdrs, &reply_msg);
 
 done1:
 	call_table_remove(call);
-	mutex_enter(&call->call_lock);
 	if (call->call_reply != NULL) {
 		freemsg(call->call_reply);
 		call->call_reply = NULL;
 	}
-	mutex_exit(&call->call_lock);
 	RPCLOG(64, "clnt_clts_kcallit_addr: xid 0x%x taken off dispatch list",
 	    p->cku_xid);
 
--- a/usr/src/uts/common/rpc/clnt_cots.c	Tue Dec 22 10:52:46 2009 +0100
+++ b/usr/src/uts/common/rpc/clnt_cots.c	Tue Dec 22 17:10:31 2009 +0100
@@ -1400,29 +1400,29 @@
 					cm_entry = NULL;
 				}
 
+				(void) xdr_rpc_free_verifier(xdrs,
+				    &reply_msg);
+
+				if (p->cku_flags & CKU_ONQUEUE) {
+					call_table_remove(call);
+					p->cku_flags &= ~CKU_ONQUEUE;
+				}
+				RPCLOG(64,
+				    "clnt_cots_kcallit: AUTH_ERROR, xid"
+				    " 0x%x removed off dispatch list\n",
+				    p->cku_xid);
+				if (call->call_reply) {
+					freemsg(call->call_reply);
+					call->call_reply = NULL;
+				}
+
 				if ((refreshes > 0) &&
 				    AUTH_REFRESH(h->cl_auth, &reply_msg,
 				    p->cku_cred)) {
 					refreshes--;
-					(void) xdr_rpc_free_verifier(xdrs,
-					    &reply_msg);
 					freemsg(mp);
 					mp = NULL;
 
-					if (p->cku_flags & CKU_ONQUEUE) {
-						call_table_remove(call);
-						p->cku_flags &= ~CKU_ONQUEUE;
-					}
-
-					RPCLOG(64,
-					    "clnt_cots_kcallit: AUTH_ERROR, xid"
-					    " 0x%x removed off dispatch list\n",
-					    p->cku_xid);
-					if (call->call_reply) {
-						freemsg(call->call_reply);
-						call->call_reply = NULL;
-					}
-
 					COTSRCSTAT_INCR(p->cku_stats,
 					    rcbadcalls);
 					COTSRCSTAT_INCR(p->cku_stats,
@@ -1455,9 +1455,8 @@
 					if (p->cku_useresvport != 1) {
 						p->cku_useresvport = 1;
 						p->cku_xid = 0;
-						(void) xdr_rpc_free_verifier
-						    (xdrs, &reply_msg);
 						freemsg(mp);
+						mp = NULL;
 						goto call_again;
 					}
 					/* FALLTHRU */
@@ -1477,6 +1476,7 @@
 				RPCLOG(1, "clnt_cots_kcallit : authentication"
 				    " failed with RPC_AUTHERROR of type %d\n",
 				    (int)p->cku_err.re_why);
+				goto cots_done;
 			}
 		}
 	} else {