Skip to content

Commit

Permalink
rpmsg: add incref_cb and decref_cb in endpoint to fix ept used-after-…
Browse files Browse the repository at this point in the history
…free

if rpmsg service free the ept when has got the ept from the ept
list in rpmsg_virtio_rx_callback, there is a used after free about
the ept, so add incref_cb and decref_cb to endpoint.
inform the user that the endpoint allocation should be referenced or
be safely free by callback.

Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>
  • Loading branch information
CV-Bowen authored and Tao Yin committed Oct 21, 2023
1 parent de33a8d commit f1d9b00
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 0 deletions.
11 changes: 11 additions & 0 deletions lib/include/openamp/rpmsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ struct rpmsg_device;
/* Returns positive value on success or negative error value on failure */
typedef int (*rpmsg_ept_cb)(struct rpmsg_endpoint *ept, void *data,
size_t len, uint32_t src, void *priv);
typedef void (*rpmsg_ept_incref_cb)(void *priv);
typedef void (*rpmsg_ept_decref_cb)(void *priv);
typedef void (*rpmsg_ns_unbind_cb)(struct rpmsg_endpoint *ept);
typedef void (*rpmsg_ns_bind_cb)(struct rpmsg_device *rdev,
const char *name, uint32_t dest);
Expand All @@ -73,6 +75,15 @@ struct rpmsg_endpoint {
/** Address of the default remote endpoint binded */
uint32_t dest_addr;

/** Callback to inform the user that the endpoint allocation should be referenced */
rpmsg_ept_incref_cb incref_cb;

/**
* Callback to inform the user that the endpoint allocation can be decremented
* or removed safely.
*/
rpmsg_ept_decref_cb decref_cb;

/**
* User rx callback, return value of this callback is reserved for future
* use, for now, only allow RPMSG_SUCCESS as return value
Expand Down
4 changes: 4 additions & 0 deletions lib/rpmsg/rpmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,8 @@ static void rpmsg_unregister_endpoint(struct rpmsg_endpoint *ept)
ept->addr);
metal_list_del(&ept->node);
ept->rdev = NULL;
if (ept && ept->decref_cb)
ept->decref_cb(ept->priv);
metal_mutex_release(&rdev->lock);
}

Expand All @@ -256,6 +258,8 @@ void rpmsg_register_endpoint(struct rpmsg_device *rdev,
rpmsg_ept_cb cb,
rpmsg_ns_unbind_cb ns_unbind_cb)
{
if (ept && ept->incref_cb)
ept->incref_cb(ept->priv);
strncpy(ept->name, name ? name : "", sizeof(ept->name));
ept->addr = src;
ept->dest_addr = dest;
Expand Down
15 changes: 15 additions & 0 deletions lib/rpmsg/rpmsg_virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,8 @@ static void rpmsg_virtio_rx_callback(struct virtqueue *vq)
/* Get the channel node from the remote device channels list. */
metal_mutex_acquire(&rdev->lock);
ept = rpmsg_get_ept_from_addr(rdev, rp_hdr->dst);
if (ept && ept->incref_cb)
ept->incref_cb(ept->priv);
metal_mutex_release(&rdev->lock);

if (ept) {
Expand All @@ -576,6 +578,8 @@ static void rpmsg_virtio_rx_callback(struct virtqueue *vq)
}

metal_mutex_acquire(&rdev->lock);
if (ept && ept->decref_cb)
ept->decref_cb(ept->priv);

/* Check whether callback wants to hold buffer */
if (!(rp_hdr->reserved & RPMSG_BUF_HELD)) {
Expand Down Expand Up @@ -616,6 +620,7 @@ static int rpmsg_virtio_ns_callback(struct rpmsg_endpoint *ept, void *data,
struct rpmsg_ns_msg *ns_msg;
uint32_t dest;
char name[RPMSG_NAME_SIZE];
bool ept_decref;

(void)priv;
(void)src;
Expand All @@ -636,11 +641,21 @@ static int rpmsg_virtio_ns_callback(struct rpmsg_endpoint *ept, void *data,
if (ns_msg->flags & RPMSG_NS_DESTROY) {
if (_ept)
_ept->dest_addr = RPMSG_ADDR_ANY;
if (_ept && _ept->incref_cb) {
_ept->incref_cb(_ept->priv);
ept_decref = true;
}
metal_mutex_release(&rdev->lock);
if (_ept && _ept->ns_unbind_cb)
_ept->ns_unbind_cb(_ept);
if (rdev->ns_unbind_cb)
rdev->ns_unbind_cb(rdev, name, dest);
if (ept_decref) {
metal_mutex_acquire(&rdev->lock);
if (_ept && _ept->decref_cb)
_ept->decref_cb(_ept->priv);
metal_mutex_release(&rdev->lock);
}
} else {
if (!_ept) {
/*
Expand Down

0 comments on commit f1d9b00

Please sign in to comment.