From c60ac212aabd299304dfbb54b1fc18c59247d9ae Mon Sep 17 00:00:00 2001 From: Ramesh Nallagopu Date: Fri, 28 Jun 2024 22:17:36 +0530 Subject: [PATCH] dsp-kernel: Fix to avoid untrusted pointer dereference Currently, the compat ioctl call distinguishes itself using a global flag. If a user sends a compat ioctl call followed by a normal ioctl call, it may result in using a user passed address as a kernel address in the fastrpcdriver. To address this issue, consider localizing the compat flag for the ioctl call. Change-Id: Ie8fc724424534102736b8c0bc594720547ab6ff6 Signed-off-by: rnallago --- dsp/adsprpc.c | 49 ++++++++++++++++++++++---------------------- dsp/adsprpc_compat.c | 5 ++--- dsp/adsprpc_shared.h | 9 +++++--- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/dsp/adsprpc.c b/dsp/adsprpc.c index 74edaac4e7..3086e9ab6c 100644 --- a/dsp/adsprpc.c +++ b/dsp/adsprpc.c @@ -1838,11 +1838,11 @@ static int context_alloc(struct fastrpc_file *fl, uint32_t kernel, struct fastrpc_ioctl_invoke *invoke = &invokefd->inv; struct fastrpc_channel_ctx *chan = NULL; unsigned long irq_flags = 0; - uint32_t is_kernel_memory = 0; + uint32_t kernel_msg = ((kernel == COMPAT_MSG) ? USER_MSG : kernel); spin_lock(&fl->hlock); if (fl->clst.num_active_ctxs > MAX_PENDING_CTX_PER_SESSION && - !(kernel || invoke->handle < FASTRPC_STATIC_HANDLE_MAX)) { + !(kernel_msg || invoke->handle < FASTRPC_STATIC_HANDLE_MAX)) { err = -EDQUOT; spin_unlock(&fl->hlock); goto bail; @@ -1873,12 +1873,7 @@ static int context_alloc(struct fastrpc_file *fl, uint32_t kernel, ctx->overs = (struct overlap *)(&ctx->attrs[bufs]); ctx->overps = (struct overlap **)(&ctx->overs[bufs]); - /* If user message, do not use copy_from_user to copy buffers for - * compat driver,as memory is already copied to kernel memory - * for compat driver - */ - is_kernel_memory = ((kernel == USER_MSG) ? (fl->is_compat) : kernel); - K_COPY_FROM_USER(err, is_kernel_memory, (void *)ctx->lpra, invoke->pra, + K_COPY_FROM_USER(err, kernel, (void *)ctx->lpra, invoke->pra, bufs * sizeof(*ctx->lpra)); if (err) { ADSPRPC_ERR( @@ -1961,7 +1956,15 @@ static int context_alloc(struct fastrpc_file *fl, uint32_t kernel, spin_lock_irqsave(&chan->ctxlock, irq_flags); me->jobid[cid]++; - for (ii = ((kernel || ctx->handle < FASTRPC_STATIC_HANDLE_MAX) + + /* + * To prevent user invocations from exhausting all entries in context + * table, it is necessary to reserve a few context table entries for + * critical kernel and static RPC calls. The index will begin at 0 for + * static handles, while user handles start from + * NUM_KERNEL_AND_STATIC_ONLY_CONTEXTS. + */ + for (ii = ((kernel_msg || ctx->handle < FASTRPC_STATIC_HANDLE_MAX) ? 0 : NUM_KERNEL_AND_STATIC_ONLY_CONTEXTS); ii < FASTRPC_CTX_MAX; ii++) { if (!chan->ctxtable[ii]) { @@ -3342,7 +3345,7 @@ static void fastrpc_update_invoke_count(uint32_t handle, uint64_t *perf_counter, static int fastrpc_check_pd_status(struct fastrpc_file *fl, char *sloc_name); int fastrpc_internal_invoke(struct fastrpc_file *fl, uint32_t mode, - uint32_t kernel, + uint32_t msg_type, struct fastrpc_ioctl_invoke_async *inv) { struct smq_invoke_ctx *ctx = NULL; @@ -3351,6 +3354,7 @@ int fastrpc_internal_invoke(struct fastrpc_file *fl, uint32_t mode, struct timespec64 invoket = {0}; uint64_t *perf_counter = NULL; bool isasyncinvoke = false, isworkdone = false; + uint32_t kernel = (msg_type == COMPAT_MSG) ? USER_MSG : msg_type; cid = fl->cid; VERIFY(err, VALID_FASTRPC_CID(cid) && @@ -3377,9 +3381,6 @@ int fastrpc_internal_invoke(struct fastrpc_file *fl, uint32_t mode, cid, invoke->handle); goto bail; } - } - - if (!kernel) { VERIFY(err, 0 == (err = context_restore_interrupted(fl, inv, &ctx))); if (err) @@ -3397,7 +3398,7 @@ int fastrpc_internal_invoke(struct fastrpc_file *fl, uint32_t mode, } trace_fastrpc_msg("context_alloc: begin"); - VERIFY(err, 0 == (err = context_alloc(fl, kernel, inv, &ctx))); + VERIFY(err, 0 == (err = context_alloc(fl, msg_type, inv, &ctx))); trace_fastrpc_msg("context_alloc: end"); if (err) goto bail; @@ -3819,7 +3820,7 @@ bail: } int fastrpc_internal_invoke2(struct fastrpc_file *fl, - struct fastrpc_ioctl_invoke2 *inv2) + struct fastrpc_ioctl_invoke2 *inv2, bool is_compat) { union { struct fastrpc_ioctl_invoke_async inv; @@ -3831,7 +3832,7 @@ int fastrpc_internal_invoke2(struct fastrpc_file *fl, struct fastrpc_proc_sess_info sess_info; } p; struct fastrpc_dsp_capabilities *dsp_cap_ptr = NULL; - uint32_t size = 0; + uint32_t size = 0, kernel = 0; int err = 0, domain = fl->cid; if (inv2->req == FASTRPC_INVOKE2_ASYNC || @@ -3859,19 +3860,20 @@ int fastrpc_internal_invoke2(struct fastrpc_file *fl, goto bail; } if (size > inv2->size) { - K_COPY_FROM_USER(err, fl->is_compat, &p.inv3, (void *)inv2->invparam, + K_COPY_FROM_USER(err, is_compat, &p.inv3, (void *)inv2->invparam, sizeof(struct fastrpc_ioctl_invoke_async_no_perf)); if (err) goto bail; memcpy(&p.inv, &p.inv3, sizeof(struct fastrpc_ioctl_invoke_crc)); memcpy(&p.inv.job, &p.inv3.job, sizeof(p.inv.job)); } else { - K_COPY_FROM_USER(err, fl->is_compat, &p.inv, (void *)inv2->invparam, size); + K_COPY_FROM_USER(err, is_compat, &p.inv, (void *)inv2->invparam, size); if (err) goto bail; } + kernel = (is_compat) ? COMPAT_MSG : USER_MSG; VERIFY(err, 0 == (err = fastrpc_internal_invoke(fl, fl->mode, - USER_MSG, &p.inv))); + kernel, &p.inv))); if (err) goto bail; break; @@ -3891,7 +3893,7 @@ int fastrpc_internal_invoke2(struct fastrpc_file *fl, err = -EBADE; goto bail; } - K_COPY_FROM_USER(err, 0, &p.user_concurrency, + K_COPY_FROM_USER(err, is_compat, &p.user_concurrency, (void *)inv2->invparam, size); if (err) goto bail; @@ -3915,7 +3917,7 @@ int fastrpc_internal_invoke2(struct fastrpc_file *fl, err = -EBADE; goto bail; } - K_COPY_FROM_USER(err, fl->is_compat, &p.buff_info, + K_COPY_FROM_USER(err, 0, &p.buff_info, (void *)inv2->invparam, inv2->size); if (err) goto bail; @@ -3930,7 +3932,7 @@ int fastrpc_internal_invoke2(struct fastrpc_file *fl, err = -EBADE; goto bail; } - K_COPY_FROM_USER(err, fl->is_compat, &p.sess_info, + K_COPY_FROM_USER(err, is_compat, &p.sess_info, (void *)inv2->invparam, inv2->size); if (err) goto bail; @@ -6499,7 +6501,6 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) fl->dsp_proc_init = 0; fl->dsp_process_state = PROCESS_CREATE_DEFAULT; fl->is_unsigned_pd = false; - fl->is_compat = false; fl->exit_notif = false; fl->exit_async = false; fl->multi_session_support = false; @@ -7499,7 +7500,7 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int ioctl_num, err = -EFAULT; goto bail; } - VERIFY(err, 0 == (err = fastrpc_internal_invoke2(fl, &p.inv2))); + VERIFY(err, 0 == (err = fastrpc_internal_invoke2(fl, &p.inv2, false))); if (err) goto bail; break; diff --git a/dsp/adsprpc_compat.c b/dsp/adsprpc_compat.c index 3cc60e220a..7c7b1e59aa 100644 --- a/dsp/adsprpc_compat.c +++ b/dsp/adsprpc_compat.c @@ -347,7 +347,7 @@ static int compat_fastrpc_ioctl_invoke(struct file *filp, if (err) return err; VERIFY(err, 0 == (err = fastrpc_internal_invoke(fl, - fl->mode, USER_MSG, inv))); + fl->mode, COMPAT_MSG, inv))); return err; } @@ -484,7 +484,7 @@ static int compat_fastrpc_ioctl_invoke2(struct file *filp, if (err) return err; - VERIFY(err, 0 == (err = fastrpc_internal_invoke2(fl, inv))); + VERIFY(err, 0 == (err = fastrpc_internal_invoke2(fl, inv, true))); return err; } @@ -975,7 +975,6 @@ long compat_fastrpc_device_ioctl(struct file *filp, unsigned int cmd, if (!filp->f_op || !filp->f_op->unlocked_ioctl) return -ENOTTY; - fl->is_compat = true; switch (cmd) { case COMPAT_FASTRPC_IOCTL_INVOKE: case COMPAT_FASTRPC_IOCTL_INVOKE_FD: diff --git a/dsp/adsprpc_shared.h b/dsp/adsprpc_shared.h index 651eded3ac..cd8ed472ff 100644 --- a/dsp/adsprpc_shared.h +++ b/dsp/adsprpc_shared.h @@ -469,9 +469,14 @@ enum fastrpc_buf_type { /* Types of RPC calls to DSP */ enum fastrpc_msg_type { + /* 64 bit user application invoke message */ USER_MSG = 0, + /* kernel invoke message with zero pid */ KERNEL_MSG_WITH_ZERO_PID, + /* kernel invoke message with non zero pid to kill the PD in DSP */ KERNEL_MSG_WITH_NONZERO_PID, + /* 32 bit user application invoke message */ + COMPAT_MSG, }; /* Fastrpc remote pd type */ @@ -911,8 +916,6 @@ struct fastrpc_file { /* Flag to indicate dynamic process creation status*/ enum fastrpc_process_create_state dsp_process_state; bool is_unsigned_pd; - /* Flag to indicate 32 bit driver*/ - bool is_compat; /* Completion objects and state for dspsignals */ struct fastrpc_dspsignal *signal_groups[DSPSIGNAL_NUM_SIGNALS / DSPSIGNAL_GROUP_SIZE]; spinlock_t dspsignals_lock; @@ -939,7 +942,7 @@ int fastrpc_internal_invoke(struct fastrpc_file *fl, uint32_t mode, struct fastrpc_ioctl_invoke_async *inv); int fastrpc_internal_invoke2(struct fastrpc_file *fl, - struct fastrpc_ioctl_invoke2 *inv2); + struct fastrpc_ioctl_invoke2 *inv2, bool is_compat); int fastrpc_internal_munmap(struct fastrpc_file *fl, struct fastrpc_ioctl_munmap *ud);