soc: hab: Fix a memory leakage when unexport

When the remote vchan is closed and unexport happens
in local at the same time, function hab_get_vchan_fromvcid
will return null in hab unexport, which will cause memory leak.

Change-Id: I8dac4f4154f24734dc2a11aa73f726cd705cc291
Signed-off-by: Yajun Li <yajunl@codeaurora.org>

diff --git a/drivers/soc/qcom/hab/hab.c b/drivers/soc/qcom/hab/hab.c
index ee7e988..7d53e61 100644 (file)
--- a/drivers/soc/qcom/hab/hab.c
+++ b/drivers/soc/qcom/hab/hab.c
@@ -209,14 +209,15 @@ void hab_ctx_free(struct kref *ref)
  * the local ioctl access based on ctx
  */
 struct virtual_channel *hab_get_vchan_fromvcid(int32_t vcid,
-               struct uhab_context *ctx)
+               struct uhab_context *ctx, int ignore_remote)
 {
        struct virtual_channel *vchan;
 
        read_lock(&ctx->ctx_lock);
        list_for_each_entry(vchan, &ctx->vchannels, node) {
                if (vcid == vchan->id) {
-                       if (vchan->otherend_closed || vchan->closed ||
+                       if ((ignore_remote ? 0 : vchan->otherend_closed) ||
+                               vchan->closed ||
                                !kref_get_unless_zero(&vchan->refcount)) {
                                pr_debug("failed to inc vcid %x remote %x session %d refcnt %d close_flg remote %d local %d\n",
                                        vchan->id, vchan->otherend_id,
@@ -550,7 +551,7 @@ long hab_vchan_send(struct uhab_context *ctx,
                return -EINVAL;
        }
 
-       vchan = hab_get_vchan_fromvcid(vcid, ctx);
+       vchan = hab_get_vchan_fromvcid(vcid, ctx, 0);
        if (!vchan || vchan->otherend_closed) {
                ret = -ENODEV;
                goto err;
@@ -597,7 +598,7 @@ int hab_vchan_recv(struct uhab_context *ctx,
        int ret = 0;
        int nonblocking_flag = flags & HABMM_SOCKET_RECV_FLAGS_NON_BLOCKING;
 
-       vchan = hab_get_vchan_fromvcid(vcid, ctx);
+       vchan = hab_get_vchan_fromvcid(vcid, ctx, 1);
        if (!vchan) {
                pr_err("vcid %X vchan 0x%pK ctx %pK\n", vcid, vchan, ctx);
                return -ENODEV;
@@ -719,24 +720,13 @@ void hab_vchan_close(struct uhab_context *ctx, int32_t vcid)
        write_lock(&ctx->ctx_lock);
        list_for_each_entry_safe(vchan, tmp, &ctx->vchannels, node) {
                if (vchan->id == vcid) {
+                       vchan->closed = 1;
                        write_unlock(&ctx->ctx_lock);
                        pr_debug("vcid %x remote %x session %d refcnt %d\n",
                                vchan->id, vchan->otherend_id,
                                vchan->session_id, get_refcnt(vchan->refcount));
-                       /*
-                        * only set when vc close is called locally by user
-                        * explicity. Used to block remote msg. if forked once
-                        * before, this local close is skipped due to child
-                        * usage. if forked but not closed locally, the local
-                        * context could NOT be closed, vchan can be prolonged
-                        * by arrived remote msgs
-                        */
-                       if (vchan->forked)
-                               vchan->forked = 0;
-                       else {
-                               vchan->closed = 1;
-                               hab_vchan_stop_notify(vchan);
-                       }
+                       /* unblocking blocked in-calls */
+                       hab_vchan_stop_notify(vchan);
                        hab_vchan_put(vchan); /* there is a lock inside */
                        write_lock(&ctx->ctx_lock);
                        break;
@@ -1090,13 +1080,16 @@ static int hab_release(struct inode *inodep, struct file *filep)
                                        vchan->closed, vchan->otherend_closed);
                                continue; /* vchan is already being freed */
                        } else {
+                               write_unlock(&ctx->ctx_lock);
                                hab_vchan_stop_notify(vchan);
                                /* put for notify. shouldn't cause free */
                                hab_vchan_put(vchan);
+                               write_lock(&ctx->ctx_lock);
                        }
                } else
                        continue;
 
+               /* for the missing local vchan close */
                write_unlock(&ctx->ctx_lock);
                hab_vchan_put(vchan); /* there is a lock inside */
                write_lock(&ctx->ctx_lock);
@@ -1117,12 +1110,6 @@ static int hab_release(struct inode *inodep, struct file *filep)
        hab_ctx_put(ctx);
        filep->private_data = NULL;
 
-       /* ctx leak check */
-       if (get_refcnt(ctx->refcount))
-               pr_warn("pending ctx release owner %d refcnt %d total %d\n",
-                               ctx->owner, get_refcnt(ctx->refcount),
-                               hab_driver.ctx_cnt);
-
        return 0;
 }
 

diff --git a/drivers/soc/qcom/hab/hab.h b/drivers/soc/qcom/hab/hab.h
index 18bcd0a..a2df3b6 100644 (file)
--- a/drivers/soc/qcom/hab/hab.h
+++ b/drivers/soc/qcom/hab/hab.h
@@ -494,7 +494,7 @@ struct virtual_channel *hab_vchan_get(struct physical_channel *pchan,
 void hab_vchan_put(struct virtual_channel *vchan);
 
 struct virtual_channel *hab_get_vchan_fromvcid(int32_t vcid,
-               struct uhab_context *ctx);
+               struct uhab_context *ctx, int ignore_remote);
 struct physical_channel *hab_pchan_alloc(struct hab_device *habdev,
                int otherend_id);
 struct physical_channel *hab_pchan_find_domid(struct hab_device *dev,

diff --git a/drivers/soc/qcom/hab/hab_mimex.c b/drivers/soc/qcom/hab/hab_mimex.c
index e7ce3eb..b037399 100644 (file)
--- a/drivers/soc/qcom/hab/hab_mimex.c
+++ b/drivers/soc/qcom/hab/hab_mimex.c
@@ -246,7 +246,7 @@ int hab_mem_export(struct uhab_context *ctx,
        if (!ctx || !param || !param->buffer)
                return -EINVAL;
 
-       vchan = hab_get_vchan_fromvcid(param->vcid, ctx);
+       vchan = hab_get_vchan_fromvcid(param->vcid, ctx, 0);
        if (!vchan || !vchan->pchan) {
                ret = -ENODEV;
                goto err;
@@ -313,7 +313,7 @@ int hab_mem_unexport(struct uhab_context *ctx,
                return -EINVAL;
 
        /* refcnt on the access */
-       vchan = hab_get_vchan_fromvcid(param->vcid, ctx);
+       vchan = hab_get_vchan_fromvcid(param->vcid, ctx, 1);
        if (!vchan || !vchan->pchan) {
                ret = -ENODEV;
                goto err_novchan;
@@ -360,7 +360,7 @@ int hab_mem_import(struct uhab_context *ctx,
        if (!ctx || !param)
                return -EINVAL;
 
-       vchan = hab_get_vchan_fromvcid(param->vcid, ctx);
+       vchan = hab_get_vchan_fromvcid(param->vcid, ctx, 0);
        if (!vchan || !vchan->pchan) {
                ret = -ENODEV;
                goto err_imp;
@@ -420,7 +420,7 @@ int hab_mem_unimport(struct uhab_context *ctx,
        if (!ctx || !param)
                return -EINVAL;
 
-       vchan = hab_get_vchan_fromvcid(param->vcid, ctx);
+       vchan = hab_get_vchan_fromvcid(param->vcid, ctx, 1);
        if (!vchan || !vchan->pchan) {
                if (vchan)
                        hab_vchan_put(vchan);

diff --git a/drivers/soc/qcom/hab/hab_msg.c b/drivers/soc/qcom/hab/hab_msg.c
index 9323cf9..40ff1a9 100644 (file)
--- a/drivers/soc/qcom/hab/hab_msg.c
+++ b/drivers/soc/qcom/hab/hab_msg.c
@@ -27,6 +27,12 @@ hab_msg_alloc(struct physical_channel *pchan, size_t sizebytes)
 {
        struct hab_message *message;
 
+       if (sizebytes > HAB_HEADER_SIZE_MASK) {
+               pr_err("pchan %s send size too large %zd\n",
+                       pchan->name, sizebytes);
+               return NULL;
+       }
+
        message = kzalloc(sizeof(*message) + sizebytes, GFP_ATOMIC);
        if (!message)
                return NULL;
@@ -153,6 +159,12 @@ static int hab_receive_create_export_ack(struct physical_channel *pchan,
                pr_err("exp ack size %zu is not as arrived %zu\n",
                                  sizeof(ack_recvd->ack), sizebytes);
 
+       if (sizebytes > HAB_HEADER_SIZE_MASK) {
+               pr_err("pchan %s read size too large %zd\n",
+                       pchan->name, sizebytes);
+               return -EINVAL;
+       }
+
        if (physical_channel_read(pchan,
                &ack_recvd->ack,
                sizebytes) != sizebytes)
@@ -169,6 +181,11 @@ static void hab_msg_drop(struct physical_channel *pchan, size_t sizebytes)
 {
        uint8_t *data = NULL;
 
+       if (sizebytes > HAB_HEADER_SIZE_MASK) {
+               pr_err("%s read size too large %zd\n", pchan->name, sizebytes);
+               return;
+       }
+
        data = kmalloc(sizebytes, GFP_ATOMIC);
        if (data == NULL)
                return;
@@ -275,6 +292,12 @@ int hab_msg_recv(struct physical_channel *pchan,
                break;
 
        case HAB_PAYLOAD_TYPE_EXPORT:
+               if (sizebytes > HAB_HEADER_SIZE_MASK) {
+                       pr_err("%s exp size too large %zd\n",
+                                       pchan->name, sizebytes);
+                       break;
+               }
+
                exp_desc = kzalloc(sizebytes, GFP_ATOMIC);
                if (!exp_desc)
                        break;

diff --git a/drivers/soc/qcom/hab/hab_open.c b/drivers/soc/qcom/hab/hab_open.c
index f09a1df..bc2b774 100644 (file)
--- a/drivers/soc/qcom/hab/hab_open.c
+++ b/drivers/soc/qcom/hab/hab_open.c
@@ -47,6 +47,12 @@ int hab_open_request_add(struct physical_channel *pchan,
        struct hab_open_request *request;
        struct timeval tv;
 
+       if (sizebytes > HAB_HEADER_SIZE_MASK) {
+               pr_err("pchan %s request size too large %zd\n",
+                       pchan->name, sizebytes);
+               return -EINVAL;
+       }
+
        node = kzalloc(sizeof(*node), GFP_ATOMIC);
        if (!node)
                return -ENOMEM;
@@ -187,6 +193,12 @@ int hab_open_receive_cancel(struct physical_channel *pchan,
        int bfound = 0;
        struct timeval tv;
 
+       if (sizebytes > HAB_HEADER_SIZE_MASK) {
+               pr_err("pchan %s cancel size too large %zd\n",
+                       pchan->name, sizebytes);
+               return -EINVAL;
+       }
+
        if (physical_channel_read(pchan, &data, sizebytes) != sizebytes)
                return -EIO;
 

diff --git a/drivers/soc/qcom/hab/hab_vchan.c b/drivers/soc/qcom/hab/hab_vchan.c
index f4145c5..770c6ba 100644 (file)
--- a/drivers/soc/qcom/hab/hab_vchan.c
+++ b/drivers/soc/qcom/hab/hab_vchan.c
@@ -206,15 +206,22 @@ static int hab_vchans_per_pchan_empty(struct physical_channel *pchan)
        empty = list_empty(&pchan->vchannels);
        if (!empty) {
                struct virtual_channel *vchan;
+               int vcnt = pchan->vcnt;
 
                list_for_each_entry(vchan, &pchan->vchannels, pnode) {
-                       pr_err("vchan %pK id %x remote id %x session %d ref %d closed %d remote close %d\n",
-                                  vchan, vchan->id, vchan->otherend_id,
-                                  vchan->session_id,
-                                  get_refcnt(vchan->refcount), vchan->closed,
-                                  vchan->otherend_closed);
+                       /* discount open-pending unpaired vchan */
+                       if (!vchan->session_id)
+                               vcnt--;
+                       else
+                               pr_err("vchan %pK %x rm %x sn %d rf %d clsd %d rm clsd %d\n",
+                                       vchan, vchan->id,
+                                       vchan->otherend_id,
+                                       vchan->session_id,
+                                       get_refcnt(vchan->refcount),
+                                       vchan->closed, vchan->otherend_closed);
                }
-
+               if (!vcnt)
+                       empty = 1;/* unpaired vchan can exist at init time */
        }
        read_unlock(&pchan->vchans_lock);
 
@@ -308,7 +315,9 @@ void hab_vchan_put(struct virtual_channel *vchan)
 int hab_vchan_query(struct uhab_context *ctx, int32_t vcid, uint64_t *ids,
                           char *names, size_t name_size, uint32_t flags)
 {
-       struct virtual_channel *vchan = hab_get_vchan_fromvcid(vcid, ctx);
+       struct virtual_channel *vchan;
+
+       vchan = hab_get_vchan_fromvcid(vcid, ctx, 1);
        if (!vchan)
                return -EINVAL;
 

diff --git a/drivers/soc/qcom/hab/qvm_comm.c b/drivers/soc/qcom/hab/qvm_comm.c
index cce24d7..ab57f27 100644 (file)
--- a/drivers/soc/qcom/hab/qvm_comm.c
+++ b/drivers/soc/qcom/hab/qvm_comm.c
@@ -56,6 +56,7 @@ int physical_channel_send(struct physical_channel *pchan,
                return -EAGAIN; /* not enough free space */
        }
 
+       header->sequence = pchan->sequence_tx + 1;
        header->signature = HAB_HEAD_SIGNATURE;
 
        if (hab_pipe_write(dev->pipe_ep,
@@ -92,7 +93,7 @@ int physical_channel_send(struct physical_channel *pchan,
        hab_pipe_write_commit(dev->pipe_ep);
        spin_unlock_bh(&dev->io_lock);
        habhyp_notify(dev);
-
+       ++pchan->sequence_tx;
        return 0;
 }
 
@@ -117,6 +118,8 @@ void physical_channel_rx_dispatch(unsigned long data)
                                header.sequence);
                }
 
+               pchan->sequence_rx = header.sequence;
+
                hab_msg_recv(pchan, &header);
        }
        spin_unlock_bh(&pchan->rxbuf_lock);