mirror of
				https://github.com/asterisk/asterisk.git
				synced 2025-10-31 18:55:19 +00:00 
			
		
		
		
	Prevent BLF subscriptions from causing deadlocks
Fix a locking inversion in sip_send_mwi_to_peer that was causing deadlocks. This function now requires that both the peer and associated pvt be unlocked before it is called for cases where peer and peer->mwipvt form a circular reference. (closes issue ASTERISK-18663) Review: https://reviewboard.asterisk.org/r/1563/ git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@343621 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
		| @@ -24345,7 +24345,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, | |||||||
| 
 | 
 | ||||||
| 		p->subscribed = MWI_NOTIFICATION; | 		p->subscribed = MWI_NOTIFICATION; | ||||||
| 		if (ast_test_flag(&authpeer->flags[1], SIP_PAGE2_SUBSCRIBEMWIONLY)) { | 		if (ast_test_flag(&authpeer->flags[1], SIP_PAGE2_SUBSCRIBEMWIONLY)) { | ||||||
|  | 			ao2_unlock(p); | ||||||
| 			add_peer_mwi_subs(authpeer); | 			add_peer_mwi_subs(authpeer); | ||||||
|  | 			ao2_lock(p); | ||||||
| 		} | 		} | ||||||
| 		if (authpeer->mwipvt && authpeer->mwipvt != p) {	/* Destroy old PVT if this is a new one */ | 		if (authpeer->mwipvt && authpeer->mwipvt != p) {	/* Destroy old PVT if this is a new one */ | ||||||
| 			/* We only allow one subscription per peer */ | 			/* We only allow one subscription per peer */ | ||||||
| @@ -24421,7 +24423,12 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, | |||||||
| 			ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED); | 			ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED); | ||||||
| 			transmit_response(p, "200 OK", req); | 			transmit_response(p, "200 OK", req); | ||||||
| 			if (p->relatedpeer) {	/* Send first notification */ | 			if (p->relatedpeer) {	/* Send first notification */ | ||||||
| 				sip_send_mwi_to_peer(p->relatedpeer, 0); | 				struct sip_peer *peer = p->relatedpeer; | ||||||
|  | 				ref_peer(peer, "ensure a peer ref is held during MWI sending"); | ||||||
|  | 				ao2_unlock(p); | ||||||
|  | 				sip_send_mwi_to_peer(peer, 0); | ||||||
|  | 				ao2_lock(p); | ||||||
|  | 				unref_peer(peer, "release a peer ref now that MWI is sent"); | ||||||
| 			} | 			} | ||||||
| 		} else if (p->subscribed != CALL_COMPLETION) { | 		} else if (p->subscribed != CALL_COMPLETION) { | ||||||
| 			if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) { | 			if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) { | ||||||
| @@ -25132,36 +25139,49 @@ static int get_cached_mwi(struct sip_peer *peer, int *new, int *old) | |||||||
| 	return in_cache; | 	return in_cache; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| /*! \brief Send message waiting indication to alert peer that they've got voicemail */ | /*! \brief Send message waiting indication to alert peer that they've got voicemail
 | ||||||
|  |  *  \note Both peer and associated sip_pvt must be unlocked prior to calling this function | ||||||
|  | */ | ||||||
| static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) | static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) | ||||||
| { | { | ||||||
| 	/* Called with peerl lock, but releases it */ | 	/* Called with peerl lock, but releases it */ | ||||||
| 	struct sip_pvt *p; | 	struct sip_pvt *p; | ||||||
| 	int newmsgs = 0, oldmsgs = 0; | 	int newmsgs = 0, oldmsgs = 0; | ||||||
|  | 	const char *vmexten; | ||||||
| 
 | 
 | ||||||
| 	if (ast_test_flag((&peer->flags[1]), SIP_PAGE2_SUBSCRIBEMWIONLY) && !peer->mwipvt) | 	ao2_lock(peer); | ||||||
|  | 
 | ||||||
|  | 	vmexten = ast_strdupa(peer->vmexten); | ||||||
|  | 
 | ||||||
|  | 	if (ast_test_flag((&peer->flags[1]), SIP_PAGE2_SUBSCRIBEMWIONLY) && !peer->mwipvt) { | ||||||
|  | 		ao2_unlock(peer); | ||||||
| 		return 0; | 		return 0; | ||||||
|  | 	} | ||||||
| 
 | 
 | ||||||
| 	/* Do we have an IP address? If not, skip this peer */ | 	/* Do we have an IP address? If not, skip this peer */ | ||||||
| 	if (ast_sockaddr_isnull(&peer->addr) && ast_sockaddr_isnull(&peer->defaddr)) | 	if (ast_sockaddr_isnull(&peer->addr) && ast_sockaddr_isnull(&peer->defaddr)) { | ||||||
|  | 		ao2_unlock(peer); | ||||||
| 		return 0; | 		return 0; | ||||||
|  | 	} | ||||||
| 
 | 
 | ||||||
| 	/* Attempt to use cached mwi to get message counts. */ | 	/* Attempt to use cached mwi to get message counts. */ | ||||||
| 	if (!get_cached_mwi(peer, &newmsgs, &oldmsgs) && !cache_only) { | 	if (!get_cached_mwi(peer, &newmsgs, &oldmsgs) && !cache_only) { | ||||||
| 		/* Fall back to manually checking the mailbox if not cache_only and get_cached_mwi failed */ | 		/* Fall back to manually checking the mailbox if not cache_only and get_cached_mwi failed */ | ||||||
| 		struct ast_str *mailbox_str = ast_str_alloca(512); | 		struct ast_str *mailbox_str = ast_str_alloca(512); | ||||||
| 		peer_mailboxes_to_str(&mailbox_str, peer); | 		peer_mailboxes_to_str(&mailbox_str, peer); | ||||||
|  | 		ao2_unlock(peer); | ||||||
| 		ast_app_inboxcount(mailbox_str->str, &newmsgs, &oldmsgs); | 		ast_app_inboxcount(mailbox_str->str, &newmsgs, &oldmsgs); | ||||||
|  | 		ao2_lock(peer); | ||||||
| 	} | 	} | ||||||
| 	ao2_lock(peer); |  | ||||||
| 
 | 
 | ||||||
| 	if (peer->mwipvt) { | 	if (peer->mwipvt) { | ||||||
| 		/* Base message on subscription */ | 		/* Base message on subscription */ | ||||||
| 		p = dialog_ref(peer->mwipvt, "sip_send_mwi_to_peer: Setting dialog ptr p from peer->mwipvt-- should this be done?"); | 		p = dialog_ref(peer->mwipvt, "sip_send_mwi_to_peer: Setting dialog ptr p from peer->mwipvt"); | ||||||
|  | 		ao2_unlock(peer); | ||||||
| 	} else { | 	} else { | ||||||
|  | 		ao2_unlock(peer); | ||||||
| 		/* Build temporary dialog for this message */ | 		/* Build temporary dialog for this message */ | ||||||
| 		if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY, NULL))) { | 		if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY, NULL))) { | ||||||
| 			ao2_unlock(peer); |  | ||||||
| 			return -1; | 			return -1; | ||||||
| 		} | 		} | ||||||
| 
 | 
 | ||||||
| @@ -25175,7 +25195,6 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) | |||||||
| 			dialog_unlink_all(p); | 			dialog_unlink_all(p); | ||||||
| 			dialog_unref(p, "unref dialog p just created via sip_alloc"); | 			dialog_unref(p, "unref dialog p just created via sip_alloc"); | ||||||
| 			/* sip_destroy(p); */ | 			/* sip_destroy(p); */ | ||||||
| 			ao2_unlock(peer); |  | ||||||
| 			return 0; | 			return 0; | ||||||
| 		} | 		} | ||||||
| 		/* Recalculate our side, and recalculate Call ID */ | 		/* Recalculate our side, and recalculate Call ID */ | ||||||
| @@ -25183,11 +25202,15 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) | |||||||
| 		build_via(p); | 		build_via(p); | ||||||
| 		ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name"); | 		ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name"); | ||||||
| 		build_callid_pvt(p); | 		build_callid_pvt(p); | ||||||
|  | 
 | ||||||
|  | 		ao2_lock(peer); | ||||||
| 		if (!ast_strlen_zero(peer->mwi_from)) { | 		if (!ast_strlen_zero(peer->mwi_from)) { | ||||||
| 			ast_string_field_set(p, mwi_from, peer->mwi_from); | 			ast_string_field_set(p, mwi_from, peer->mwi_from); | ||||||
| 		} else if (!ast_strlen_zero(default_mwi_from)) { | 		} else if (!ast_strlen_zero(default_mwi_from)) { | ||||||
| 			ast_string_field_set(p, mwi_from, default_mwi_from); | 			ast_string_field_set(p, mwi_from, default_mwi_from); | ||||||
| 		} | 		} | ||||||
|  | 		ao2_unlock(peer); | ||||||
|  | 
 | ||||||
| 		ao2_t_link(dialogs, p, "Linking in under new name"); | 		ao2_t_link(dialogs, p, "Linking in under new name"); | ||||||
| 		/* Destroy this session after 32 secs */ | 		/* Destroy this session after 32 secs */ | ||||||
| 		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); | 		sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); | ||||||
| @@ -25200,10 +25223,10 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) | |||||||
| 	/* Send MWI */ | 	/* Send MWI */ | ||||||
| 	ast_set_flag(&p->flags[0], SIP_OUTGOING); | 	ast_set_flag(&p->flags[0], SIP_OUTGOING); | ||||||
| 	/* the following will decrement the refcount on p as it finishes */ | 	/* the following will decrement the refcount on p as it finishes */ | ||||||
| 	transmit_notify_with_mwi(p, newmsgs, oldmsgs, peer->vmexten); | 	transmit_notify_with_mwi(p, newmsgs, oldmsgs, vmexten); | ||||||
| 	sip_pvt_unlock(p); | 	sip_pvt_unlock(p); | ||||||
| 	dialog_unref(p, "unref dialog ptr p just before it goes out of scope at the end of sip_send_mwi_to_peer."); | 	dialog_unref(p, "unref dialog ptr p just before it goes out of scope at the end of sip_send_mwi_to_peer."); | ||||||
| 	ao2_unlock(peer); | 
 | ||||||
| 	return 0; | 	return 0; | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user