Commit 6f5410b6 authored by Linus Torvalds's avatar Linus Torvalds

Merge tag 'rpmsg-3.5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg

Pull rpmsg fixes from Ohad Ben-Cohen:
 "Fixing two (somewhat rare) endpoint-related race issues, both of which
  were reported by Fernando Guzman Lugo."

* tag 'rpmsg-3.5-fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/ohad/rpmsg:
  rpmsg: make sure inflight messages don't invoke just-removed callbacks
  rpmsg: avoid premature deallocation of endpoints
parents 84836bf9 15fd943a
...@@ -188,6 +188,26 @@ static int rpmsg_uevent(struct device *dev, struct kobj_uevent_env *env) ...@@ -188,6 +188,26 @@ static int rpmsg_uevent(struct device *dev, struct kobj_uevent_env *env)
rpdev->id.name); rpdev->id.name);
} }
/**
* __ept_release() - deallocate an rpmsg endpoint
* @kref: the ept's reference count
*
* This function deallocates an ept, and is invoked when its @kref refcount
* drops to zero.
*
* Never invoke this function directly!
*/
static void __ept_release(struct kref *kref)
{
struct rpmsg_endpoint *ept = container_of(kref, struct rpmsg_endpoint,
refcount);
/*
* At this point no one holds a reference to ept anymore,
* so we can directly free it
*/
kfree(ept);
}
/* for more info, see below documentation of rpmsg_create_ept() */ /* for more info, see below documentation of rpmsg_create_ept() */
static struct rpmsg_endpoint *__rpmsg_create_ept(struct virtproc_info *vrp, static struct rpmsg_endpoint *__rpmsg_create_ept(struct virtproc_info *vrp,
struct rpmsg_channel *rpdev, rpmsg_rx_cb_t cb, struct rpmsg_channel *rpdev, rpmsg_rx_cb_t cb,
...@@ -206,6 +226,9 @@ static struct rpmsg_endpoint *__rpmsg_create_ept(struct virtproc_info *vrp, ...@@ -206,6 +226,9 @@ static struct rpmsg_endpoint *__rpmsg_create_ept(struct virtproc_info *vrp,
return NULL; return NULL;
} }
kref_init(&ept->refcount);
mutex_init(&ept->cb_lock);
ept->rpdev = rpdev; ept->rpdev = rpdev;
ept->cb = cb; ept->cb = cb;
ept->priv = priv; ept->priv = priv;
...@@ -238,7 +261,7 @@ static struct rpmsg_endpoint *__rpmsg_create_ept(struct virtproc_info *vrp, ...@@ -238,7 +261,7 @@ static struct rpmsg_endpoint *__rpmsg_create_ept(struct virtproc_info *vrp,
idr_remove(&vrp->endpoints, request); idr_remove(&vrp->endpoints, request);
free_ept: free_ept:
mutex_unlock(&vrp->endpoints_lock); mutex_unlock(&vrp->endpoints_lock);
kfree(ept); kref_put(&ept->refcount, __ept_release);
return NULL; return NULL;
} }
...@@ -302,11 +325,17 @@ EXPORT_SYMBOL(rpmsg_create_ept); ...@@ -302,11 +325,17 @@ EXPORT_SYMBOL(rpmsg_create_ept);
static void static void
__rpmsg_destroy_ept(struct virtproc_info *vrp, struct rpmsg_endpoint *ept) __rpmsg_destroy_ept(struct virtproc_info *vrp, struct rpmsg_endpoint *ept)
{ {
/* make sure new inbound messages can't find this ept anymore */
mutex_lock(&vrp->endpoints_lock); mutex_lock(&vrp->endpoints_lock);
idr_remove(&vrp->endpoints, ept->addr); idr_remove(&vrp->endpoints, ept->addr);
mutex_unlock(&vrp->endpoints_lock); mutex_unlock(&vrp->endpoints_lock);
kfree(ept); /* make sure in-flight inbound messages won't invoke cb anymore */
mutex_lock(&ept->cb_lock);
ept->cb = NULL;
mutex_unlock(&ept->cb_lock);
kref_put(&ept->refcount, __ept_release);
} }
/** /**
...@@ -790,12 +819,28 @@ static void rpmsg_recv_done(struct virtqueue *rvq) ...@@ -790,12 +819,28 @@ static void rpmsg_recv_done(struct virtqueue *rvq)
/* use the dst addr to fetch the callback of the appropriate user */ /* use the dst addr to fetch the callback of the appropriate user */
mutex_lock(&vrp->endpoints_lock); mutex_lock(&vrp->endpoints_lock);
ept = idr_find(&vrp->endpoints, msg->dst); ept = idr_find(&vrp->endpoints, msg->dst);
/* let's make sure no one deallocates ept while we use it */
if (ept)
kref_get(&ept->refcount);
mutex_unlock(&vrp->endpoints_lock); mutex_unlock(&vrp->endpoints_lock);
if (ept && ept->cb) if (ept) {
ept->cb(ept->rpdev, msg->data, msg->len, ept->priv, msg->src); /* make sure ept->cb doesn't go away while we use it */
else mutex_lock(&ept->cb_lock);
if (ept->cb)
ept->cb(ept->rpdev, msg->data, msg->len, ept->priv,
msg->src);
mutex_unlock(&ept->cb_lock);
/* farewell, ept, we don't need you anymore */
kref_put(&ept->refcount, __ept_release);
} else
dev_warn(dev, "msg received with no recepient\n"); dev_warn(dev, "msg received with no recepient\n");
/* publish the real size of the buffer */ /* publish the real size of the buffer */
......
...@@ -38,6 +38,8 @@ ...@@ -38,6 +38,8 @@
#include <linux/types.h> #include <linux/types.h>
#include <linux/device.h> #include <linux/device.h>
#include <linux/mod_devicetable.h> #include <linux/mod_devicetable.h>
#include <linux/kref.h>
#include <linux/mutex.h>
/* The feature bitmap for virtio rpmsg */ /* The feature bitmap for virtio rpmsg */
#define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */ #define VIRTIO_RPMSG_F_NS 0 /* RP supports name service notifications */
...@@ -120,7 +122,9 @@ typedef void (*rpmsg_rx_cb_t)(struct rpmsg_channel *, void *, int, void *, u32); ...@@ -120,7 +122,9 @@ typedef void (*rpmsg_rx_cb_t)(struct rpmsg_channel *, void *, int, void *, u32);
/** /**
* struct rpmsg_endpoint - binds a local rpmsg address to its user * struct rpmsg_endpoint - binds a local rpmsg address to its user
* @rpdev: rpmsg channel device * @rpdev: rpmsg channel device
* @refcount: when this drops to zero, the ept is deallocated
* @cb: rx callback handler * @cb: rx callback handler
* @cb_lock: must be taken before accessing/changing @cb
* @addr: local rpmsg address * @addr: local rpmsg address
* @priv: private data for the driver's use * @priv: private data for the driver's use
* *
...@@ -140,7 +144,9 @@ typedef void (*rpmsg_rx_cb_t)(struct rpmsg_channel *, void *, int, void *, u32); ...@@ -140,7 +144,9 @@ typedef void (*rpmsg_rx_cb_t)(struct rpmsg_channel *, void *, int, void *, u32);
*/ */
struct rpmsg_endpoint { struct rpmsg_endpoint {
struct rpmsg_channel *rpdev; struct rpmsg_channel *rpdev;
struct kref refcount;
rpmsg_rx_cb_t cb; rpmsg_rx_cb_t cb;
struct mutex cb_lock;
u32 addr; u32 addr;
void *priv; void *priv;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment