mgcp: fix use-after-free and add callback for endpoint cleanup

Since we will support multiple different types of endpoints in the
future, all these endpoints will handle connections slightly different
and there will be possibly state that needs to be kept consistant
when a connection is deleted.

In mgcp_network.c where we implement the callback that is used to
create an rtp-bride-endpoint. In that callback we cache the pointer
of the connection we where we want to bride to (opposite connection).
When one of the connections is deleted using a DLCX operation, the
pointer is still there and the next incoming packet causes a use-
after-free segfault.

- introduce an endpoint specific callback function that is executed
  before removing the connection.

- implement the endpoint specific callback for rtp bridge endpoints,
  so that the use-after-free is prevented.

Change-Id: I921d9bbe58be1c3298e164a37f3c974880b3759f
This commit is contained in:
Philipp Maier
2018-01-24 11:39:32 +01:00
committed by Harald Welte
parent 5656fbf49d
commit df5d219f39
5 changed files with 38 additions and 1 deletions

View File

@@ -33,6 +33,13 @@ typedef int (*mgcp_dispatch_rtp_cb) (int proto, struct sockaddr_in *addr,
char *buf, unsigned int buf_size,
struct mgcp_conn *conn);
/* Callback type for endpoint specific cleanup actions. This function
* is automatically executed when a connection is freed (see mgcp_conn_free()
* in mgcp_conn.c). Depending on the type of the endpoint there may be endpoint
* specific things to take care of once a connection has been removed. */
typedef void (*mgcp_cleanup_cp) (struct mgcp_endpoint *endp,
struct mgcp_conn *conn);
/*! MGCP endpoint properties */
struct mgcp_endpoint_type {
/*!< maximum number of connections */
@@ -40,6 +47,9 @@ struct mgcp_endpoint_type {
/*!< callback that defines how to dispatch incoming RTP data */
mgcp_dispatch_rtp_cb dispatch_rtp_cb;
/*!< callback that implements endpoint specific cleanup actions */
mgcp_cleanup_cp cleanup_cb;
};
/*! MGCP endpoint typeset */

View File

@@ -265,6 +265,7 @@ int mgcp_send(struct mgcp_endpoint *endp, int is_rtp, struct sockaddr_in *addr,
int mgcp_send_dummy(struct mgcp_endpoint *endp, struct mgcp_conn_rtp *conn);
int mgcp_dispatch_rtp_bridge_cb(int proto, struct sockaddr_in *addr, char *buf,
unsigned int buf_size, struct mgcp_conn *conn);
void mgcp_cleanup_rtp_bridge_cb(struct mgcp_endpoint *endp, struct mgcp_conn *conn);
int mgcp_bind_net_rtp_port(struct mgcp_endpoint *endp, int rtp_port,
struct mgcp_conn_rtp *conn);
void mgcp_free_rtp_port(struct mgcp_rtp_end *end);

View File

@@ -201,6 +201,12 @@ void mgcp_conn_free(struct mgcp_endpoint *endp, const char *id)
if (!conn)
return;
/* Run endpoint cleanup action. By this we inform the endpoint about
* the removal of the connection and allow it to clean up its inner
* state accordingly */
if (endp->type->cleanup_cb)
endp->type->cleanup_cb(endp, conn);
switch (conn->type) {
case MGCP_CONN_TYPE_RTP:
osmux_disable_conn(&conn->u.rtp);

View File

@@ -28,7 +28,8 @@
const struct mgcp_endpoint_typeset ep_typeset = {
/* Specify endpoint properties for RTP endpoint */
.rtp.max_conns = 2,
.rtp.dispatch_rtp_cb = mgcp_dispatch_rtp_bridge_cb
.rtp.dispatch_rtp_cb = mgcp_dispatch_rtp_bridge_cb,
.rtp.cleanup_cb = mgcp_cleanup_rtp_bridge_cb
};
/*! release endpoint, all open connections are closed.

View File

@@ -1043,6 +1043,25 @@ int mgcp_dispatch_rtp_bridge_cb(int proto, struct sockaddr_in *addr, char *buf,
}
/*! cleanup an endpoint when a connection on an RTP bridge endpoint is removed.
* \param[in] endp Endpoint on which the connection resides.
* \param[in] conn Connection that is about to be removed (ignored).
* \returns 0 on success, -1 on ERROR. */
void mgcp_cleanup_rtp_bridge_cb(struct mgcp_endpoint *endp, struct mgcp_conn *conn)
{
struct mgcp_conn *conn_cleanup;
/* In mgcp_dispatch_rtp_bridge_cb() we use conn->priv to cache the
* pointer to the destination connection, so that we do not have
* to go through the list every time an RTP packet arrives. To prevent
* a use-after-free situation we invalidate this information for all
* connections present when one connection is removed from the
* endpoint. */
llist_for_each_entry(conn_cleanup, &endp->conns, entry) {
conn_cleanup->priv = NULL;
}
}
/* Handle incoming RTP data from NET */
static int rtp_data_net(struct osmo_fd *fd, unsigned int what)
{