| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <fec1e345-1ebb-467c-93d1-264ad0279096@quicinc.com>
Date: Wed, 29 Jan 2025 10:27:45 +0530
From: Ekansh Gupta <quic_ekangupt@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <srinivas.kandagatla@...aro.org>, <linux-arm-msm@...r.kernel.org>,
<gregkh@...uxfoundation.org>, <quic_bkumar@...cinc.com>,
<linux-kernel@...r.kernel.org>, <quic_chennak@...cinc.com>,
<dri-devel@...ts.freedesktop.org>, <arnd@...db.de>
Subject: Re: [PATCH v2 2/5] misc: fastrpc: Introduce context params structure
On 1/27/2025 9:13 PM, Dmitry Baryshkov wrote:
> On Mon, Jan 27, 2025 at 10:12:36AM +0530, Ekansh Gupta wrote:
>> Add structure to invoke context parameterms. This structure is meant
> Nit: for invoke context parameters.
Ack.
>
>> to carry invoke context specific data. This structure is passed to
>> internal invocation call to save the data in context. Examples of
>> data intended to part of this structure are: CRC user memory address,
>> poll timeout for invoke call, call priority etc.
>>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.com>
>> ---
>> drivers/misc/fastrpc.c | 138 ++++++++++++++++++++++++++++++++++-------
>> 1 file changed, 117 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 1a936d462519..c29d5536195e 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -254,6 +254,11 @@ struct fastrpc_invoke_ctx {
>> struct fastrpc_channel_ctx *cctx;
>> };
>>
>> +struct fastrpc_ctx_args {
>> + struct fastrpc_invoke_args *args;
>> + void __user *crc;
>> +};
> Why do we need a separate struct? Can we use fastrpc_invoke_ctx instead?
As fastrpc_invoke_ctx structure is allocated and returned from fastrpc_context_alloc
and getting fastrpc_context_free. I was thinking of keeping the user passed
information in a different structure.
>
>> +
>> struct fastrpc_session_ctx {
>> struct device *dev;
>> int sid;
>> @@ -574,7 +579,7 @@ 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_ctx_args *cargs)
>> {
>> struct fastrpc_channel_ctx *cctx = user->cctx;
>> struct fastrpc_invoke_ctx *ctx = NULL;
>> @@ -605,7 +610,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
>> kfree(ctx);
>> return ERR_PTR(-ENOMEM);
>> }
>> - ctx->args = args;
>> + ctx->args = cargs->args;
>> fastrpc_get_buff_overlaps(ctx);
>> }
>>
>> @@ -1133,7 +1138,7 @@ 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)
>> + struct fastrpc_ctx_args *cargs)
>> {
>> struct fastrpc_invoke_ctx *ctx = NULL;
>> struct fastrpc_buf *buf, *b;
>> @@ -1151,7 +1156,7 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel,
>> return -EPERM;
>> }
>>
>> - ctx = fastrpc_context_alloc(fl, kernel, sc, args);
>> + ctx = fastrpc_context_alloc(fl, kernel, sc, cargs);
>> if (IS_ERR(ctx))
>> return PTR_ERR(ctx);
>>
>> @@ -1233,6 +1238,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_ctx_args *cargs;
>> struct fastrpc_phy_page pages[1];
>> char *name;
>> int err;
>> @@ -1307,15 +1313,25 @@ static int fastrpc_init_create_static_process(struct fastrpc_user *fl,
>> args[2].length = sizeof(*pages);
>> args[2].fd = -1;
>>
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs) {
>> + err = -ENOMEM;
>> + goto err_invoke;
>> + }
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_CREATE_STATIC, 3, 0);
>>
>> err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
>> - sc, args);
>> - if (err)
>> + sc, cargs);
>> + if (err) {
>> + kfree(cargs);
> No, this should be a part of the error path.
Ack.
>
>> goto err_invoke;
>> + }
>>
>> kfree(args);
>> kfree(name);
>> + kfree(cargs);
>>
>> return 0;
>> err_invoke:
>> @@ -1351,6 +1367,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>> {
>> struct fastrpc_init_create init;
>> struct fastrpc_invoke_args *args;
>> + struct fastrpc_ctx_args *cargs;
>> struct fastrpc_phy_page pages[1];
>> struct fastrpc_map *map = NULL;
>> struct fastrpc_buf *imem = NULL;
>> @@ -1438,16 +1455,26 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>> args[5].length = sizeof(inbuf.siglen);
>> args[5].fd = -1;
>>
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs) {
>> + err = -ENOMEM;
>> + goto err_invoke;
>> + }
>> +
>> + cargs->args = args;
>> 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);
>> - if (err)
>> + sc, cargs);
>> + if (err) {
>> + kfree(cargs);
> Likewise.
Ack.
--ekansh
>
>> goto err_invoke;
>> + }
>>
>> kfree(args);
>> + kfree(cargs);
>>
>> return 0;
>>
>> @@ -1498,17 +1525,27 @@ 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];
>> - int client_id = 0;
>> + struct fastrpc_ctx_args *cargs;
>> + int client_id = 0, err;
>> u32 sc;
>>
>> client_id = fl->client_id;
>> args[0].ptr = (u64)(uintptr_t) &client_id;
>> args[0].length = sizeof(client_id);
>> args[0].fd = -1;
>> +
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_RELEASE, 1, 0);
>>
>> - return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
>> - sc, &args[0]);
>> + err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
>> + sc, cargs);
>> + kfree(cargs);
>> +
>> + return err;
>> }
>>
>> static int fastrpc_device_release(struct inode *inode, struct file *file)
>> @@ -1643,22 +1680,33 @@ 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];
>> - int client_id = fl->client_id;
>> + struct fastrpc_ctx_args *cargs;
>> + int client_id = fl->client_id, err;
>> u32 sc;
>>
>> args[0].ptr = (u64)(uintptr_t) &client_id;
>> args[0].length = sizeof(client_id);
>> args[0].fd = -1;
>> +
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0);
>> fl->pd = pd;
>>
>> - return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
>> - sc, &args[0]);
>> + err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
>> + sc, cargs);
>> + kfree(cargs);
>> +
>> + return err;
>> }
>>
>> static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp)
>> {
>> struct fastrpc_invoke_args *args = NULL;
>> + struct fastrpc_ctx_args *cargs;
>> struct fastrpc_invoke inv;
>> u32 nscalars;
>> int err;
>> @@ -1679,9 +1727,16 @@ static int fastrpc_invoke(struct fastrpc_user *fl, char __user *argp)
>> return -EFAULT;
>> }
>> }
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs) {
>> + kfree(args);
>> + return -ENOMEM;
>> + }
>>
>> - err = fastrpc_internal_invoke(fl, false, inv.handle, inv.sc, args);
>> + cargs->args = args;
>> + err = fastrpc_internal_invoke(fl, false, inv.handle, inv.sc, cargs);
>> kfree(args);
>> + kfree(cargs);
>>
>> return err;
>> }
>> @@ -1690,6 +1745,8 @@ 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_ctx_args *cargs;
>> + int err;
>>
>> /*
>> * Capability filled in userspace. This carries the information
>> @@ -1706,8 +1763,15 @@ 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);
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> + err = fastrpc_internal_invoke(fl, true, FASTRPC_DSP_UTILITIES_HANDLE,
>> + FASTRPC_SCALARS(0, 1, 1), cargs);
>> + kfree(cargs);
>> + return err;
>> }
>>
>> static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
>> @@ -1794,6 +1858,7 @@ 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_ctx_args *cargs;
>> struct fastrpc_munmap_req_msg req_msg;
>> struct device *dev = fl->sctx->dev;
>> int err;
>> @@ -1806,9 +1871,14 @@ 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);
>>
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MUNMAP, 1, 0);
>> err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
>> - &args[0]);
>> + cargs);
>> if (!err) {
>> dev_dbg(dev, "unmmap\tpt 0x%09lx OK\n", buf->raddr);
>> spin_lock(&fl->lock);
>> @@ -1818,6 +1888,7 @@ static int fastrpc_req_munmap_impl(struct fastrpc_user *fl, struct fastrpc_buf *
>> } else {
>> dev_err(dev, "unmmap\tpt 0x%09lx ERROR\n", buf->raddr);
>> }
>> + kfree(cargs);
>>
>> return err;
>> }
>> @@ -1852,6 +1923,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_ctx_args *cargs;
>> struct fastrpc_buf *buf = NULL;
>> struct fastrpc_mmap_req_msg req_msg;
>> struct fastrpc_mmap_rsp_msg rsp_msg;
>> @@ -1902,12 +1974,18 @@ 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);
>>
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MMAP, 2, 1);
>> err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
>> - &args[0]);
>> + cargs);
>> if (err) {
>> dev_err(dev, "mmap error (len 0x%08llx)\n", buf->size);
>> fastrpc_buf_free(buf);
>> + kfree(cargs);
>> return err;
>> }
>>
>> @@ -1942,17 +2020,20 @@ static int fastrpc_req_mmap(struct fastrpc_user *fl, char __user *argp)
>> dev_dbg(dev, "mmap\t\tpt 0x%09lx OK [len 0x%08llx]\n",
>> buf->raddr, buf->size);
>>
>> + kfree(cargs);
>> return 0;
>>
>> err_assign:
>> fastrpc_req_munmap_impl(fl, buf);
>>
>> + kfree(cargs);
>> return err;
>> }
>>
>> 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_ctx_args *cargs;
>> struct fastrpc_map *map = NULL, *iter, *m;
>> struct fastrpc_mem_unmap_req_msg req_msg = { 0 };
>> int err = 0;
>> @@ -1982,14 +2063,21 @@ 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);
>>
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_UNMAP, 1, 0);
>> err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc,
>> - &args[0]);
>> + cargs);
>> if (err) {
>> dev_err(dev, "unmmap\tpt fd = %d, 0x%09llx error\n", map->fd, map->raddr);
>> + kfree(cargs);
>> return err;
>> }
>> fastrpc_map_put(map);
>> + kfree(cargs);
>>
>> return 0;
>> }
>> @@ -2007,6 +2095,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_ctx_args *cargs;
>> struct fastrpc_mem_map_req_msg req_msg = { 0 };
>> struct fastrpc_mmap_rsp_msg rsp_msg = { 0 };
>> struct fastrpc_mem_unmap req_unmap = { 0 };
>> @@ -2051,8 +2140,13 @@ 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);
>>
>> + cargs = kzalloc(sizeof(*cargs), GFP_KERNEL);
>> + if (!cargs)
>> + return -ENOMEM;
>> +
>> + cargs->args = args;
>> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_MEM_MAP, 3, 1);
>> - err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, &args[0]);
>> + err = fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE, sc, cargs);
>> if (err) {
>> dev_err(dev, "mem mmap error, fd %d, vaddr %llx, size %lld\n",
>> req.fd, req.vaddrin, map->size);
>> @@ -2072,11 +2166,13 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>> fastrpc_req_mem_unmap_impl(fl, &req_unmap);
>> return -EFAULT;
>> }
>> + kfree(cargs);
>>
>> return 0;
>>
>> err_invoke:
>> fastrpc_map_put(map);
>> + kfree(cargs);
>>
>> return err;
>> }
>> --
>> 2.34.1
>>
Powered by blists - more mailing lists