Re: [PATCH v3 8/9] misc: fastrpc: Restrict untrusted app to spawn signed PD

From: Ekansh Gupta
Date: Mon Jun 03 2024 - 02:28:14 EST



On 5/31/2024 5:19 AM, Dmitry Baryshkov wrote:
On Thu, May 30, 2024 at 03:50:26PM +0530, Ekansh Gupta wrote:
Some untrusted applications will not have access to open fastrpc
device nodes and a privileged process can open the device node on
behalf of the application. Add a check to restrict such untrusted
applications from offloading to signed PD.

Fixes: 7f1f481263c3 ("misc: fastrpc: check before loading process to the DSP")
Cc: stable <stable@xxxxxxxxxx>
Signed-off-by: Ekansh Gupta <quic_ekangupt@xxxxxxxxxxx>
---
drivers/misc/fastrpc.c | 23 ++++++++++++++++++-----
1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 73fa0e536cf9..32615ccde7ac 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -328,6 +328,7 @@ struct fastrpc_user {
int pd;
bool is_secure_dev;
bool is_unsigned_pd;
+ bool untrusted_process;
char *servloc_name;
/* Lock for lists */
spinlock_t lock;
@@ -1249,13 +1250,17 @@ static bool is_session_rejected(struct fastrpc_user *fl, bool unsigned_pd_reques
* channel is configured as secure and block untrusted apps on channel
* that does not support unsigned PD offload
*/
- if (!fl->cctx->unsigned_support || !unsigned_pd_request) {
- dev_err(&fl->cctx->rpdev->dev, "Error: Untrusted application trying to offload to signed PD\n");
- return true;
- }
+ if (!fl->cctx->unsigned_support || !unsigned_pd_request)
+ goto reject_session;
}
+ /* Check if untrusted process is trying to offload to signed PD */
+ if (fl->untrusted_process && !unsigned_pd_request)
+ goto reject_session;
return false;
+reject_session:
+ dev_dbg(&fl->cctx->rpdev->dev, "Error: Untrusted application trying to offload to signed PD\n");
+ return true;
}
static void fastrpc_mmap_remove_pdr(struct fastrpc_static_pd *spd)
@@ -1504,12 +1509,20 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
goto err;
}
+ /*
+ * Third-party apps don't have permission to open the fastrpc device, so
Permissions depend on the end-user setup. Is it going to break if the
user sets 0666 mode for fastrpc nodes?

If the root user sets 0666 for fastrpc nodes, it is expected that this check will get bypassed.


+ * it is opened on their behalf by a priveleged process. This is detected
+ * by comparing current PID with the one stored during device open.
+ */
+ if (current->tgid != fl->tgid)
+ fl->untrusted_process = true;
If the comment talks about PIDs, when why are you comparing GIDs here?

It should be GID, I'll update the comment in next spin.


+
if (init.attrs & FASTRPC_MODE_UNSIGNED_MODULE)
fl->is_unsigned_pd = true;
if (is_session_rejected(fl, fl->is_unsigned_pd)) {
- err = -ECONNREFUSED;
+ err = -EACCES;
goto err;
}
--
2.43.0