This patch series adds secure domains support. All DSP domains other
than CDSP are set as secure by default and CDSP is set as secure domain
if fastrpc DT node carries secure domains property. If any process is
getting initialized using non-secure device and the dsp channel is
secure, then the session gets rejected.
Jeya R (4):
dt-bindings: devicetree documentation for secure domain
misc: fastrpc: Add secure device node support
misc: fastrpc: Set channel as secure
misc: fastrpc: reject non-secure node for secure domain
.../devicetree/bindings/misc/qcom,fastrpc.txt | 6 ++
drivers/misc/fastrpc.c | 64 +++++++++++++++++++++-
2 files changed, 68 insertions(+), 2 deletions(-)
--
2.7.4
Reject session if domain is secure and device non-secure. Also check if
opened device node is proper.
Signed-off-by: Jeya R <[email protected]>
---
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");
+ 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;
+ }
+
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) {
--
2.7.4
On 24/09/2021 13:19, Jeya R wrote:
> This patch series adds secure domains support. All DSP domains other
> than CDSP are set as secure by default and CDSP is set as secure domain
This is going to break the existing devices that work with this driver?
Alteast the non cdsp cases.
like msm8996, sdm845, sm8250 ....
> if fastrpc DT node carries secure domains property. If any process is
> getting initialized using non-secure device and the dsp channel is
> secure, then the session gets rejected.
Could you elaborate on what exactly you meant by secure here?
Is this SE linux policy we are talking about ?
Why can't we deal with this directly on /dev/[adsp|cdsp]-fastrpc nodes,
why do we need this extra secured node?
--srini
>
> Jeya R (4):
> dt-bindings: devicetree documentation for secure domain
> misc: fastrpc: Add secure device node support
> misc: fastrpc: Set channel as secure
> misc: fastrpc: reject non-secure node for secure domain
>
> .../devicetree/bindings/misc/qcom,fastrpc.txt | 6 ++
> drivers/misc/fastrpc.c | 64 +++++++++++++++++++++-
> 2 files changed, 68 insertions(+), 2 deletions(-)
>
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 <[email protected]>
> ---
> 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) {
>