From dc68a9504353655ef66838f0ff09e5a218ac3db4 Mon Sep 17 00:00:00 2001 From: Boojin Kim Date: Thu, 28 Jun 2018 12:35:14 +0900 Subject: [PATCH] [COMMON] chub: fix chub driver kernel panic This patch fixs the following kernel panic - remove access after free during download_image - remove invalid chub access Change-Id: Icd760df4aef48ef7273f3f18fb83aa73d0672a2d Signed-off-by: Boojin Kim --- drivers/staging/nanohub/Makefile | 1 - drivers/staging/nanohub/chub.c | 41 ++++++++++++++++++++---------- drivers/staging/nanohub/chub.h | 5 ++++ drivers/staging/nanohub/chub_dbg.c | 5 ++-- drivers/staging/nanohub/chub_ipc.c | 12 +++++++-- drivers/staging/nanohub/comms.c | 27 ++++++++++++++++++++ 6 files changed, 73 insertions(+), 18 deletions(-) diff --git a/drivers/staging/nanohub/Makefile b/drivers/staging/nanohub/Makefile index d5614a7321b9..37676ff2e785 100644 --- a/drivers/staging/nanohub/Makefile +++ b/drivers/staging/nanohub/Makefile @@ -2,7 +2,6 @@ # Makefile for nanohub # -ccflags-$(CONFIG_CONTEXTHUB_DEBUG) := -DDEBUG obj-$(CONFIG_NANOHUB) += nanohub.o nanohub-$(CONFIG_CHRE_SENSORHUB_HAL) := main.o comms.o nanohub-$(CONFIG_NANOHUB_SPI) += spi.o bl.o diff --git a/drivers/staging/nanohub/chub.c b/drivers/staging/nanohub/chub.c index 5eba4dcf9612..58965bfc463d 100644 --- a/drivers/staging/nanohub/chub.c +++ b/drivers/staging/nanohub/chub.c @@ -724,8 +724,16 @@ int contexthub_reset(struct contexthub_ipc_info *ipc) { int ret; - /* TODO: add wait lock */ dev_info(ipc->dev, "%s\n", __func__); +#if 0 + spin_lock(&ipc->reset_lock); + if (atomic_read(&ipc->chub_status) == CHUB_ST_RUN) { + spin_unlock(&ipc->reset_lock); + pr_err("%s: chub already reset. run: %d\n", + __func__, atomic_read(&ipc->chub_status)); + return 0; + } +#endif ret = contexthub_ipc_write_event(ipc, MAILBOX_EVT_SHUTDOWN); if (ret) { pr_err("%s: shutdonw fails, ret:%d\n", __func__, ret); @@ -747,7 +755,9 @@ int contexthub_reset(struct contexthub_ipc_info *ipc) ret = contexthub_ipc_write_event(ipc, MAILBOX_EVT_RESET); if (ret) pr_err("%s: reset fails, ret:%d\n", __func__, ret); - +#if 0 + spin_unlock(&ipc->reset_lock); +#endif return ret; } @@ -767,7 +777,6 @@ int contexthub_download_image(struct contexthub_ipc_info *ipc, int bl) dev_info(ipc->dev, "%s: bootloader(size:0x%x) on %lx\n", __func__, (int)entry->size, (unsigned long)ipc_get_base(IPC_REG_BL)); - release_firmware(entry); } else { ret = request_firmware(&entry, ipc->os_name, ipc->dev); @@ -777,11 +786,10 @@ int contexthub_download_image(struct contexthub_ipc_info *ipc, int bl) return ret; } memcpy(ipc_get_base(IPC_REG_OS), entry->data, entry->size); - release_firmware(entry); - dev_info(ipc->dev, "%s: %s(size:0x%x) on %lx\n", __func__, ipc->os_name, (int)entry->size, (unsigned long)ipc_get_base(IPC_REG_OS)); + release_firmware(entry); } return 0; @@ -817,6 +825,7 @@ static void handle_utc_work_func(struct work_struct *work) container_of(work, struct contexthub_ipc_info, utc_work); int trycnt = 0; +#if 0 while (ipc->utc_run) { msleep(20000); ipc_write_val(AP, sched_clock()); @@ -825,6 +834,7 @@ static void handle_utc_work_func(struct work_struct *work) if (!(++trycnt % 10)) log_flush(ipc->fw_log); }; +#endif dev_dbg(ipc->dev, "%s is done with %d try\n", __func__, trycnt); } @@ -867,7 +877,6 @@ static void handle_debug_work_func(struct work_struct *work) if (need_reset) goto do_reset; - log_flush(ipc->fw_log); /* chub fw error */ switch (event) { case IPC_DEBUG_CHUB_FULL_LOG: @@ -898,12 +907,9 @@ static void handle_debug_work_func(struct work_struct *work) ipc->err_cnt[fw_err]++; do_reset: - dev_info(ipc->dev, "%s: reset chub due to irq trigger error:%d\n", - __func__, err); - /* req to chub fw dump */ - ret = contexthub_ipc_write_event(ipc, MAILBOX_EVT_DUMP_STATUS); - /* dump log into file */ - log_dump_all(err); + dev_info(ipc->dev, "%s: error:%d, alive:%d\n", + __func__, err, alive); + /* dump hw & sram into file */ chub_dbg_dump_hw(ipc, err); if (need_reset) { @@ -916,8 +922,12 @@ do_reset: dev_info(ipc->dev, "%s: chub reset! should be recovery\n", __func__); if (CHUB_ERR_NANOHUB_WDT == CHUB_ERR_NANOHUB_WDT) - enable_irq(ipc->irq_wdt); + if (ipc->irq_wdt) + enable_irq(ipc->irq_wdt); } + } else { + /* dump log into file: DO NOT logbuf dueto sram corruption */ + log_dump_all(err); } } @@ -1033,6 +1043,7 @@ static irqreturn_t contexthub_irq_handler(int irq, void *data) return IRQ_HANDLED; } +#ifdef WDT_ENABLE static irqreturn_t contexthub_irq_wdt_handler(int irq, void *data) { struct contexthub_ipc_info *ipc = data; @@ -1044,6 +1055,7 @@ static irqreturn_t contexthub_irq_wdt_handler(int irq, void *data) return IRQ_HANDLED; } +#endif static int contexthub_get_cmgp_clocks(struct device *dev) { @@ -1163,6 +1175,7 @@ static __init int contexthub_ipc_hw_init(struct platform_device *pdev, return ret; } +#ifdef WDT_ENABLE /* get wdt interrupt optionally */ chub->irq_wdt = irq_of_parse_and_map(node, 1); if (chub->irq_wdt > 0) { @@ -1178,6 +1191,7 @@ static __init int contexthub_ipc_hw_init(struct platform_device *pdev, } else { dev_info(dev, "don't use wdt irq:%d\n", irq); } +#endif /* get MAILBOX SFR */ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "mailbox"); @@ -1372,6 +1386,7 @@ static int contexthub_ipc_probe(struct platform_device *pdev) attributes[i].attr.name); } + spin_lock_init(&chub->reset_lock); init_waitqueue_head(&chub->read_lock.event); init_waitqueue_head(&chub->chub_alive_lock.event); INIT_WORK(&chub->debug_work, handle_debug_work_func); diff --git a/drivers/staging/nanohub/chub.h b/drivers/staging/nanohub/chub.h index fa3ca2b1a6b0..aa080694b23d 100644 --- a/drivers/staging/nanohub/chub.h +++ b/drivers/staging/nanohub/chub.h @@ -117,6 +117,10 @@ enum CHUB_ERR_TYPE { CHUB_ERR_WRITE_FAIL, CHUB_ERR_EVTQ_NO_HW_TRIGGER, CHUB_ERR_CHUB_NO_RESPONSE, + CHUB_ERR_COMMS_NACK, + CHUB_ERR_COMMS_BUSY, + CHUB_ERR_COMMS_UNKNOWN, + CHUB_ERR_COMMS, CHUB_ERR_NANOHUB_FAULT, /* chub error */ CHUB_ERR_NANOHUB_ASSERT, CHUB_ERR_NANOHUB_ERROR, @@ -159,6 +163,7 @@ struct contexthub_ipc_info { atomic_t chub_status; atomic_t irq1_apInt; atomic_t wakeup_chub; + spinlock_t reset_lock; int irq_mailbox; int irq_wdt; int err_cnt[CHUB_ERR_MAX]; diff --git a/drivers/staging/nanohub/chub_dbg.c b/drivers/staging/nanohub/chub_dbg.c index 8a0a3d2d2718..84daddebb1b9 100644 --- a/drivers/staging/nanohub/chub_dbg.c +++ b/drivers/staging/nanohub/chub_dbg.c @@ -108,8 +108,6 @@ static void chub_dbg_write_file(struct device *dev, char *name, void *buf, int s out: set_fs(old_fs); } -#else -#define chub_dbg_write_file(a) do { } while (0) #endif void chub_dbg_dump_hw(struct contexthub_ipc_info *ipc, int reason) @@ -132,6 +130,7 @@ void chub_dbg_dump_hw(struct contexthub_ipc_info *ipc, int reason) ipc_get_base(IPC_REG_DUMP), ipc_get_chub_mem_size()); +#ifdef CONFIG_CONTEXTHUB_DEBUG /* write file */ dev_dbg(ipc->dev, "%s: write file: sram:%p, dram:%p(off:%d), size:%d\n", @@ -143,6 +142,7 @@ void chub_dbg_dump_hw(struct contexthub_ipc_info *ipc, int reason) chub_dbg_write_file(ipc->dev, "sram", &p_dbg_dump->sram[p_dbg_dump->sram_start], ipc_get_chub_mem_size()); +#endif } contexthub_release(ipc); } @@ -405,6 +405,7 @@ static ssize_t chub_get_dump_status_store(struct device *dev, return 0; } + chub_dbg_dump_status(ipc); contexthub_ipc_write_event(ipc, MAILBOX_EVT_DUMP_STATUS); contexthub_release(ipc); diff --git a/drivers/staging/nanohub/chub_ipc.c b/drivers/staging/nanohub/chub_ipc.c index 03037af04a69..23c5d81fd18f 100644 --- a/drivers/staging/nanohub/chub_ipc.c +++ b/drivers/staging/nanohub/chub_ipc.c @@ -252,8 +252,8 @@ inline void ipc_copy_bytes(u8 *dst, u8 *src, int size) static inline int ipc_io_data(enum ipc_data_list dir, u8 *buf, u16 length) { struct ipc_buf *ipc_data; - int eq; - int dq; + u32 eq; + u32 dq; int useful = 0; u8 size_lower; u8 size_upper; @@ -277,6 +277,14 @@ static inline int ipc_io_data(enum ipc_data_list dir, u8 *buf, u16 length) eq = ipc_data->eq; dq = ipc_data->dq; + /* check index due to sram corruption */ + if ((eq > IPC_DATA_SIZE) || (dq > IPC_DATA_SIZE) || + (ipc_data->full > 1) || (ipc_data->empty > 1)) { + CSP_PRINTF_ERROR("%s: invalid index:%d, %d, %d, %d\n", + __func__, eq, dq, ipc_data->full, ipc_data->empty); + return -1; + } + #ifdef USE_IPC_BUF_LOG CSP_PRINTF_INFO("%s: dir:%s(w:%d, r:%d, cnt:%d), e:%d d:%d, empty:%d, full:%d, ipc_data:%p, len:%d\n", __func__, dir ? "a2c" : "c2a", ipc_data->cnt_dbg_wt, diff --git a/drivers/staging/nanohub/comms.c b/drivers/staging/nanohub/comms.c index 8beb5b60d673..59be58fc073d 100644 --- a/drivers/staging/nanohub/comms.c +++ b/drivers/staging/nanohub/comms.c @@ -285,6 +285,28 @@ static int read_msg(struct nanohub_data *data, struct nanohub_packet *response, return ret; } +#ifdef CONFIG_NANOHUB_MAILBOX /* remove invalid error check */ +static inline void contexthub_update_result(struct nanohub_data *data, int err) +{ + struct contexthub_ipc_info *ipc = data->pdata->mailbox_client; + + if (!err) + ipc->err_cnt[CHUB_ERR_COMMS] = 0; + else { + ipc->err_cnt[CHUB_ERR_COMMS]++; + + if (err == ERROR_NACK) + ipc->err_cnt[CHUB_ERR_COMMS_NACK]++; + else if (err == ERROR_BUSY) + ipc->err_cnt[CHUB_ERR_COMMS_BUSY]++; + else + ipc->err_cnt[CHUB_ERR_COMMS_BUSY]++; + } +} +#else +#define contexthub_update_err_cnt(a, b) do { } while (0) +#endif + static int get_reply(struct nanohub_data *data, struct nanohub_packet *response, uint32_t seq) { @@ -323,6 +345,8 @@ static int get_reply(struct nanohub_data *data, struct nanohub_packet *response, ret = ERROR_NACK; else if (response->reason == CMD_COMMS_BUSY) ret = ERROR_BUSY; + else + pr_warn("nanohub: invalid reason %d\n", __func__, response->reason); } if (response->seq != seq) @@ -343,6 +367,8 @@ static int get_reply(struct nanohub_data *data, struct nanohub_packet *response, b[i + 24]); } ret = ERROR_NACK; + pr_warn("nanohub: invalid seq %d\n", __func__, + response->reason); } return ret; @@ -378,6 +404,7 @@ static int nanohub_comms_tx_rx(struct nanohub_data *data, ret = ERROR_NACK; } + contexthub_update_result(data, ret); return ret; } -- 2.20.1