| Message ID | 20241007084518.3649876-2-quic_ekangupt@quicinc.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Add invokeV2 to support new features | expand |
On Mon, Oct 07, 2024 at 02:15:15PM +0530, Ekansh Gupta wrote: > +struct fastrpc_invoke_v2 { > + struct fastrpc_invoke inv; > + __u64 crc; > + __u32 reserved[16]; I'm sure we have been over this before, but if you have a reserved field, you HAVE to check that it is zero if you ever want to use it in the future. As you did not check it, just remove it from this structure as obviously it can never be used for anything. Your internal review should have caught this, if not, please go fix it AND get some internal review and signed-off-by before submitting this series again. thanks, greg k-h
On Mon, Oct 07, 2024 at 02:15:15PM GMT, Ekansh Gupta wrote: > InvokeV2 request is intended to support multiple enhanced invoke > requests like CRC check, performance counter enablement and polling > mode for RPC invocations. CRC check is getting enabled as part of > this patch. CRC check for input and output argument helps in ensuring > data consistency over a remote call. If user intends to enable CRC > check, first local user CRC is calculated at user end and a CRC buffer > is passed to DSP to capture remote CRC values. DSP is expected to > write to the remote CRC buffer which is then compared at user level > with the local CRC values. This doesn't explain why this is necessary. Why do you need to checksum arguments? Also, what if the DSP firmware doesn't support CRC? How should userspace know that? > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> > --- > drivers/misc/fastrpc.c | 161 ++++++++++++++++++++++++------------ > include/uapi/misc/fastrpc.h | 7 ++ > 2 files changed, 116 insertions(+), 52 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 74181b8c386b..8e817a763d1d 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -573,13 +573,15 @@ static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx) > > static struct fastrpc_invoke_ctx *fastrpc_context_alloc( > struct fastrpc_user *user, u32 kernel, u32 sc, > - struct fastrpc_invoke_args *args) > + struct fastrpc_invoke_v2 *inv2) > { > struct fastrpc_channel_ctx *cctx = user->cctx; > struct fastrpc_invoke_ctx *ctx = NULL; > + struct fastrpc_invoke_args *args = NULL; Why do you need to init to NULL if you are going to set it two lines below? > unsigned long flags; > int ret; > > + args = (struct fastrpc_invoke_args *)inv2->inv.args; Why does it need a typecast? > ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); > if (!ctx) > return ERR_PTR(-ENOMEM); > @@ -611,6 +613,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( > /* Released in fastrpc_context_put() */ > fastrpc_channel_ctx_get(cctx); > > + ctx->crc = (u32 *)(uintptr_t)inv2->crc; Oh, but why? Also is it a user pointer or in-kernel data? If it's a user-based pointer, where is the accessiblity check? Why isn't it annotated properly? > ctx->sc = sc; > ctx->retval = -1; > ctx->pid = current->pid; > @@ -1070,6 +1073,7 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, > struct fastrpc_invoke_buf *list; > struct fastrpc_phy_page *pages; > u64 *fdlist; > + u32 *crclist; > int i, inbufs, outbufs, handles; > > inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); > @@ -1078,6 +1082,7 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, > list = fastrpc_invoke_buf_start(rpra, ctx->nscalars); > pages = fastrpc_phy_page_start(list, ctx->nscalars); > fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); > + crclist = (u32 *)(fdlist + FASTRPC_MAX_FDLIST); I think we should rewrite this parsing somehow. Is the format of data documented somewhere? > > for (i = inbufs; i < ctx->nbufs; ++i) { > if (!ctx->maps[i]) { > @@ -1102,6 +1107,12 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, > fastrpc_map_put(mmap); > } > > + if (ctx->crc && crclist && rpra) { > + if (copy_to_user((void __user *)ctx->crc, crclist, > + FASTRPC_MAX_CRCLIST * sizeof(u32))) Oh, so it's a user pointer. Then u32* was completely incorrect. Also you are copying FASTRPC_MAX_CRCLIST elements. Are all of them filled? Or are we leaking some data to userspace? > + return -EFAULT; > + } > + > return 0; > } > > @@ -1137,13 +1148,12 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, > > } > > -static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > - u32 handle, u32 sc, > - struct fastrpc_invoke_args *args) > +static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, struct fastrpc_invoke_v2 *inv2) Please don't touch what doesn't need to be touched. You are replacing handle/sc/args with inv2, not touching the first line. > { > struct fastrpc_invoke_ctx *ctx = NULL; > struct fastrpc_buf *buf, *b; > - > + struct fastrpc_invoke inv; > + u32 handle, sc; > int err = 0; > > if (!fl->sctx) > @@ -1152,12 +1162,15 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > if (!fl->cctx->rpdev) > return -EPIPE; > > + inv = inv2->inv; > + handle = inv.handle; > + sc = inv.sc; > if (handle == FASTRPC_INIT_HANDLE && !kernel) { > dev_warn_ratelimited(fl->sctx->dev, "user app trying to send a kernel RPC message (%d)\n", handle); > return -EPERM; > } > > - ctx = fastrpc_context_alloc(fl, kernel, sc, args); > + ctx = fastrpc_context_alloc(fl, kernel, sc, inv2); > if (IS_ERR(ctx)) > return PTR_ERR(ctx); > > @@ -1239,6 +1252,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, > { > struct fastrpc_init_create_static init; > struct fastrpc_invoke_args *args; > + struct fastrpc_invoke_v2 ioctl = {0}; Why do you need to init it? > struct fastrpc_phy_page pages[1]; > char *name; > int err; > @@ -1248,7 +1262,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, > u32 namelen; > u32 pageslen; > } inbuf; > - u32 sc; > > args = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL); > if (!args) > @@ -1313,10 +1326,10 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, > args[2].length = sizeof(*pages); > args[2].fd = -1; > > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0); > - > - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, > - sc, args); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0); > + ioctl.inv.args = (u64)args; Can you pass it as is, without typecasts? > + err = fastrpc_internal_invoke(fl, true, &ioctl); > if (err) > goto err_invoke; > > @@ -1357,6 +1370,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > { > struct fastrpc_init_create init; > struct fastrpc_invoke_args *args; > + struct fastrpc_invoke_v2 ioctl = {0}; > struct fastrpc_phy_page pages[1]; > struct fastrpc_map *map = NULL; > struct fastrpc_buf *imem = NULL; > @@ -1370,7 +1384,6 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > u32 attrs; > u32 siglen; > } inbuf; > - u32 sc; > bool unsigned_module = false; > > args = kcalloc(FASTRPC_CREATE_PROCESS_NARGS, sizeof(*args), GFP_KERNEL); > @@ -1444,12 +1457,12 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > args[5].length = sizeof(inbuf.siglen); > args[5].fd = -1; > > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE, 4, 0); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE, 4, 0); > if (init.attrs) > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_ATTR, 4, 0); > - > - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, > - sc, args); > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_ATTR, 4, 0); if (init.attrs) ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_ATTR, 4, 0); else ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE, 4, 0); > + ioctl.inv.args = (u64)args; > + err = fastrpc_internal_invoke(fl, true, &ioctl); > if (err) > goto err_invoke; > > @@ -1501,17 +1514,18 @@ static void fastrpc_session_free(struct fastrpc_channel_ctx *cctx, > static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) > { > struct fastrpc_invoke_args args[1]; > + struct fastrpc_invoke_v2 ioctl = {0}; > int tgid = 0; > - u32 sc; > > tgid = fl->tgid; > args[0].ptr = (u64)(uintptr_t) &tgid; > args[0].length = sizeof(tgid); > args[0].fd = -1; > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_RELEASE, 1, 0); > > - return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, > - sc, &args[0]); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_RELEASE, 1, 0); > + ioctl.inv.args = (u64)args; > + return fastrpc_internal_invoke(fl, true, &ioctl); > } > > static int fastrpc_device_release(struct inode *inode, struct file *file) > @@ -1647,45 +1661,77 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) > { > struct fastrpc_invoke_args args[1]; > + struct fastrpc_invoke_v2 ioctl = {0}; > int tgid = fl->tgid; > - u32 sc; > > args[0].ptr = (u64)(uintptr_t) &tgid; > args[0].length = sizeof(tgid); > args[0].fd = -1; > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0); > fl->pd = pd; > > - return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, > - sc, &args[0]); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0); > + ioctl.inv.args = (u64)args; > + return fastrpc_internal_invoke(fl, true, &ioctl); > } > > -static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp) > +static int fastrpc_copy_args(struct fastrpc_invoke *inv) > { > struct fastrpc_invoke_args *args = NULL; > - struct fastrpc_invoke inv; > u32 nscalars; > - int err; > - > - if (copy_from_user(&inv, argp, sizeof(inv))) > - return -EFAULT; > > /* nscalars is truncated here to max supported value */ > - nscalars = REMOTE_SCALARS_LENGTH(inv.sc); > + nscalars = REMOTE_SCALARS_LENGTH(inv->sc); > if (nscalars) { > args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL); > if (!args) > return -ENOMEM; > > - if (copy_from_user(args, (void __user *)(uintptr_t)inv.args, > + if (copy_from_user(args, (void __user *)(uintptr_t)inv->args, Wait... So inv->args is a user pointer? Then how can you assign a kernel-based pointer to the same field? I think you need to sanitize your structures. One is userspace-facing. It should be using userspace data pointers, etc. Another one is a kernel representation of the ioctl args. It might have a different structure, it shouldn't contain __user data, etc. > nscalars * sizeof(*args))) { > kfree(args); > return -EFAULT; > } > } > + inv->args = args; > > - err = fastrpc_internal_invoke(fl, false, inv.handle, inv.sc, args); > - kfree(args); > + return 0; > +} Looking at the rest of the code, I think the patch needs to be split. CRC is the minor issue at this point, please focus on getting existing data being handled correctly while refactoring the code to use new structure. I'd suggest seeing two struct definitions: one for the userspace and another one for the kernel space. > + > +static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp) > +{ > + struct fastrpc_invoke_v2 ioctl = {0}; > + struct fastrpc_invoke inv; > + int err; > + > + if (copy_from_user(&inv, argp, sizeof(inv))) > + return -EFAULT; > + > + err = fastrpc_copy_args(&inv); > + if (err) > + return err; > + > + ioctl.inv = inv; > + err = fastrpc_internal_invoke(fl, false, &ioctl); > + kfree(inv.args); > + > + return err; > +} > + > +static int fastrpc_invokev2(struct fastrpc_user *fl, char __user *argp) > +{ > + struct fastrpc_invoke_v2 inv2 = {0}; > + int err; > + > + if (copy_from_user(&inv2, argp, sizeof(inv2))) > + return -EFAULT; > + > + err = fastrpc_copy_args(&inv2.inv); > + if (err) > + return err; > + > + err = fastrpc_internal_invoke(fl, false, &inv2); > + kfree(inv2.inv.args); > > return err; > } > @@ -1694,6 +1740,7 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr > uint32_t dsp_attr_buf_len) > { > struct fastrpc_invoke_args args[2] = { 0 }; > + struct fastrpc_invoke_v2 ioctl = {0}; > > /* > * Capability filled in userspace. This carries the information > @@ -1710,8 +1757,10 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr > args[1].length = dsp_attr_buf_len * sizeof(u32); > args[1].fd = -1; > > - return fastrpc_internal_invoke(fl, true, FASTRPC_DSP_UTILITIES_HANDLE, > - FASTRPC_SCALARS(0, 1, 1), args); > + ioctl.inv.handle = FASTRPC_DSP_UTILITIES_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(0, 1, 1); > + ioctl.inv.args = (u64)args; > + return fastrpc_internal_invoke(fl, true, &ioctl); > } > > static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap, > @@ -1798,10 +1847,10 @@ static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *buf) > { > struct fastrpc_invoke_args args[1] = { [0] = { 0 } }; > + struct fastrpc_invoke_v2 ioctl = {0}; > struct fastrpc_munmap_req_msg req_msg; > struct device *dev = fl->sctx->dev; > int err; > - u32 sc; > > req_msg.pgid = fl->tgid; > req_msg.size = buf->size; > @@ -1810,9 +1859,10 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * > args[0].ptr = (u64) (uintptr_t) &req_msg; > args[0].length = sizeof(req_msg); > > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0); > - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, > - &args[0]); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0); > + ioctl.inv.args = (u64)args; > + err = fastrpc_internal_invoke(fl, true, &ioctl); > if (!err) { > dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr); > spin_lock(&fl->lock); > @@ -1856,6 +1906,7 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > { > struct fastrpc_invoke_args args[3] = { [0 ... 2] = { 0 } }; > + struct fastrpc_invoke_v2 ioctl = {0}; > struct fastrpc_buf *buf = NULL; > struct fastrpc_mmap_req_msg req_msg; > struct fastrpc_mmap_rsp_msg rsp_msg; > @@ -1863,7 +1914,6 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > struct fastrpc_req_mmap req; > struct device *dev = fl->sctx->dev; > int err; > - u32 sc; > > if (copy_from_user(&req, argp, sizeof(req))) > return -EFAULT; > @@ -1906,9 +1956,10 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > args[2].ptr = (u64) (uintptr_t) &rsp_msg; > args[2].length = sizeof(rsp_msg); > > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1); > - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, > - &args[0]); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1); > + ioctl.inv.args = (u64)args; > + err = fastrpc_internal_invoke(fl, true, &ioctl); > if (err) { > dev_err(dev, "mmap error (len 0x%08llx)\n", buf->size); > fastrpc_buf_free(buf); > @@ -1957,10 +2008,10 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_mem_unmap *req) > { > struct fastrpc_invoke_args args[1] = { [0] = { 0 } }; > + struct fastrpc_invoke_v2 ioctl = {0}; > struct fastrpc_map *map = NULL, *iter, *m; > struct fastrpc_mem_unmap_req_msg req_msg = { 0 }; > int err = 0; > - u32 sc; > struct device *dev = fl->sctx->dev; > > spin_lock(&fl->lock); > @@ -1986,9 +2037,10 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me > args[0].ptr = (u64) (uintptr_t) &req_msg; > args[0].length = sizeof(req_msg); > > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_UNMAP, 1, 0); > - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, > - &args[0]); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_UNMAP, 1, 0); > + ioctl.inv.args = (u64)args; > + err = fastrpc_internal_invoke(fl, true, &ioctl); > if (err) { > dev_err(dev, "unmmap\tpt fd = %d, 0x%09llx error\n", map->fd, map->raddr); > return err; > @@ -2011,6 +2063,7 @@ static int fastrpc_req_mem_unmap(struct fastrpc_user *fl, char __user *argp) > static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) > { > struct fastrpc_invoke_args args[4] = { [0 ... 3] = { 0 } }; > + struct fastrpc_invoke_v2 ioctl = {0}; > struct fastrpc_mem_map_req_msg req_msg = { 0 }; > struct fastrpc_mmap_rsp_msg rsp_msg = { 0 }; > struct fastrpc_mem_unmap req_unmap = { 0 }; > @@ -2019,7 +2072,6 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) > struct device *dev = fl->sctx->dev; > struct fastrpc_map *map = NULL; > int err; > - u32 sc; > > if (copy_from_user(&req, argp, sizeof(req))) > return -EFAULT; > @@ -2055,8 +2107,10 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) > args[3].ptr = (u64) (uintptr_t) &rsp_msg; > args[3].length = sizeof(rsp_msg); > > - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_MAP, 3, 1); > - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, &args[0]); > + ioctl.inv.handle = FASTRPC_INIT_HANDLE; > + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_MAP, 3, 1); > + ioctl.inv.args = (u64)args; > + err = fastrpc_internal_invoke(fl, true, &ioctl); > if (err) { > dev_err(dev, "mem mmap error, fd %d, vaddr %llx, size %lld\n", > req.fd, req.vaddrin, map->size); > @@ -2096,6 +2150,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd, > case FASTRPC_IOCTL_INVOKE: > err = fastrpc_invoke(fl, argp); > break; > + case FASTRPC_IOCTL_INVOKEV2: > + err = fastrpc_invokev2(fl, argp); > + break; > case FASTRPC_IOCTL_INIT_ATTACH: > err = fastrpc_init_attach(fl, ROOT_PD); > break; > diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h > index f33d914d8f46..406b80555d41 100644 > --- a/include/uapi/misc/fastrpc.h > +++ b/include/uapi/misc/fastrpc.h > @@ -17,6 +17,7 @@ > #define FASTRPC_IOCTL_MEM_MAP _IOWR('R', 10, struct fastrpc_mem_map) > #define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct fastrpc_mem_unmap) > #define FASTRPC_IOCTL_GET_DSP_INFO _IOWR('R', 13, struct fastrpc_ioctl_capability) > +#define FASTRPC_IOCTL_INVOKEV2 _IOWR('R', 14, struct fastrpc_invoke_v2) > > /** > * enum fastrpc_map_flags - control flags for mapping memory on DSP user process > @@ -80,6 +81,12 @@ struct fastrpc_invoke { > __u64 args; > }; > > +struct fastrpc_invoke_v2 { > + struct fastrpc_invoke inv; > + __u64 crc; > + __u32 reserved[16]; > +}; > + > struct fastrpc_init_create { > __u32 filelen; /* elf file length */ > __s32 filefd; /* fd for the file */ > -- > 2.34.1 >
Hi Ekansh, kernel test robot noticed the following build warnings: [auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc2 next-20241004] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Add-CRC-support-using-invokeV2-request/20241007-164734 base: char-misc/char-misc-testing patch link: https://lore.kernel.org/r/20241007084518.3649876-2-quic_ekangupt%40quicinc.com patch subject: [PATCH v1 1/4] misc: fastrpc: Add CRC support using invokeV2 request config: arc-randconfig-001-20241008 (https://download.01.org/0day-ci/archive/20241008/202410080906.2kCMkY5c-lkp@intel.com/config) compiler: arc-elf-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410080906.2kCMkY5c-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410080906.2kCMkY5c-lkp@intel.com/ All warnings (new ones prefixed by >>): drivers/misc/fastrpc.c: In function 'fastrpc_context_alloc': >> drivers/misc/fastrpc.c:584:16: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] 584 | args = (struct fastrpc_invoke_args *)inv2->inv.args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_init_create_static_process': >> drivers/misc/fastrpc.c:1331:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1331 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_init_create_process': drivers/misc/fastrpc.c:1464:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1464 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_release_current_dsp_process': drivers/misc/fastrpc.c:1527:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1527 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_init_attach': drivers/misc/fastrpc.c:1674:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1674 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_copy_args': >> drivers/misc/fastrpc.c:1696:19: warning: assignment to '__u64' {aka 'long long unsigned int'} from 'struct fastrpc_invoke_args *' makes integer from pointer without a cast [-Wint-conversion] 1696 | inv->args = args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_invoke': >> drivers/misc/fastrpc.c:1716:18: warning: passing argument 1 of 'kfree' makes pointer from integer without a cast [-Wint-conversion] 1716 | kfree(inv.args); | ~~~^~~~~ | | | __u64 {aka long long unsigned int} In file included from include/linux/mm.h:33, from include/linux/scatterlist.h:8, from include/linux/dma-buf.h:19, from drivers/misc/fastrpc.c:7: include/linux/slab.h:446:24: note: expected 'const void *' but argument is of type '__u64' {aka 'long long unsigned int'} 446 | void kfree(const void *objp); | ~~~~~~~~~~~~^~~~ drivers/misc/fastrpc.c: In function 'fastrpc_invokev2': drivers/misc/fastrpc.c:1734:23: warning: passing argument 1 of 'kfree' makes pointer from integer without a cast [-Wint-conversion] 1734 | kfree(inv2.inv.args); | ~~~~~~~~^~~~~ | | | __u64 {aka long long unsigned int} include/linux/slab.h:446:24: note: expected 'const void *' but argument is of type '__u64' {aka 'long long unsigned int'} 446 | void kfree(const void *objp); | ~~~~~~~~~~~~^~~~ drivers/misc/fastrpc.c: In function 'fastrpc_get_info_from_dsp': drivers/misc/fastrpc.c:1762:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1762 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_req_munmap_impl': drivers/misc/fastrpc.c:1864:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1864 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_req_mmap': drivers/misc/fastrpc.c:1961:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 1961 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_req_mem_unmap_impl': drivers/misc/fastrpc.c:2042:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 2042 | ioctl.inv.args = (u64)args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_req_mem_map': drivers/misc/fastrpc.c:2112:26: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] 2112 | ioctl.inv.args = (u64)args; | ^ vim +584 drivers/misc/fastrpc.c 573 574 static struct fastrpc_invoke_ctx *fastrpc_context_alloc( 575 struct fastrpc_user *user, u32 kernel, u32 sc, 576 struct fastrpc_invoke_v2 *inv2) 577 { 578 struct fastrpc_channel_ctx *cctx = user->cctx; 579 struct fastrpc_invoke_ctx *ctx = NULL; 580 struct fastrpc_invoke_args *args = NULL; 581 unsigned long flags; 582 int ret; 583 > 584 args = (struct fastrpc_invoke_args *)inv2->inv.args; 585 ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); 586 if (!ctx) 587 return ERR_PTR(-ENOMEM); 588 589 INIT_LIST_HEAD(&ctx->node); 590 ctx->fl = user; 591 ctx->nscalars = REMOTE_SCALARS_LENGTH(sc); 592 ctx->nbufs = REMOTE_SCALARS_INBUFS(sc) + 593 REMOTE_SCALARS_OUTBUFS(sc); 594 595 if (ctx->nscalars) { 596 ctx->maps = kcalloc(ctx->nscalars, 597 sizeof(*ctx->maps), GFP_KERNEL); 598 if (!ctx->maps) { 599 kfree(ctx); 600 return ERR_PTR(-ENOMEM); 601 } 602 ctx->olaps = kcalloc(ctx->nscalars, 603 sizeof(*ctx->olaps), GFP_KERNEL); 604 if (!ctx->olaps) { 605 kfree(ctx->maps); 606 kfree(ctx); 607 return ERR_PTR(-ENOMEM); 608 } 609 ctx->args = args; 610 fastrpc_get_buff_overlaps(ctx); 611 } 612 613 /* Released in fastrpc_context_put() */ 614 fastrpc_channel_ctx_get(cctx); 615 616 ctx->crc = (u32 *)(uintptr_t)inv2->crc; 617 ctx->sc = sc; 618 ctx->retval = -1; 619 ctx->pid = current->pid; 620 ctx->tgid = user->tgid; 621 ctx->cctx = cctx; 622 init_completion(&ctx->work); 623 INIT_WORK(&ctx->put_work, fastrpc_context_put_wq); 624 625 spin_lock(&user->lock); 626 list_add_tail(&ctx->node, &user->pending); 627 spin_unlock(&user->lock); 628 629 spin_lock_irqsave(&cctx->lock, flags); 630 ret = idr_alloc_cyclic(&cctx->ctx_idr, ctx, 1, 631 FASTRPC_CTX_MAX, GFP_ATOMIC); 632 if (ret < 0) { 633 spin_unlock_irqrestore(&cctx->lock, flags); 634 goto err_idr; 635 } 636 ctx->ctxid = ret << 4; 637 spin_unlock_irqrestore(&cctx->lock, flags); 638 639 kref_init(&ctx->refcount); 640 641 return ctx; 642 err_idr: 643 spin_lock(&user->lock); 644 list_del(&ctx->node); 645 spin_unlock(&user->lock); 646 fastrpc_channel_ctx_put(cctx); 647 kfree(ctx->maps); 648 kfree(ctx->olaps); 649 kfree(ctx); 650 651 return ERR_PTR(ret); 652 } 653
Hi Ekansh,
kernel test robot noticed the following build errors:
[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc2 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Add-CRC-support-using-invokeV2-request/20241007-164734
base: char-misc/char-misc-testing
patch link: https://lore.kernel.org/r/20241007084518.3649876-2-quic_ekangupt%40quicinc.com
patch subject: [PATCH v1 1/4] misc: fastrpc: Add CRC support using invokeV2 request
config: arm-randconfig-002-20241008 (https://download.01.org/0day-ci/archive/20241008/202410080940.S7Rar8Zi-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project fef3566a25ff0e34fb87339ba5e13eca17cec00f)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410080940.S7Rar8Zi-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410080940.S7Rar8Zi-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/misc/fastrpc.c:7:
In file included from include/linux/dma-buf.h:19:
In file included from include/linux/scatterlist.h:8:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/misc/fastrpc.c:1696:12: error: incompatible pointer to integer conversion assigning to '__u64' (aka 'unsigned long long') from 'struct fastrpc_invoke_args *' [-Wint-conversion]
1696 | inv->args = args;
| ^ ~~~~
>> drivers/misc/fastrpc.c:1716:8: error: incompatible integer to pointer conversion passing '__u64' (aka 'unsigned long long') to parameter of type 'const void *' [-Wint-conversion]
1716 | kfree(inv.args);
| ^~~~~~~~
include/linux/slab.h:446:24: note: passing argument to parameter 'objp' here
446 | void kfree(const void *objp);
| ^
drivers/misc/fastrpc.c:1734:8: error: incompatible integer to pointer conversion passing '__u64' (aka 'unsigned long long') to parameter of type 'const void *' [-Wint-conversion]
1734 | kfree(inv2.inv.args);
| ^~~~~~~~~~~~~
include/linux/slab.h:446:24: note: passing argument to parameter 'objp' here
446 | void kfree(const void *objp);
| ^
1 warning and 3 errors generated.
vim +1696 drivers/misc/fastrpc.c
1677
1678 static int fastrpc_copy_args(struct fastrpc_invoke *inv)
1679 {
1680 struct fastrpc_invoke_args *args = NULL;
1681 u32 nscalars;
1682
1683 /* nscalars is truncated here to max supported value */
1684 nscalars = REMOTE_SCALARS_LENGTH(inv->sc);
1685 if (nscalars) {
1686 args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL);
1687 if (!args)
1688 return -ENOMEM;
1689
1690 if (copy_from_user(args, (void __user *)(uintptr_t)inv->args,
1691 nscalars * sizeof(*args))) {
1692 kfree(args);
1693 return -EFAULT;
1694 }
1695 }
> 1696 inv->args = args;
1697
1698 return 0;
1699 }
1700
1701 static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp)
1702 {
1703 struct fastrpc_invoke_v2 ioctl = {0};
1704 struct fastrpc_invoke inv;
1705 int err;
1706
1707 if (copy_from_user(&inv, argp, sizeof(inv)))
1708 return -EFAULT;
1709
1710 err = fastrpc_copy_args(&inv);
1711 if (err)
1712 return err;
1713
1714 ioctl.inv = inv;
1715 err = fastrpc_internal_invoke(fl, false, &ioctl);
> 1716 kfree(inv.args);
1717
1718 return err;
1719 }
1720
Hi Ekansh, kernel test robot noticed the following build errors: [auto build test ERROR on char-misc/char-misc-testing] [also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc2 next-20241004] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Add-CRC-support-using-invokeV2-request/20241007-164734 base: char-misc/char-misc-testing patch link: https://lore.kernel.org/r/20241007084518.3649876-2-quic_ekangupt%40quicinc.com patch subject: [PATCH v1 1/4] misc: fastrpc: Add CRC support using invokeV2 request config: loongarch-allmodconfig (https://download.01.org/0day-ci/archive/20241008/202410081016.Nuyub4XK-lkp@intel.com/config) compiler: loongarch64-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241008/202410081016.Nuyub4XK-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410081016.Nuyub4XK-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/misc/fastrpc.c: In function 'fastrpc_copy_args': >> drivers/misc/fastrpc.c:1696:19: error: assignment to '__u64' {aka 'long long unsigned int'} from 'struct fastrpc_invoke_args *' makes integer from pointer without a cast [-Wint-conversion] 1696 | inv->args = args; | ^ drivers/misc/fastrpc.c: In function 'fastrpc_invoke': >> drivers/misc/fastrpc.c:1716:18: error: passing argument 1 of 'kfree' makes pointer from integer without a cast [-Wint-conversion] 1716 | kfree(inv.args); | ~~~^~~~~ | | | __u64 {aka long long unsigned int} In file included from include/linux/fs.h:45, from arch/loongarch/include/asm/elf.h:9, from include/linux/elf.h:6, from include/linux/module.h:19, from include/linux/device/driver.h:21, from include/linux/device.h:32, from drivers/misc/fastrpc.c:6: include/linux/slab.h:446:24: note: expected 'const void *' but argument is of type '__u64' {aka 'long long unsigned int'} 446 | void kfree(const void *objp); | ~~~~~~~~~~~~^~~~ drivers/misc/fastrpc.c: In function 'fastrpc_invokev2': drivers/misc/fastrpc.c:1734:23: error: passing argument 1 of 'kfree' makes pointer from integer without a cast [-Wint-conversion] 1734 | kfree(inv2.inv.args); | ~~~~~~~~^~~~~ | | | __u64 {aka long long unsigned int} include/linux/slab.h:446:24: note: expected 'const void *' but argument is of type '__u64' {aka 'long long unsigned int'} 446 | void kfree(const void *objp); | ~~~~~~~~~~~~^~~~ vim +1696 drivers/misc/fastrpc.c 1677 1678 static int fastrpc_copy_args(struct fastrpc_invoke *inv) 1679 { 1680 struct fastrpc_invoke_args *args = NULL; 1681 u32 nscalars; 1682 1683 /* nscalars is truncated here to max supported value */ 1684 nscalars = REMOTE_SCALARS_LENGTH(inv->sc); 1685 if (nscalars) { 1686 args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL); 1687 if (!args) 1688 return -ENOMEM; 1689 1690 if (copy_from_user(args, (void __user *)(uintptr_t)inv->args, 1691 nscalars * sizeof(*args))) { 1692 kfree(args); 1693 return -EFAULT; 1694 } 1695 } > 1696 inv->args = args; 1697 1698 return 0; 1699 } 1700 1701 static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp) 1702 { 1703 struct fastrpc_invoke_v2 ioctl = {0}; 1704 struct fastrpc_invoke inv; 1705 int err; 1706 1707 if (copy_from_user(&inv, argp, sizeof(inv))) 1708 return -EFAULT; 1709 1710 err = fastrpc_copy_args(&inv); 1711 if (err) 1712 return err; 1713 1714 ioctl.inv = inv; 1715 err = fastrpc_internal_invoke(fl, false, &ioctl); > 1716 kfree(inv.args); 1717 1718 return err; 1719 } 1720
Hi Ekansh, kernel test robot noticed the following build warnings: [auto build test WARNING on char-misc/char-misc-testing] [also build test WARNING on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.12-rc2 next-20241008] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Add-CRC-support-using-invokeV2-request/20241007-164734 base: char-misc/char-misc-testing patch link: https://lore.kernel.org/r/20241007084518.3649876-2-quic_ekangupt%40quicinc.com patch subject: [PATCH v1 1/4] misc: fastrpc: Add CRC support using invokeV2 request config: alpha-randconfig-r112-20241008 (https://download.01.org/0day-ci/archive/20241009/202410090402.lospEFvZ-lkp@intel.com/config) compiler: alpha-linux-gcc (GCC) 13.3.0 reproduce: (https://download.01.org/0day-ci/archive/20241009/202410090402.lospEFvZ-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410090402.lospEFvZ-lkp@intel.com/ sparse warnings: (new ones prefixed by >>) >> drivers/misc/fastrpc.c:1696:19: sparse: sparse: incorrect type in assignment (different base types) @@ expected unsigned long long [usertype] args @@ got struct fastrpc_invoke_args *[assigned] args @@ drivers/misc/fastrpc.c:1696:19: sparse: expected unsigned long long [usertype] args drivers/misc/fastrpc.c:1696:19: sparse: got struct fastrpc_invoke_args *[assigned] args >> drivers/misc/fastrpc.c:1716:18: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected void const *objp @@ got unsigned long long [addressable] [usertype] args @@ drivers/misc/fastrpc.c:1716:18: sparse: expected void const *objp drivers/misc/fastrpc.c:1716:18: sparse: got unsigned long long [addressable] [usertype] args drivers/misc/fastrpc.c:1734:23: sparse: sparse: incorrect type in argument 1 (different base types) @@ expected void const *objp @@ got unsigned long long [addressable] [usertype] args @@ drivers/misc/fastrpc.c:1734:23: sparse: expected void const *objp drivers/misc/fastrpc.c:1734:23: sparse: got unsigned long long [addressable] [usertype] args drivers/misc/fastrpc.c: note: in included file (through include/linux/swait.h, include/linux/completion.h): include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true vim +1696 drivers/misc/fastrpc.c 1677 1678 static int fastrpc_copy_args(struct fastrpc_invoke *inv) 1679 { 1680 struct fastrpc_invoke_args *args = NULL; 1681 u32 nscalars; 1682 1683 /* nscalars is truncated here to max supported value */ 1684 nscalars = REMOTE_SCALARS_LENGTH(inv->sc); 1685 if (nscalars) { 1686 args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL); 1687 if (!args) 1688 return -ENOMEM; 1689 1690 if (copy_from_user(args, (void __user *)(uintptr_t)inv->args, 1691 nscalars * sizeof(*args))) { 1692 kfree(args); 1693 return -EFAULT; 1694 } 1695 } > 1696 inv->args = args; 1697 1698 return 0; 1699 } 1700 1701 static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp) 1702 { 1703 struct fastrpc_invoke_v2 ioctl = {0}; 1704 struct fastrpc_invoke inv; 1705 int err; 1706 1707 if (copy_from_user(&inv, argp, sizeof(inv))) 1708 return -EFAULT; 1709 1710 err = fastrpc_copy_args(&inv); 1711 if (err) 1712 return err; 1713 1714 ioctl.inv = inv; 1715 err = fastrpc_internal_invoke(fl, false, &ioctl); > 1716 kfree(inv.args); 1717 1718 return err; 1719 } 1720
On 10/7/2024 2:23 PM, Greg KH wrote: > On Mon, Oct 07, 2024 at 02:15:15PM +0530, Ekansh Gupta wrote: >> +struct fastrpc_invoke_v2 { >> + struct fastrpc_invoke inv; >> + __u64 crc; >> + __u32 reserved[16]; > I'm sure we have been over this before, but if you have a reserved > field, you HAVE to check that it is zero if you ever want to use it in > the future. As you did not check it, just remove it from this structure > as obviously it can never be used for anything. > > Your internal review should have caught this, if not, please go fix it > AND get some internal review and signed-off-by before submitting this > series again. Thanks for your insights. I'll fix this in the next patch. My apologies for missing it. --ekansh > > thanks, > > greg k-h
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 74181b8c386b..8e817a763d1d 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -573,13 +573,15 @@ static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx) static struct fastrpc_invoke_ctx *fastrpc_context_alloc( struct fastrpc_user *user, u32 kernel, u32 sc, - struct fastrpc_invoke_args *args) + struct fastrpc_invoke_v2 *inv2) { struct fastrpc_channel_ctx *cctx = user->cctx; struct fastrpc_invoke_ctx *ctx = NULL; + struct fastrpc_invoke_args *args = NULL; unsigned long flags; int ret; + args = (struct fastrpc_invoke_args *)inv2->inv.args; ctx = kzalloc(sizeof(*ctx), GFP_KERNEL); if (!ctx) return ERR_PTR(-ENOMEM); @@ -611,6 +613,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc( /* Released in fastrpc_context_put() */ fastrpc_channel_ctx_get(cctx); + ctx->crc = (u32 *)(uintptr_t)inv2->crc; ctx->sc = sc; ctx->retval = -1; ctx->pid = current->pid; @@ -1070,6 +1073,7 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, struct fastrpc_invoke_buf *list; struct fastrpc_phy_page *pages; u64 *fdlist; + u32 *crclist; int i, inbufs, outbufs, handles; inbufs = REMOTE_SCALARS_INBUFS(ctx->sc); @@ -1078,6 +1082,7 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, list = fastrpc_invoke_buf_start(rpra, ctx->nscalars); pages = fastrpc_phy_page_start(list, ctx->nscalars); fdlist = (uint64_t *)(pages + inbufs + outbufs + handles); + crclist = (u32 *)(fdlist + FASTRPC_MAX_FDLIST); for (i = inbufs; i < ctx->nbufs; ++i) { if (!ctx->maps[i]) { @@ -1102,6 +1107,12 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx, fastrpc_map_put(mmap); } + if (ctx->crc && crclist && rpra) { + if (copy_to_user((void __user *)ctx->crc, crclist, + FASTRPC_MAX_CRCLIST * sizeof(u32))) + return -EFAULT; + } + return 0; } @@ -1137,13 +1148,12 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx, } -static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, - u32 handle, u32 sc, - struct fastrpc_invoke_args *args) +static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, struct fastrpc_invoke_v2 *inv2) { struct fastrpc_invoke_ctx *ctx = NULL; struct fastrpc_buf *buf, *b; - + struct fastrpc_invoke inv; + u32 handle, sc; int err = 0; if (!fl->sctx) @@ -1152,12 +1162,15 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, if (!fl->cctx->rpdev) return -EPIPE; + inv = inv2->inv; + handle = inv.handle; + sc = inv.sc; if (handle == FASTRPC_INIT_HANDLE && !kernel) { dev_warn_ratelimited(fl->sctx->dev, "user app trying to send a kernel RPC message (%d)\n", handle); return -EPERM; } - ctx = fastrpc_context_alloc(fl, kernel, sc, args); + ctx = fastrpc_context_alloc(fl, kernel, sc, inv2); if (IS_ERR(ctx)) return PTR_ERR(ctx); @@ -1239,6 +1252,7 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, { struct fastrpc_init_create_static init; struct fastrpc_invoke_args *args; + struct fastrpc_invoke_v2 ioctl = {0}; struct fastrpc_phy_page pages[1]; char *name; int err; @@ -1248,7 +1262,6 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, u32 namelen; u32 pageslen; } inbuf; - u32 sc; args = kcalloc(FASTRPC_CREATE_STATIC_PROCESS_NARGS, sizeof(*args), GFP_KERNEL); if (!args) @@ -1313,10 +1326,10 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl, args[2].length = sizeof(*pages); args[2].fd = -1; - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0); - - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, - sc, args); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0); + ioctl.inv.args = (u64)args; + err = fastrpc_internal_invoke(fl, true, &ioctl); if (err) goto err_invoke; @@ -1357,6 +1370,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, { struct fastrpc_init_create init; struct fastrpc_invoke_args *args; + struct fastrpc_invoke_v2 ioctl = {0}; struct fastrpc_phy_page pages[1]; struct fastrpc_map *map = NULL; struct fastrpc_buf *imem = NULL; @@ -1370,7 +1384,6 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, u32 attrs; u32 siglen; } inbuf; - u32 sc; bool unsigned_module = false; args = kcalloc(FASTRPC_CREATE_PROCESS_NARGS, sizeof(*args), GFP_KERNEL); @@ -1444,12 +1457,12 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, args[5].length = sizeof(inbuf.siglen); args[5].fd = -1; - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE, 4, 0); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE, 4, 0); if (init.attrs) - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_ATTR, 4, 0); - - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, - sc, args); + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_ATTR, 4, 0); + ioctl.inv.args = (u64)args; + err = fastrpc_internal_invoke(fl, true, &ioctl); if (err) goto err_invoke; @@ -1501,17 +1514,18 @@ static void fastrpc_session_free(struct fastrpc_channel_ctx *cctx, static int fastrpc_release_current_dsp_process(struct fastrpc_user *fl) { struct fastrpc_invoke_args args[1]; + struct fastrpc_invoke_v2 ioctl = {0}; int tgid = 0; - u32 sc; tgid = fl->tgid; args[0].ptr = (u64)(uintptr_t) &tgid; args[0].length = sizeof(tgid); args[0].fd = -1; - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_RELEASE, 1, 0); - return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, - sc, &args[0]); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_RELEASE, 1, 0); + ioctl.inv.args = (u64)args; + return fastrpc_internal_invoke(fl, true, &ioctl); } static int fastrpc_device_release(struct inode *inode, struct file *file) @@ -1647,45 +1661,77 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl, char __user *argp) static int fastrpc_init_attach(struct fastrpc_user *fl, int pd) { struct fastrpc_invoke_args args[1]; + struct fastrpc_invoke_v2 ioctl = {0}; int tgid = fl->tgid; - u32 sc; args[0].ptr = (u64)(uintptr_t) &tgid; args[0].length = sizeof(tgid); args[0].fd = -1; - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0); fl->pd = pd; - return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, - sc, &args[0]); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0); + ioctl.inv.args = (u64)args; + return fastrpc_internal_invoke(fl, true, &ioctl); } -static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp) +static int fastrpc_copy_args(struct fastrpc_invoke *inv) { struct fastrpc_invoke_args *args = NULL; - struct fastrpc_invoke inv; u32 nscalars; - int err; - - if (copy_from_user(&inv, argp, sizeof(inv))) - return -EFAULT; /* nscalars is truncated here to max supported value */ - nscalars = REMOTE_SCALARS_LENGTH(inv.sc); + nscalars = REMOTE_SCALARS_LENGTH(inv->sc); if (nscalars) { args = kcalloc(nscalars, sizeof(*args), GFP_KERNEL); if (!args) return -ENOMEM; - if (copy_from_user(args, (void __user *)(uintptr_t)inv.args, + if (copy_from_user(args, (void __user *)(uintptr_t)inv->args, nscalars * sizeof(*args))) { kfree(args); return -EFAULT; } } + inv->args = args; - err = fastrpc_internal_invoke(fl, false, inv.handle, inv.sc, args); - kfree(args); + return 0; +} + +static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp) +{ + struct fastrpc_invoke_v2 ioctl = {0}; + struct fastrpc_invoke inv; + int err; + + if (copy_from_user(&inv, argp, sizeof(inv))) + return -EFAULT; + + err = fastrpc_copy_args(&inv); + if (err) + return err; + + ioctl.inv = inv; + err = fastrpc_internal_invoke(fl, false, &ioctl); + kfree(inv.args); + + return err; +} + +static int fastrpc_invokev2(struct fastrpc_user *fl, char __user *argp) +{ + struct fastrpc_invoke_v2 inv2 = {0}; + int err; + + if (copy_from_user(&inv2, argp, sizeof(inv2))) + return -EFAULT; + + err = fastrpc_copy_args(&inv2.inv); + if (err) + return err; + + err = fastrpc_internal_invoke(fl, false, &inv2); + kfree(inv2.inv.args); return err; } @@ -1694,6 +1740,7 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr uint32_t dsp_attr_buf_len) { struct fastrpc_invoke_args args[2] = { 0 }; + struct fastrpc_invoke_v2 ioctl = {0}; /* * Capability filled in userspace. This carries the information @@ -1710,8 +1757,10 @@ static int fastrpc_get_info_from_dsp(struct fastrpc_user *fl, uint32_t *dsp_attr args[1].length = dsp_attr_buf_len * sizeof(u32); args[1].fd = -1; - return fastrpc_internal_invoke(fl, true, FASTRPC_DSP_UTILITIES_HANDLE, - FASTRPC_SCALARS(0, 1, 1), args); + ioctl.inv.handle = FASTRPC_DSP_UTILITIES_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(0, 1, 1); + ioctl.inv.args = (u64)args; + return fastrpc_internal_invoke(fl, true, &ioctl); } static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap, @@ -1798,10 +1847,10 @@ static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp) static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *buf) { struct fastrpc_invoke_args args[1] = { [0] = { 0 } }; + struct fastrpc_invoke_v2 ioctl = {0}; struct fastrpc_munmap_req_msg req_msg; struct device *dev = fl->sctx->dev; int err; - u32 sc; req_msg.pgid = fl->tgid; req_msg.size = buf->size; @@ -1810,9 +1859,10 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf * args[0].ptr = (u64) (uintptr_t) &req_msg; args[0].length = sizeof(req_msg); - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0); - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, - &args[0]); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0); + ioctl.inv.args = (u64)args; + err = fastrpc_internal_invoke(fl, true, &ioctl); if (!err) { dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr); spin_lock(&fl->lock); @@ -1856,6 +1906,7 @@ static int fastrpc_req_munmap(struct fastrpc_user *fl, char __user *argp) static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) { struct fastrpc_invoke_args args[3] = { [0 ... 2] = { 0 } }; + struct fastrpc_invoke_v2 ioctl = {0}; struct fastrpc_buf *buf = NULL; struct fastrpc_mmap_req_msg req_msg; struct fastrpc_mmap_rsp_msg rsp_msg; @@ -1863,7 +1914,6 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) struct fastrpc_req_mmap req; struct device *dev = fl->sctx->dev; int err; - u32 sc; if (copy_from_user(&req, argp, sizeof(req))) return -EFAULT; @@ -1906,9 +1956,10 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) args[2].ptr = (u64) (uintptr_t) &rsp_msg; args[2].length = sizeof(rsp_msg); - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1); - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, - &args[0]); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1); + ioctl.inv.args = (u64)args; + err = fastrpc_internal_invoke(fl, true, &ioctl); if (err) { dev_err(dev, "mmap error (len 0x%08llx)\n", buf->size); fastrpc_buf_free(buf); @@ -1957,10 +2008,10 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp) static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_mem_unmap *req) { struct fastrpc_invoke_args args[1] = { [0] = { 0 } }; + struct fastrpc_invoke_v2 ioctl = {0}; struct fastrpc_map *map = NULL, *iter, *m; struct fastrpc_mem_unmap_req_msg req_msg = { 0 }; int err = 0; - u32 sc; struct device *dev = fl->sctx->dev; spin_lock(&fl->lock); @@ -1986,9 +2037,10 @@ static int fastrpc_req_mem_unmap_impl(struct fastrpc_user *fl, struct fastrpc_me args[0].ptr = (u64) (uintptr_t) &req_msg; args[0].length = sizeof(req_msg); - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_UNMAP, 1, 0); - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, - &args[0]); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_UNMAP, 1, 0); + ioctl.inv.args = (u64)args; + err = fastrpc_internal_invoke(fl, true, &ioctl); if (err) { dev_err(dev, "unmmap\tpt fd = %d, 0x%09llx error\n", map->fd, map->raddr); return err; @@ -2011,6 +2063,7 @@ static int fastrpc_req_mem_unmap(struct fastrpc_user *fl, char __user *argp) static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) { struct fastrpc_invoke_args args[4] = { [0 ... 3] = { 0 } }; + struct fastrpc_invoke_v2 ioctl = {0}; struct fastrpc_mem_map_req_msg req_msg = { 0 }; struct fastrpc_mmap_rsp_msg rsp_msg = { 0 }; struct fastrpc_mem_unmap req_unmap = { 0 }; @@ -2019,7 +2072,6 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) struct device *dev = fl->sctx->dev; struct fastrpc_map *map = NULL; int err; - u32 sc; if (copy_from_user(&req, argp, sizeof(req))) return -EFAULT; @@ -2055,8 +2107,10 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp) args[3].ptr = (u64) (uintptr_t) &rsp_msg; args[3].length = sizeof(rsp_msg); - sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_MAP, 3, 1); - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, &args[0]); + ioctl.inv.handle = FASTRPC_INIT_HANDLE; + ioctl.inv.sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_MAP, 3, 1); + ioctl.inv.args = (u64)args; + err = fastrpc_internal_invoke(fl, true, &ioctl); if (err) { dev_err(dev, "mem mmap error, fd %d, vaddr %llx, size %lld\n", req.fd, req.vaddrin, map->size); @@ -2096,6 +2150,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd, case FASTRPC_IOCTL_INVOKE: err = fastrpc_invoke(fl, argp); break; + case FASTRPC_IOCTL_INVOKEV2: + err = fastrpc_invokev2(fl, argp); + break; case FASTRPC_IOCTL_INIT_ATTACH: err = fastrpc_init_attach(fl, ROOT_PD); break; diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h index f33d914d8f46..406b80555d41 100644 --- a/include/uapi/misc/fastrpc.h +++ b/include/uapi/misc/fastrpc.h @@ -17,6 +17,7 @@ #define FASTRPC_IOCTL_MEM_MAP _IOWR('R', 10, struct fastrpc_mem_map) #define FASTRPC_IOCTL_MEM_UNMAP _IOWR('R', 11, struct fastrpc_mem_unmap) #define FASTRPC_IOCTL_GET_DSP_INFO _IOWR('R', 13, struct fastrpc_ioctl_capability) +#define FASTRPC_IOCTL_INVOKEV2 _IOWR('R', 14, struct fastrpc_invoke_v2) /** * enum fastrpc_map_flags - control flags for mapping memory on DSP user process @@ -80,6 +81,12 @@ struct fastrpc_invoke { __u64 args; }; +struct fastrpc_invoke_v2 { + struct fastrpc_invoke inv; + __u64 crc; + __u32 reserved[16]; +}; + struct fastrpc_init_create { __u32 filelen; /* elf file length */ __s32 filefd; /* fd for the file */
InvokeV2 request is intended to support multiple enhanced invoke requests like CRC check, performance counter enablement and polling mode for RPC invocations. CRC check is getting enabled as part of this patch. CRC check for input and output argument helps in ensuring data consistency over a remote call. If user intends to enable CRC check, first local user CRC is calculated at user end and a CRC buffer is passed to DSP to capture remote CRC values. DSP is expected to write to the remote CRC buffer which is then compared at user level with the local CRC values. Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com> --- drivers/misc/fastrpc.c | 161 ++++++++++++++++++++++++------------ include/uapi/misc/fastrpc.h | 7 ++ 2 files changed, 116 insertions(+), 52 deletions(-)