[asterisk-commits] rizzo: branch rizzo/astobj2 r47357 - /team/rizzo/astobj2/channels/chan_sip.c

[asterisk-commits at lists.digium.com]

Author: rizzo
Date: Thu Nov  9 04:55:16 2006
New Revision: 47357

URL: http://svn.digium.com/view/asterisk?view=rev&rev=47357
Log:
in case of excessive retries, sipsock_read() may risk to
dereference a pointer to a pvt that has already gone away.
Rearrange the code to handle the case properly.


Modified:
    team/rizzo/astobj2/channels/chan_sip.c

Modified: team/rizzo/astobj2/channels/chan_sip.c
URL: http://svn.digium.com/view/asterisk/team/rizzo/astobj2/channels/chan_sip.c?view=diff&rev=47357&r1=47356&r2=47357
==============================================================================
--- team/rizzo/astobj2/channels/chan_sip.c (original)
+++ team/rizzo/astobj2/channels/chan_sip.c Thu Nov  9 04:55:16 2006
@@ -4483,8 +4483,11 @@
 	return found;
 }
 
-/*! \brief Connect incoming SIP message to current dialog or create new dialog structure
-	Called by handle_request, sipsock_read */
+/*!
+ * Connect incoming SIP message to current dialog or create new dialog structure
+ * Returns a reference to the sip_pvt object, remember to give it back once done (done)
+ *	Called by handle_request, sipsock_read
+ */
 static struct sip_pvt *find_call(struct sip_request *req, struct sockaddr_in *sin, const int intended_method)
 {
 	static int profile_id = -1;
@@ -4584,7 +4587,7 @@
 	if (option_debug > 1 && intended_method == SIP_RESPONSE)
 		ast_log(LOG_DEBUG, "That's odd...  Got a response on a call we dont know about. Callid %s\n", callid ? callid : "<unknown>");
 
-	return p;
+	return NULL;
 }
 
 /*! \brief Parse register=> line in sip.conf and add to registry */
@@ -14978,22 +14981,25 @@
 		if (option_debug)
 			ast_log(LOG_DEBUG, "Failed to grab owner channel lock, trying again. (SIP call %s)\n", p->callid);
 		sip_pvt_unlock(p);
+		pvt_unref(p);	/* release the reference */
+		p = NULL;	/* because we don't have a good value anymore */
 		/* Sleep for a very short amount of time */
 		usleep(1);
 	}
-	p->recv = sin;
-
-	if (!ast_test_flag(&p->flags[0], SIP_NO_HISTORY)) /* This is a request or response, note what it was for */
-		append_history(p, "Rx", "%s / %s / %s", req.data, get_header(&req, "CSeq"), req.rlPart2);
-
 	if (!lockretry) {
-		ast_log(LOG_ERROR, "We could NOT get the channel lock for %s! \n", S_OR(p->owner->name, "- no channel name ??? - "));
-		ast_log(LOG_ERROR, "SIP transaction failed: %s \n", p->callid);
+		/* we cannot trust p, we have no reference */
+		ast_log(LOG_ERROR, "We could NOT get the channel lock, SIP transaction failed: %s\n",
+			get_header(&req, "Call-ID"));
 		transmit_response(p, "503 Server error", &req);	/* We must respond according to RFC 3261 sec 12.2 */
 		/* XXX We could add retry-after to make sure they come back */
 		append_history(p, "LockFail", "Owner lock failed, transaction failed.");
 		return 1;
 	}
+	p->recv = sin;
+
+	if (!ast_test_flag(&p->flags[0], SIP_NO_HISTORY)) /* This is a request or response, note what it was for */
+		append_history(p, "Rx", "%s / %s / %s", req.data, get_header(&req, "CSeq"), req.rlPart2);
+
 	nounlock = 0;
 	if (handle_request(p, &req, &sin, &recount, &nounlock) == -1) {
 		/* Request failed */
@@ -15004,6 +15010,7 @@
 	if (p->owner && !nounlock)
 		ast_channel_unlock(p->owner);
 	sip_pvt_unlock(p);
+	pvt_unref(p);
 	if (recount)
 		ast_update_use_count();