[COMMON] chub: fix chub driver kernel panic
authorBoojin Kim <boojin.kim@samsung.com>
Thu, 28 Jun 2018 03:35:14 +0000 (12:35 +0900)
committerYoungmin Nam <youngmin.nam@samsung.com>
Fri, 29 Jun 2018 09:14:25 +0000 (18:14 +0900)
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 <boojin.kim@samsung.com>
drivers/staging/nanohub/Makefile
drivers/staging/nanohub/chub.c
drivers/staging/nanohub/chub.h
drivers/staging/nanohub/chub_dbg.c
drivers/staging/nanohub/chub_ipc.c
drivers/staging/nanohub/comms.c

index d5614a7321b97cff299bef2a3de916642cbf29f6..37676ff2e7856198269e2276219e56be43331623 100644 (file)
@@ -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
index 5eba4dcf9612c7b0fcbf14d201f8474056ac08c7..58965bfc463da922d3132329047e4fc8395c7031 100644 (file)
@@ -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);
index fa3ca2b1a6b0fe9526fc980f2900d7b065beccd1..aa080694b23d43dc0376e016c62829c9294d0f07 100644 (file)
@@ -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];
index 8a0a3d2d2718a2769a4cd721320a4563965e217a..84daddebb1b9f9ccc12a40cdcd30372f7cfbc0ec 100644 (file)
@@ -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);
index 03037af04a6961f3586b2f7665d6b706cac21253..23c5d81fd18fc3eb357c59016f5aabfee9617185 100644 (file)
@@ -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,
index 8beb5b60d6736a493dfdb037d7a0a61dbabf60e4..59be58fc073dbda99ac29276020ef2adc8ca50d6 100644 (file)
@@ -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;
 }