Fix double free, occurs if a channel destroyed callback is fired from interrupt durin...
authorMichal Princ (NXA17570) <michal.princ@nxp.com>
Thu, 31 Mar 2016 09:05:04 +0000 (11:05 +0200)
committerWendy Liang <jliang@xilinx.com>
Mon, 11 Apr 2016 18:53:06 +0000 (11:53 -0700)
- rpmsg_rdev_get_chnl_from_addr() removed because it is not used

Signed-off-by: Michal Princ (NXA17570) <michal.princ@nxp.com>
lib/include/openamp/rpmsg_core.h
lib/rpmsg/remote_device.c
lib/rpmsg/rpmsg_core.c

index 53fd58b8fc32f0266578750b679f4371daa5666a..22abe18cb4386391aad9bc3ee0c1d77a73661e60 100644 (file)
@@ -160,8 +160,6 @@ int rpmsg_rdev_init(struct remote_device **rdev, int dev_id, int role,
 void rpmsg_rdev_deinit(struct remote_device *rdev);
 struct llist *rpmsg_rdev_get_chnl_node_from_id(struct remote_device *rdev,
                                               char *rp_chnl_id);
-struct llist *rpmsg_rdev_get_chnl_from_addr(struct remote_device *rdev,
-                                           unsigned long addr);
 struct llist *rpmsg_rdev_get_endpoint_from_addr(struct remote_device *rdev,
                                                unsigned long addr);
 int rpmsg_rdev_notify(struct remote_device *rdev);
index 9538644788628719dc2752f9209432a5bbb88489..aea5b1dcdae5a126fe39c5ce62f04b68bc0ab371 100644 (file)
@@ -206,7 +206,10 @@ void rpmsg_rdev_deinit(struct remote_device *rdev)
        }
 
        /* Delete name service endpoint */
+
+       env_lock_mutex(rdev->lock);
        node = rpmsg_rdev_get_endpoint_from_addr(rdev, RPMSG_NS_EPT_ADDR);
+       env_unlock_mutex(rdev->lock);
        if (node) {
                _destroy_endpoint(rdev, (struct rpmsg_endpoint *)node->data);
        }
@@ -234,7 +237,8 @@ void rpmsg_rdev_deinit(struct remote_device *rdev)
 /**
  * rpmsg_rdev_get_chnl_node_from_id
  *
- * This function returns channel node based on channel name.
+ * This function returns channel node based on channel name. It must be called
+ * with mutex locked.
  *
  * @param stack      - pointer to remote device
  * @param rp_chnl_id - rpmsg channel name
@@ -250,51 +254,15 @@ struct llist *rpmsg_rdev_get_chnl_node_from_id(struct remote_device *rdev,
 
        rp_chnl_head = rdev->rp_channels;
 
-       env_lock_mutex(rdev->lock);
        while (rp_chnl_head) {
                rp_chnl = (struct rpmsg_channel *)rp_chnl_head->data;
                if (env_strncmp
                    (rp_chnl->name, rp_chnl_id, sizeof(rp_chnl->name))
                    == 0) {
-                       env_unlock_mutex(rdev->lock);
                        return rp_chnl_head;
                }
                rp_chnl_head = rp_chnl_head->next;
        }
-       env_unlock_mutex(rdev->lock);
-
-       return RPMSG_NULL;
-}
-
-/**
- * rpmsg_rdev_get_chnl_from_addr
- *
- * This function returns channel node based on src/dst address.
- *
- * @param rdev - pointer remote device control block
- * @param addr - src/dst address
- *
- * @return - channel node
- *
- */
-struct llist *rpmsg_rdev_get_chnl_from_addr(struct remote_device *rdev,
-                                           unsigned long addr)
-{
-       struct rpmsg_channel *rp_chnl;
-       struct llist *rp_chnl_head;
-
-       rp_chnl_head = rdev->rp_channels;
-
-       env_lock_mutex(rdev->lock);
-       while (rp_chnl_head) {
-               rp_chnl = (struct rpmsg_channel *)rp_chnl_head->data;
-               if ((rp_chnl->src == addr) || (rp_chnl->dst == addr)) {
-                       env_unlock_mutex(rdev->lock);
-                       return rp_chnl_head;
-               }
-               rp_chnl_head = rp_chnl_head->next;
-       }
-       env_unlock_mutex(rdev->lock);
 
        return RPMSG_NULL;
 }
@@ -302,7 +270,8 @@ struct llist *rpmsg_rdev_get_chnl_from_addr(struct remote_device *rdev,
 /**
  * rpmsg_rdev_get_endpoint_from_addr
  *
- * This function returns endpoint node based on src address.
+ * This function returns endpoint node based on src address. It must be called
+ * with mutex locked.
  *
  * @param rdev - pointer remote device control block
  * @param addr - src address
@@ -317,17 +286,14 @@ struct llist *rpmsg_rdev_get_endpoint_from_addr(struct remote_device *rdev,
 
        rp_ept_lut_head = rdev->rp_endpoints;
 
-       env_lock_mutex(rdev->lock);
        while (rp_ept_lut_head) {
                struct rpmsg_endpoint *rp_ept =
                    (struct rpmsg_endpoint *)rp_ept_lut_head->data;
                if (rp_ept->addr == addr) {
-                       env_unlock_mutex(rdev->lock);
                        return rp_ept_lut_head;
                }
                rp_ept_lut_head = rp_ept_lut_head->next;
        }
-       env_unlock_mutex(rdev->lock);
 
        return RPMSG_NULL;
 }
index a2a44de2aaca5dff8795d9b7ac50eb5ac6455aa7..eff7e1ec4fbef4a70b6e7c65174de99b406e32b2 100644 (file)
@@ -188,16 +188,19 @@ void _rpmsg_delete_channel(struct rpmsg_channel *rp_chnl)
 {
        struct llist *node;
        if (rp_chnl) {
+               env_lock_mutex(rp_chnl->rdev->lock);
                node =
                    rpmsg_rdev_get_chnl_node_from_id(rp_chnl->rdev,
                                                     rp_chnl->name);
                if (node) {
-                       env_lock_mutex(rp_chnl->rdev->lock);
                        remove_from_list(&rp_chnl->rdev->rp_channels, node);
                        env_unlock_mutex(rp_chnl->rdev->lock);
+                       /* free node and rp_chnl */
                        env_free_memory(node);
+                       env_free_memory(rp_chnl);
+               } else {
+                       env_unlock_mutex(rp_chnl->rdev->lock);
                }
-               env_free_memory(rp_chnl);
        }
 }
 
@@ -291,16 +294,19 @@ void _destroy_endpoint(struct remote_device *rdev,
                       struct rpmsg_endpoint *rp_ept)
 {
        struct llist *node;
+       env_lock_mutex(rdev->lock);
        node = rpmsg_rdev_get_endpoint_from_addr(rdev, rp_ept->addr);
        if (node) {
-               env_lock_mutex(rdev->lock);
                rpmsg_release_address(rdev->bitmap, RPMSG_ADDR_BMP_SIZE,
                                      rp_ept->addr);
                remove_from_list(&rdev->rp_endpoints, node);
                env_unlock_mutex(rdev->lock);
+               /* free node and rp_ept */
                env_free_memory(node);
+               env_free_memory(rp_ept);
+       } else {
+               env_unlock_mutex(rdev->lock);
        }
-       env_free_memory(rp_ept);
 }
 
 /**
@@ -586,7 +592,9 @@ void rpmsg_rx_callback(struct virtqueue *vq)
        while (rp_hdr) {
 
                /* Get the channel node from the remote device channels list. */
+               env_lock_mutex(rdev->lock);
                node = rpmsg_rdev_get_endpoint_from_addr(rdev, rp_hdr->dst);
+               env_unlock_mutex(rdev->lock);
 
                if (!node)
                        /* Fatal error no endpoint for the given dst addr. */
@@ -657,7 +665,9 @@ void rpmsg_ns_callback(struct rpmsg_channel *server_chnl, void *data, int len,
        ns_msg->name[len - 1] = '\0';
 
        if (ns_msg->flags & RPMSG_NS_DESTROY) {
+               env_lock_mutex(rdev->lock);
                node = rpmsg_rdev_get_chnl_node_from_id(rdev, ns_msg->name);
+               env_unlock_mutex(rdev->lock);
                if (node) {
                        rp_chnl = (struct rpmsg_channel *)node->data;
                        if (rdev->channel_destroyed) {