Skip to content

Commit

Permalink
rpmsg: add release cb and refcnt in end pointto fix ept used-after-free
Browse files Browse the repository at this point in the history
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 refcnt to end point and call the rpmsg service
release callback when ept callback fininshed.

Signed-off-by: yintao <yintao@xiaomi.com>
Signed-off-by: Bowen Wang <wangbowen6@xiaomi.com>
  • Loading branch information
CV-Bowen authored and yintao committed Oct 10, 2023
1 parent 937ba1d commit b90441b
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 1 deletion.
22 changes: 22 additions & 0 deletions lib/include/openamp/rpmsg.h
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ 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_release_cb)(struct rpmsg_endpoint *ept);
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 +74,12 @@ struct rpmsg_endpoint {
/** Address of the default remote endpoint binded */
uint32_t dest_addr;

/** Reference count of the endpoint */
atomic_uint refcnt;

/** Callback to free the endpoint */
rpmsg_ept_release_cb release_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 Expand Up @@ -142,6 +149,21 @@ struct rpmsg_device {
bool support_ns;
};

/**
* @brief Increase the endpoint reference count
*
* @ept: pointer to rpmsg endpoint
*
*/
void rpmsg_incref(struct rpmsg_endpoint *ept);

/**
* @brief Decrease the end point reference count
*
* @ept: pointer to rpmsg endpoint
*/
void rpmsg_decref(struct rpmsg_endpoint *ept);

/**
* @brief Send a message across to the remote processor,
* specifying source and destination address.
Expand Down
14 changes: 14 additions & 0 deletions lib/rpmsg/rpmsg.c
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,18 @@ static int rpmsg_set_address(unsigned long *bitmap, int size, int addr)
}
}

void rpmsg_incref(struct rpmsg_endpoint *ept)
{
atomic_fetch_add(&ept->refcnt, 1);
}

void rpmsg_decref(struct rpmsg_endpoint *ept)
{
if (atomic_fetch_sub(&ept->refcnt, 1) == 1 && ept->release_cb) {
ept->release_cb(ept);
}
}

int rpmsg_send_offchannel_raw(struct rpmsg_endpoint *ept, uint32_t src,
uint32_t dst, const void *data, int len,
int wait)
Expand Down Expand Up @@ -247,6 +259,7 @@ static void rpmsg_unregister_endpoint(struct rpmsg_endpoint *ept)
metal_list_del(&ept->node);
ept->rdev = NULL;
metal_mutex_release(&rdev->lock);
rpmsg_decref(ept);
}

void rpmsg_register_endpoint(struct rpmsg_device *rdev,
Expand All @@ -256,6 +269,7 @@ void rpmsg_register_endpoint(struct rpmsg_device *rdev,
rpmsg_ept_cb cb,
rpmsg_ns_unbind_cb ns_unbind_cb)
{
rpmsg_incref(ept);
strncpy(ept->name, name ? name : "", sizeof(ept->name));
ept->addr = src;
ept->dest_addr = dest;
Expand Down
7 changes: 6 additions & 1 deletion lib/rpmsg/rpmsg_virtio.c
Original file line number Diff line number Diff line change
Expand Up @@ -568,8 +568,10 @@ static void rpmsg_virtio_rx_callback(struct virtqueue *vq)
*/
ept->dest_addr = rp_hdr->src;
}
rpmsg_incref(ept);
status = ept->cb(ept, RPMSG_LOCATE_DATA(rp_hdr),
rp_hdr->len, rp_hdr->src, ept->priv);
rpmsg_decref(ept);

RPMSG_ASSERT(status >= 0,
"unexpected callback status\r\n");
Expand Down Expand Up @@ -637,8 +639,11 @@ static int rpmsg_virtio_ns_callback(struct rpmsg_endpoint *ept, void *data,
if (_ept)
_ept->dest_addr = RPMSG_ADDR_ANY;
metal_mutex_release(&rdev->lock);
if (_ept && _ept->ns_unbind_cb)
if (_ept && _ept->ns_unbind_cb) {
rpmsg_incref(_ept);
_ept->ns_unbind_cb(_ept);
rpmsg_decref(_ept);
}
if (rdev->ns_unbind_cb)
rdev->ns_unbind_cb(rdev, name, dest);
} else {
Expand Down

0 comments on commit b90441b

Please sign in to comment.