Messages in this thread | | | Subject | Re: [PATCH 4/4] misc: fastrpc: reject non-secure node for secure domain | From | Srinivas Kandagatla <> | Date | Wed, 29 Sep 2021 15:06:51 +0100 |
| |
On 24/09/2021 13:19, Jeya R wrote: > Reject session if domain is secure and device non-secure. Also check if > opened device node is proper. > > Signed-off-by: Jeya R <jeyr@codeaurora.org> > --- > drivers/misc/fastrpc.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index 631713d..adf2700 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -235,6 +235,7 @@ struct fastrpc_user { > spinlock_t lock; > /* lock for allocations */ > struct mutex mutex; > + int dev_minor; > }; > > static void fastrpc_free_map(struct kref *ref) > @@ -1013,6 +1014,17 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, > return err; > } > > +static int is_session_rejected(struct fastrpc_user *fl) > +{ > + /* Check if the device node is non-secure and channel is secure*/ > + if ((fl->dev_minor == fl->cctx->miscdev.minor) && fl->cctx->secure) { > + dev_err(&fl->cctx->rpdev->dev, "Cannot use non-secure device" > + "node on secure channel\n");
Am bit confused here,
so we create 2 device nodes /dev/fastrpc-adsp, /dev/fastrpc-adsp-secure and mark any domain other than cdsp as secure then here we check if access is via /dev/fastrpc-adsp then reject it.
But question is why did we even create /dev/fastrpc-adsp in first place?
Can you explain how is this secure and non secure nodes supposed to work?
> + return -EACCES; > + } > + return 0; > +} > + > static int fastrpc_init_create_process(struct fastrpc_user *fl, > char __user *argp) > { > @@ -1033,6 +1045,10 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl, > } inbuf; > u32 sc; > > + err = is_session_rejected(fl); > + if (err) > + return err; > + > args = kcalloc(FASTRPC_CREATE_PROCESS_NARGS, sizeof(*args), GFP_KERNEL); > if (!args) > return -ENOMEM; > @@ -1221,6 +1237,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) > struct fastrpc_user *fl = NULL; > struct miscdevice *currdev = NULL; > unsigned long flags; > + int dev_minor = MINOR(inode->i_rdev); > > if (!filp) > return -EFAULT; > @@ -1234,6 +1251,12 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) > else > cctx = miscdev_to_cctx(filp->private_data); >
> + if (!((dev_minor == cctx->miscdev.minor) || > + (dev_minor == cctx->securedev.minor))) { > + dev_err(&cctx->rpdev->dev, "Device node is not proper\n"); > + return -EFAULT; > + }
This is bit of over checking, how can we enter without accessing proper device node?
> + > fl = kzalloc(sizeof(*fl), GFP_KERNEL); > if (!fl) > return -ENOMEM; > @@ -1250,6 +1273,7 @@ static int fastrpc_device_open(struct inode *inode, struct file *filp) > INIT_LIST_HEAD(&fl->user); > fl->tgid = current->tgid; > fl->cctx = cctx; > + fl->dev_minor = dev_minor; > > fl->sctx = fastrpc_session_alloc(cctx); > if (!fl->sctx) { >
| |