[asterisk-bugs] [JIRA] (ASTERISK-25645) res_rtp_asterisk: Lock inversion

[Dade Brandon (JIRA)]

    [ https://issues.asterisk.org/jira/browse/ASTERISK-25645?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=232337#comment-232337 ] 

Dade Brandon commented on ASTERISK-25645:
-----------------------------------------

@one47
Hi Steve,

If that patch is working, is there any chance of submitting it to pjproject, or maybe in to the bundled pjproject? (I think @jcolp could confirm if we can patch the bundled pjproject directly in the asterisk repository, or if alternatively it has to go through github.com/asterisk/pjproject -- I wasn't even aware that Asterisk 13 still had a bundled copy of pjproject.) 

The pjproject bundled with asterisk-11, which doesn't use group locks, looks like this:

```
            /* Release mutex in case app destroy us in the callback */
            ice_status = ice->ice_status;
            on_ice_complete = ice->cb.on_ice_complete;
            has_mutex = PJ_FALSE;
            pj_mutex_unlock(ice->mutex);

            /* Notify app about ICE completion*/
            if (on_ice_complete)
                (*on_ice_complete)(ice, ice_status);
```

lower down in this function, it does not re-acquire ice->mutex, so I imagine that your patch, which released but does not re-acquire, is also correct, although I don't fully understand the differences between the group lock ice->mutex lock.  Before looking at the older pjproject code, I assumed that your suggested pjproject patch should have been first checking to see if the lock had already been acquired before releasing, and then also re-acquiring after the on_ice_complete callback (which may still be the case, but I doubt it..?)

> res_rtp_asterisk: Lock inversion
> --------------------------------
>
>                 Key: ASTERISK-25645
>                 URL: https://issues.asterisk.org/jira/browse/ASTERISK-25645
>             Project: Asterisk
>          Issue Type: Bug
>      Security Level: None
>          Components: Resources/res_rtp_asterisk
>            Reporter: Steve Davies
>         Attachments: deadlocked_threads.txt, experimental_anti_deadlock, unlock_ice_before_callback
>
>
> Reported by Steve Davies on asterisk-dev:
> commit 5e6b1476a087407a052f007d326c504cfeefebe7
> ASTERISK-25614
> 2 code paths which approximate the following will cause a lock-inversion deadlock:
> approximate call orders are:
> a)
> pj_timer_heap_poll (PJ_LOCK)
> ast_rtp_on_ice_complete
> ast_rtp_instance_set_remote_address
> remote_address_set
> ast_rtp_remote_address_set
> (DTLS_LOCK)
> ...
> b)
> ast_pbx...
> app_dial
> bridge...
> read
> rtp_read
> ...
> __rtp_recvfrom
> (DTLS_LOCK)
> dtls_srtp_check_pending
> __rtp_sendto
> pj_ice_sess_send_data
> (PJ_LOCK)



--
This message was sent by Atlassian JIRA
(v6.2#6252)