From 32782582c33233887604432cebb7538f17b0f174 Mon Sep 17 00:00:00 2001 From: Yajun Li Date: Thu, 25 Oct 2018 11:08:25 +0800 Subject: [PATCH] soc: hab: fix uninitialized variable and relocate vchan refcnt Init hab msg pointer to avoid accessing an uninitialized pointer. The format specifier "%p" can leak kernel addresses. Use "%pK" instead. Check the status of the pfn_table, because of wrong pagetable coming from the corresponding hab client unexpectedly. Change-Id: Ic8c6ba0243d27007d014165f2869a5775a96c09d Signed-off-by: Yajun Li --- drivers/soc/qcom/hab/hab.c | 4 ++-- drivers/soc/qcom/hab/hab.h | 3 ++- drivers/soc/qcom/hab/hab_mem_linux.c | 9 ++++++++- drivers/soc/qcom/hab/hab_mimex.c | 4 ++-- drivers/soc/qcom/hab/hab_msg.c | 4 ++-- drivers/soc/qcom/hab/hab_stat.c | 9 ++++++--- drivers/soc/qcom/hab/hab_vchan.c | 2 ++ 7 files changed, 24 insertions(+), 11 deletions(-) diff --git a/drivers/soc/qcom/hab/hab.c b/drivers/soc/qcom/hab/hab.c index ebe7dfc4a5b6..ee7e9881b193 100644 --- a/drivers/soc/qcom/hab/hab.c +++ b/drivers/soc/qcom/hab/hab.c @@ -599,7 +599,7 @@ int hab_vchan_recv(struct uhab_context *ctx, vchan = hab_get_vchan_fromvcid(vcid, ctx); if (!vchan) { - pr_err("vcid %X, vchan %p ctx %p\n", vcid, vchan, ctx); + pr_err("vcid %X vchan 0x%pK ctx %pK\n", vcid, vchan, ctx); return -ENODEV; } @@ -1134,7 +1134,7 @@ static long hab_ioctl(struct file *filep, unsigned int cmd, unsigned long arg) struct hab_recv *recv_param; struct hab_send *send_param; struct hab_info *info_param; - struct hab_message *msg; + struct hab_message *msg = NULL; void *send_data; unsigned char data[256] = { 0 }; long ret = 0; diff --git a/drivers/soc/qcom/hab/hab.h b/drivers/soc/qcom/hab/hab.h index cbc049e89d63..18bcd0af8058 100644 --- a/drivers/soc/qcom/hab/hab.h +++ b/drivers/soc/qcom/hab/hab.h @@ -214,6 +214,7 @@ struct physical_channel { /* debug only */ uint32_t sequence_tx; uint32_t sequence_rx; + uint32_t status; /* vchans on this pchan */ struct list_head vchannels; @@ -255,8 +256,8 @@ struct hab_export_ack_recvd { }; struct hab_message { - size_t sizebytes; struct list_head node; + size_t sizebytes; uint32_t data[]; }; diff --git a/drivers/soc/qcom/hab/hab_mem_linux.c b/drivers/soc/qcom/hab/hab_mem_linux.c index 60156c6b00f0..670efdea212f 100644 --- a/drivers/soc/qcom/hab/hab_mem_linux.c +++ b/drivers/soc/qcom/hab/hab_mem_linux.c @@ -53,6 +53,14 @@ static struct pages_list *pages_list_create( if (!pfn_table) return ERR_PTR(-EINVAL); + pfn = pfn_table->first_pfn; + if (pfn_valid(pfn) == 0 || page_is_ram(pfn) == 0) { + pr_err("imp sanity failed pfn %lx valid %d ram %d pchan %s\n", + pfn, pfn_valid(pfn), + page_is_ram(pfn), exp->pchan->name); + return ERR_PTR(-EINVAL); + } + size = exp->payload_count * sizeof(struct page *); pages = kmalloc(size, GFP_KERNEL); if (!pages) @@ -64,7 +72,6 @@ static struct pages_list *pages_list_create( return ERR_PTR(-ENOMEM); } - pfn = pfn_table->first_pfn; for (i = 0; i < pfn_table->nregions; i++) { for (j = 0; j < pfn_table->region[i].size; j++) { pages[k] = pfn_to_page(pfn+j); diff --git a/drivers/soc/qcom/hab/hab_mimex.c b/drivers/soc/qcom/hab/hab_mimex.c index efa4bb383ec6..e7ce3ebdddc8 100644 --- a/drivers/soc/qcom/hab/hab_mimex.c +++ b/drivers/soc/qcom/hab/hab_mimex.c @@ -277,8 +277,8 @@ int hab_mem_export(struct uhab_context *ctx, &pdata_size); } if (ret < 0) { - pr_err("habmem_hyp_grant failed size=%d ret=%d\n", - pdata_size, ret); + pr_err("habmem_hyp_grant vc %x failed size=%d ret=%d\n", + param->vcid, pdata_size, ret); goto err; } diff --git a/drivers/soc/qcom/hab/hab_msg.c b/drivers/soc/qcom/hab/hab_msg.c index 71010be414d5..9323cf948fb9 100644 --- a/drivers/soc/qcom/hab/hab_msg.c +++ b/drivers/soc/qcom/hab/hab_msg.c @@ -210,7 +210,7 @@ int hab_msg_recv(struct physical_channel *pchan, */ vchan = hab_vchan_get(pchan, header); if (!vchan) { - pr_info("vchan is not found, payload type %d, vchan id %x, sizebytes %zx, session %d\n", + pr_debug("vchan not found type %d vcid %x sz %zx sesn %d\n", payload_type, vchan_id, sizebytes, session_id); if (sizebytes) { @@ -313,7 +313,7 @@ int hab_msg_recv(struct physical_channel *pchan, case HAB_PAYLOAD_TYPE_CLOSE: /* remote request close */ - pr_info("remote request close vcid %pK %X other id %X session %d refcnt %d\n", + pr_debug("remote close vcid %pK %X other id %X session %d refcnt %d\n", vchan, vchan->id, vchan->otherend_id, session_id, get_refcnt(vchan->refcount)); hab_vchan_stop(vchan); diff --git a/drivers/soc/qcom/hab/hab_stat.c b/drivers/soc/qcom/hab/hab_stat.c index f0a4207bc871..b78e5e7e9867 100644 --- a/drivers/soc/qcom/hab/hab_stat.c +++ b/drivers/soc/qcom/hab/hab_stat.c @@ -61,14 +61,17 @@ int hab_stat_show_vchan(struct hab_driver *driver, continue; ret = hab_stat_buffer_print(buf, size, - "mmid %s role %d local %d remote %d vcnt %d:\n", + "nm %s r %d lc %d rm %d sq_t %d sq_r %d st 0x%x vn %d:\n", pchan->name, pchan->is_be, pchan->vmid_local, - pchan->vmid_remote, pchan->vcnt); + pchan->vmid_remote, pchan->sequence_tx, + pchan->sequence_rx, pchan->status, pchan->vcnt); read_lock(&pchan->vchans_lock); list_for_each_entry(vc, &pchan->vchannels, pnode) { ret = hab_stat_buffer_print(buf, size, - "%08X ", vc->id); + "%08X(%d:%d) ", vc->id, + get_refcnt(vc->refcount), + vc->otherend_closed); } ret = hab_stat_buffer_print(buf, size, "\n"); read_unlock(&pchan->vchans_lock); diff --git a/drivers/soc/qcom/hab/hab_vchan.c b/drivers/soc/qcom/hab/hab_vchan.c index 8d818b8fafb1..f4145c5869f8 100644 --- a/drivers/soc/qcom/hab/hab_vchan.c +++ b/drivers/soc/qcom/hab/hab_vchan.c @@ -167,6 +167,7 @@ hab_vchan_get(struct physical_channel *pchan, struct hab_header *header) return vchan; } +/* wake up local waiting Q, so stop-vchan can be processed */ void hab_vchan_stop(struct virtual_channel *vchan) { if (vchan) { @@ -190,6 +191,7 @@ void hab_vchans_stop(struct physical_channel *pchan) read_unlock(&pchan->vchans_lock); } +/* send vchan close to remote and stop receiving anything locally */ void hab_vchan_stop_notify(struct virtual_channel *vchan) { hab_send_close_msg(vchan); -- 2.11.0