From 3396e5da3621e8495f9f8d823d00786e3f2cf0f0 Mon Sep 17 00:00:00 2001 From: Jilai Wang Date: Thu, 25 Jan 2024 11:41:04 -0500 Subject: [PATCH] msm: npu v1: Fix OOB issue in IPC between driver and firmware We shoudn't trust the data from firmware, and need to validate all content before using them. Change-Id: I39b58f3d482931a932dab7ca4b8cc3e4d9086b36 Signed-off-by: Jilai Wang --- drivers/media/platform/msm/npu/npu_common.h | 6 -- drivers/media/platform/msm/npu/npu_debugfs.c | 70 ------------------- drivers/media/platform/msm/npu/npu_host_ipc.c | 46 +++++++++--- .../media/platform/msm/npu/npu_hw_access.c | 46 ------------ .../media/platform/msm/npu/npu_hw_access.h | 3 - drivers/media/platform/msm/npu/npu_mgr.c | 60 ++-------------- 6 files changed, 43 insertions(+), 188 deletions(-) diff --git a/drivers/media/platform/msm/npu/npu_common.h b/drivers/media/platform/msm/npu/npu_common.h index 18d896c98e96..1f18e052cc62 100644 --- a/drivers/media/platform/msm/npu/npu_common.h +++ b/drivers/media/platform/msm/npu/npu_common.h @@ -94,12 +94,6 @@ struct npu_debugfs_ctx { struct dentry *root; uint32_t reg_off; uint32_t reg_cnt; - uint8_t *log_buf; - struct mutex log_lock; - uint32_t log_num_bytes_buffered; - uint32_t log_read_index; - uint32_t log_write_index; - uint32_t log_buf_size; }; struct npu_debugfs_reg_ctx { diff --git a/drivers/media/platform/msm/npu/npu_debugfs.c b/drivers/media/platform/msm/npu/npu_debugfs.c index 18c8df0a4bb0..c813f3657b03 100644 --- a/drivers/media/platform/msm/npu/npu_debugfs.c +++ b/drivers/media/platform/msm/npu/npu_debugfs.c @@ -27,8 +27,6 @@ */ static int npu_debug_open(struct inode *inode, struct file *file); static int npu_debug_release(struct inode *inode, struct file *file); -static ssize_t npu_debug_log_read(struct file *file, - char __user *user_buf, size_t count, loff_t *ppos); static ssize_t npu_debug_ctrl_write(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos); @@ -38,13 +36,6 @@ static ssize_t npu_debug_ctrl_write(struct file *file, */ static struct npu_device *g_npu_dev; -static const struct file_operations npu_log_fops = { - .open = npu_debug_open, - .release = npu_debug_release, - .read = npu_debug_log_read, - .write = NULL, -}; - static const struct file_operations npu_ctrl_fops = { .open = npu_debug_open, .release = npu_debug_release, @@ -69,47 +60,6 @@ static int npu_debug_release(struct inode *inode, struct file *file) return 0; } -/* ------------------------------------------------------------------------- - * Function Implementations - DebugFS Log - * ------------------------------------------------------------------------- - */ -static ssize_t npu_debug_log_read(struct file *file, - char __user *user_buf, size_t count, loff_t *ppos) -{ - size_t len = 0; - struct npu_device *npu_dev = file->private_data; - struct npu_debugfs_ctx *debugfs; - - pr_debug("npu_dev %pK %pK\n", npu_dev, g_npu_dev); - npu_dev = g_npu_dev; - debugfs = &npu_dev->debugfs_ctx; - - /* mutex log lock */ - mutex_lock(&debugfs->log_lock); - - if (debugfs->log_num_bytes_buffered != 0) { - len = min(debugfs->log_num_bytes_buffered, - debugfs->log_buf_size - debugfs->log_read_index); - len = min(count, len); - if (copy_to_user(user_buf, (debugfs->log_buf + - debugfs->log_read_index), len)) { - pr_err("%s failed to copy to user\n", __func__); - mutex_unlock(&debugfs->log_lock); - return -EFAULT; - } - debugfs->log_read_index += len; - if (debugfs->log_read_index == debugfs->log_buf_size) - debugfs->log_read_index = 0; - - debugfs->log_num_bytes_buffered -= len; - *ppos += len; - } - - /* mutex log unlock */ - mutex_unlock(&debugfs->log_lock); - - return len; -} /* ------------------------------------------------------------------------- * Function Implementations - DebugFS Control @@ -193,12 +143,6 @@ int npu_debugfs_init(struct npu_device *npu_dev) return -ENODEV; } - if (!debugfs_create_file("log", 0644, debugfs->root, - npu_dev, &npu_log_fops)) { - pr_err("debugfs_create_file log fail\n"); - goto err; - } - if (!debugfs_create_file("ctrl", 0644, debugfs->root, npu_dev, &npu_ctrl_fops)) { pr_err("debugfs_create_file ctrl fail\n"); @@ -235,14 +179,6 @@ int npu_debugfs_init(struct npu_device *npu_dev) goto err; } - debugfs->log_num_bytes_buffered = 0; - debugfs->log_read_index = 0; - debugfs->log_write_index = 0; - debugfs->log_buf_size = NPU_LOG_BUF_SIZE; - debugfs->log_buf = kzalloc(debugfs->log_buf_size, GFP_KERNEL); - if (!debugfs->log_buf) - goto err; - mutex_init(&debugfs->log_lock); return 0; @@ -255,12 +191,6 @@ void npu_debugfs_deinit(struct npu_device *npu_dev) { struct npu_debugfs_ctx *debugfs = &npu_dev->debugfs_ctx; - debugfs->log_num_bytes_buffered = 0; - debugfs->log_read_index = 0; - debugfs->log_write_index = 0; - debugfs->log_buf_size = 0; - kfree(debugfs->log_buf); - if (!IS_ERR_OR_NULL(debugfs->root)) { debugfs_remove_recursive(debugfs->root); debugfs->root = NULL; diff --git a/drivers/media/platform/msm/npu/npu_host_ipc.c b/drivers/media/platform/msm/npu/npu_host_ipc.c index 940a35d9ada8..2dc80c2c2784 100644 --- a/drivers/media/platform/msm/npu/npu_host_ipc.c +++ b/drivers/media/platform/msm/npu/npu_host_ipc.c @@ -33,15 +33,16 @@ struct npu_queue_tuple { uint32_t size; uint32_t hdr; + uint32_t start_offset; }; -static const struct npu_queue_tuple npu_q_setup[6] = { - { 1024, IPC_QUEUE_CMD_HIGH_PRIORITY | TX_HDR_TYPE | RX_HDR_TYPE }, - { 4096, IPC_QUEUE_APPS_EXEC | TX_HDR_TYPE | RX_HDR_TYPE }, - { 4096, IPC_QUEUE_DSP_EXEC | TX_HDR_TYPE | RX_HDR_TYPE }, - { 4096, IPC_QUEUE_APPS_RSP | TX_HDR_TYPE | RX_HDR_TYPE }, - { 4096, IPC_QUEUE_DSP_RSP | TX_HDR_TYPE | RX_HDR_TYPE }, - { 1024, IPC_QUEUE_LOG | TX_HDR_TYPE | RX_HDR_TYPE }, +static struct npu_queue_tuple npu_q_setup[6] = { + { 1024, IPC_QUEUE_CMD_HIGH_PRIORITY | TX_HDR_TYPE | RX_HDR_TYPE, 0}, + { 4096, IPC_QUEUE_APPS_EXEC | TX_HDR_TYPE | RX_HDR_TYPE, 0}, + { 4096, IPC_QUEUE_DSP_EXEC | TX_HDR_TYPE | RX_HDR_TYPE, 0}, + { 4096, IPC_QUEUE_APPS_RSP | TX_HDR_TYPE | RX_HDR_TYPE, 0}, + { 4096, IPC_QUEUE_DSP_RSP | TX_HDR_TYPE | RX_HDR_TYPE, 0}, + { 1024, IPC_QUEUE_LOG | TX_HDR_TYPE | RX_HDR_TYPE, 0}, }; /* ------------------------------------------------------------------------- @@ -111,6 +112,7 @@ static int npu_host_ipc_init_hfi(struct npu_device *npu_dev) /* queue is active */ q_hdr->qhdr_status = 0x01; q_hdr->qhdr_start_offset = cur_start_offset; + npu_q_setup[q_idx].start_offset = cur_start_offset; q_size = npu_q_setup[q_idx].size; q_hdr->qhdr_type = npu_q_setup[q_idx].hdr; /* in bytes */ @@ -212,6 +214,18 @@ static int ipc_queue_read(struct npu_device *npu_dev, /* Read the queue */ MEMR(npu_dev, (void *)((size_t)offset), (uint8_t *)&queue, HFI_QUEUE_HEADER_SIZE); + + if (queue.qhdr_type != npu_q_setup[target_que].hdr || + queue.qhdr_q_size != npu_q_setup[target_que].size || + queue.qhdr_read_idx >= queue.qhdr_q_size || + queue.qhdr_write_idx >= queue.qhdr_q_size || + queue.qhdr_start_offset != + npu_q_setup[target_que].start_offset) { + pr_err("Invalid Queue header\n"); + status = -EIO; + goto exit; + } + /* check if queue is empty */ if (queue.qhdr_read_idx == queue.qhdr_write_idx) { /* @@ -235,8 +249,10 @@ static int ipc_queue_read(struct npu_device *npu_dev, target_que, packet_size); - if (packet_size == 0) { - status = -EPERM; + if ((packet_size == 0) || + (packet_size > NPU_IPC_BUF_LENGTH)) { + pr_err("Invalid packet size %d\n", packet_size); + status = -EINVAL; goto exit; } new_read_idx = queue.qhdr_read_idx + packet_size; @@ -306,6 +322,18 @@ static int ipc_queue_write(struct npu_device *npu_dev, MEMR(npu_dev, (void *)((size_t)offset), (uint8_t *)&queue, HFI_QUEUE_HEADER_SIZE); + + if (queue.qhdr_type != npu_q_setup[target_que].hdr || + queue.qhdr_q_size != npu_q_setup[target_que].size || + queue.qhdr_read_idx >= queue.qhdr_q_size || + queue.qhdr_write_idx >= queue.qhdr_q_size || + queue.qhdr_start_offset != + npu_q_setup[target_que].start_offset) { + pr_err("Invalid Queue header\n"); + status = -EIO; + goto exit; + } + packet_size = (*(uint32_t *)packet); if (packet_size == 0) { /* assign failed status and return */ diff --git a/drivers/media/platform/msm/npu/npu_hw_access.c b/drivers/media/platform/msm/npu/npu_hw_access.c index 3b7a420a1d11..113407d34a21 100644 --- a/drivers/media/platform/msm/npu/npu_hw_access.c +++ b/drivers/media/platform/msm/npu/npu_hw_access.c @@ -442,49 +442,3 @@ void subsystem_put_local(void *sub_system_handle) return subsystem_put(sub_system_handle); } -/* ------------------------------------------------------------------------- - * Functions - Log - * ------------------------------------------------------------------------- - */ -void npu_process_log_message(struct npu_device *npu_dev, uint32_t *message, - uint32_t size) -{ - struct npu_debugfs_ctx *debugfs = &npu_dev->debugfs_ctx; - - /* mutex log lock */ - mutex_lock(&debugfs->log_lock); - - if ((debugfs->log_num_bytes_buffered + size) > - debugfs->log_buf_size) { - /* No more space, invalidate it all and start over */ - debugfs->log_read_index = 0; - debugfs->log_write_index = size; - debugfs->log_num_bytes_buffered = size; - memcpy(debugfs->log_buf, message, size); - } else { - if ((debugfs->log_write_index + size) > - debugfs->log_buf_size) { - /* Wrap around case */ - uint8_t *src_addr = (uint8_t *)message; - uint8_t *dst_addr = NULL; - uint32_t remaining_to_end = debugfs->log_buf_size - - debugfs->log_write_index + 1; - dst_addr = debugfs->log_buf + debugfs->log_write_index; - memcpy(dst_addr, src_addr, remaining_to_end); - src_addr = &(src_addr[remaining_to_end]); - dst_addr = debugfs->log_buf; - memcpy(dst_addr, src_addr, size-remaining_to_end); - debugfs->log_write_index = size-remaining_to_end; - } else { - memcpy((debugfs->log_buf + debugfs->log_write_index), - message, size); - debugfs->log_write_index += size; - if (debugfs->log_write_index == debugfs->log_buf_size) - debugfs->log_write_index = 0; - } - debugfs->log_num_bytes_buffered += size; - } - - /* mutex log unlock */ - mutex_unlock(&debugfs->log_lock); -} diff --git a/drivers/media/platform/msm/npu/npu_hw_access.h b/drivers/media/platform/msm/npu/npu_hw_access.h index 311f4fb84870..06ed0e6d132d 100644 --- a/drivers/media/platform/msm/npu/npu_hw_access.h +++ b/drivers/media/platform/msm/npu/npu_hw_access.h @@ -83,7 +83,4 @@ void npu_disable_sys_cache(struct npu_device *npu_dev); void *subsystem_get_local(char *sub_system); void subsystem_put_local(void *sub_system_handle); -void npu_process_log_message(struct npu_device *npu_dev, uint32_t *msg, - uint32_t size); - #endif /* _NPU_HW_ACCESS_H*/ diff --git a/drivers/media/platform/msm/npu/npu_mgr.c b/drivers/media/platform/msm/npu/npu_mgr.c index f36aff27ccc5..6b017934f985 100644 --- a/drivers/media/platform/msm/npu/npu_mgr.c +++ b/drivers/media/platform/msm/npu/npu_mgr.c @@ -20,11 +20,6 @@ * Defines * ------------------------------------------------------------------------- */ -#define LOG_MSG_HEADER_SIZE 20 -#define LOG_MSG_START_MSG_INDEX 5 -#define LOG_MSG_TOTAL_SIZE_INDEX 0 -#define LOG_MSG_MSG_ID_INDEX 1 - #define NPU_FW_TIMEOUT_POLL_INTERVAL_MS 10 #define NPU_FW_TIMEOUT_MS 1000 @@ -48,9 +43,7 @@ static void free_network(struct npu_host_ctx *ctx, struct npu_client *client, static int network_get(struct npu_network *network); static int network_put(struct npu_network *network); static void app_msg_proc(struct npu_host_ctx *host_ctx, uint32_t *msg); -static void log_msg_proc(struct npu_device *npu_dev, uint32_t *msg); static void host_session_msg_hdlr(struct npu_device *npu_dev); -static void host_session_log_hdlr(struct npu_device *npu_dev); static int host_error_hdlr(struct npu_device *npu_dev, bool force); static int npu_send_network_cmd(struct npu_device *npu_dev, struct npu_network *network, void *cmd_ptr); @@ -391,7 +384,6 @@ static void host_irq_wq(struct work_struct *work) if (host_error_hdlr(npu_dev, false)) return; - host_session_log_hdlr(npu_dev); host_session_msg_hdlr(npu_dev); } @@ -768,6 +760,12 @@ static void app_msg_proc(struct npu_host_ctx *host_ctx, uint32_t *msg) } pr_debug("network id : %llu\n", network->id); + if (exe_rsp_pkt->header.size < sizeof(*exe_rsp_pkt)) { + pr_err("invalid packet header size, header.size: %d\n", + exe_rsp_pkt->header.size); + network_put(network); + break; + } stats_size = exe_rsp_pkt->header.size - sizeof(*exe_rsp_pkt); pr_debug("stats_size %d:%d\n", exe_rsp_pkt->header.size, stats_size); @@ -986,52 +984,6 @@ skip_read_msg: kfree(msg); } -static void log_msg_proc(struct npu_device *npu_dev, uint32_t *msg) -{ - uint32_t msg_id; - uint32_t *log_msg; - uint32_t size; - - msg_id = msg[LOG_MSG_MSG_ID_INDEX]; - size = msg[LOG_MSG_TOTAL_SIZE_INDEX] - LOG_MSG_HEADER_SIZE; - - switch (msg_id) { - case NPU_IPC_MSG_EVENT_NOTIFY: - /* Process the message */ - log_msg = &(msg[LOG_MSG_START_MSG_INDEX]); - npu_process_log_message(npu_dev, log_msg, size); - break; - default: - pr_err("unsupported log response received %d\n", msg_id); - break; - } -} - -static void host_session_log_hdlr(struct npu_device *npu_dev) -{ - uint32_t *msg; - struct npu_host_ctx *host_ctx = &npu_dev->host_ctx; - - msg = kzalloc(sizeof(uint32_t) * NPU_IPC_BUF_LENGTH, GFP_KERNEL); - - if (!msg) - return; - - mutex_lock(&host_ctx->lock); - if (host_ctx->fw_state == FW_DISABLED) { - pr_warn("handle npu session msg when FW is disabled\n"); - goto skip_read_msg; - } - - while (npu_host_ipc_read_msg(npu_dev, IPC_QUEUE_LOG, msg) == 0) { - pr_debug("received from log queue\n"); - log_msg_proc(npu_dev, msg); - } - -skip_read_msg: - mutex_unlock(&host_ctx->lock); - kfree(msg); -} /* ------------------------------------------------------------------------- * Function Definitions - Functionality