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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <9a9f5646-a554-4b65-8122-d212bb665c81@umsystem.edu>
Date: Thu, 15 Aug 2024 02:34:18 +0000
From: "Selvaraj, Joel (MU-Student)" <jsbrq@...souri.edu>
To: "srinivas.kandagatla@...aro.org" <srinivas.kandagatla@...aro.org>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Ekansh
 Gupta <quic_ekangupt@...cinc.com>, stable <stable@...nel.org>, Dmitry
 Baryshkov <dmitry.baryshkov@...aro.org>
Subject: Re: [PATCH 6/6] misc: fastrpc: Restrict untrusted app to attach to
 privileged PD

Hi Srinivas Kandagatla and Ekansh Gupta,

On 6/28/24 06:45, srinivas.kandagatla@...aro.org wrote:
> From: Ekansh Gupta <quic_ekangupt@...cinc.com>
> 
> Untrusted application with access to only non-secure fastrpc device
> node can attach to root_pd or static PDs if it can make the respective
> init request. This can cause problems as the untrusted application
> can send bad requests to root_pd or static PDs. Add changes to reject
> attach to privileged PDs if the request is being made using non-secure
> fastrpc device node.
> 
> Fixes: 0871561055e6 ("misc: fastrpc: Add support for audiopd")
> Cc: stable <stable@...nel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.com>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> ---
>   drivers/misc/fastrpc.c      | 22 +++++++++++++++++++---
>   include/uapi/misc/fastrpc.h |  3 +++
>   2 files changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 5680856c0fb8..a7a2bcedb37e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2087,6 +2087,16 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>   	return err;
>   }
>   
> +static int is_attach_rejected(struct fastrpc_user *fl)
> +{
> +	/* Check if the device node is non-secure */
> +	if (!fl->is_secure_dev) {
> +		dev_dbg(&fl->cctx->rpdev->dev, "untrusted app trying to attach to privileged DSP PD\n");
> +		return -EACCES;
> +	}
> +	return 0;
> +}

This broke userspace for us. Sensors stopped working in SDM845 and other 
qcom SoC devices running postmarketOS. Trying to communicate with the 
fastrpc device just ends up with a permission denied error. This was 
previously working. I am not sure if this is intended. Here are my two 
observations:

1. if change the if condition to

`if (!fl->is_secure_dev && fl->cctx->secure)`

similar to how it's done in fastrpc's `is_session_rejected()` function, 
then it works. But I am not sure if this is an valid fix. But currently, 
fastrpc will simply deny access to all fastrpc device that contains the 
`qcom,non-secure-domain` dt property. Is that the intended change? 
Because I see a lot of adsp, cdsp and sdsp fastrpc nodes have that dt 
property.

2. In the `fastrpc_rpmsg_probe()` function, it is commented that,

"Unsigned PD offloading is only supported on CDSP"

Does this mean adsp and sdsp shouldn't have the `qcom,non-secure-domain` 
dt property? In fact, it was reported that removing this dt property and 
using the `/dev/fastrpc-sdsp-secure` node instead works fine too. Is 
this the correct way to fix it?

I don't know much about fastrpc, just reporting the issue and guessing 
here. It would be really if this can be fixed before the stable release.

Thank you,
Joel Selvaraj


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ