linux-kernel - Re: [PATCH] Revert "misc: fastrpc: Restrict untrusted app to attach to privileged PD"

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: <b93eda80-828c-4873-a52b-34428f3570df@linaro.org>
Date: Thu, 15 Aug 2024 15:55:01 +0100
From: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
To: Griffin Kroah-Hartman <griffin@...ah.com>, amahesh@....qualcomm.com
Cc: linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, stable <stable@...nel.org>,
 Ekansh Gupta <quic_ekangupt@...cinc.com>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Joel Selvaraj <joelselvaraj.oss@...il.com>
Subject: Re: [PATCH] Revert "misc: fastrpc: Restrict untrusted app to attach
 to privileged PD"



On 15/08/2024 10:49, Griffin Kroah-Hartman wrote:
> This reverts commit bab2f5e8fd5d2f759db26b78d9db57412888f187.
> 
> Joel reported that this commit breaks userspace and stops sensors in
> SDM845 from working. Also breaks other qcom SoC devices running postmarketOS.
> 
> Cc: stable <stable@...nel.org>
> Cc: Ekansh Gupta <quic_ekangupt@...cinc.com>
> Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> Cc: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> Reported-by: Joel Selvaraj <joelselvaraj.oss@...il.com>
> Link: https://lore.kernel.org/r/9a9f5646-a554-4b65-8122-d212bb665c81@umsystem.edu
> Signed-off-by: Griffin Kroah-Hartman <griffin@...ah.com>
> ---


Lets fix this properly by updating existing device tree entries and 
bindings to enforce misuse of "qcom,non-secure-domain" on all Qcom SoC's.

As this patch is only doing part of the job and breaking existing 
platforms,

Acked-by: Srinivas Kandagatla <srinivas.kandagatla@...aro.org>

--srini



--srini
>   drivers/misc/fastrpc.c      | 22 +++-------------------
>   include/uapi/misc/fastrpc.h |  3 ---
>   2 files changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 5204fda51da3..339d126414d4 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2085,16 +2085,6 @@ 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;
> -}
> -
>   static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
>   				 unsigned long arg)
>   {
> @@ -2107,19 +2097,13 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
>   		err = fastrpc_invoke(fl, argp);
>   		break;
>   	case FASTRPC_IOCTL_INIT_ATTACH:
> -		err = is_attach_rejected(fl);
> -		if (!err)
> -			err = fastrpc_init_attach(fl, ROOT_PD);
> +		err = fastrpc_init_attach(fl, ROOT_PD);
>   		break;
>   	case FASTRPC_IOCTL_INIT_ATTACH_SNS:
> -		err = is_attach_rejected(fl);
> -		if (!err)
> -			err = fastrpc_init_attach(fl, SENSORS_PD);
> +		err = fastrpc_init_attach(fl, SENSORS_PD);
>   		break;
>   	case FASTRPC_IOCTL_INIT_CREATE_STATIC:
> -		err = is_attach_rejected(fl);
> -		if (!err)
> -			err = fastrpc_init_create_static_process(fl, argp);
> +		err = fastrpc_init_create_static_process(fl, argp);
>   		break;
>   	case FASTRPC_IOCTL_INIT_CREATE:
>   		err = fastrpc_init_create_process(fl, argp);
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index 91583690bddc..f33d914d8f46 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -8,14 +8,11 @@
>   #define FASTRPC_IOCTL_ALLOC_DMA_BUFF	_IOWR('R', 1, struct fastrpc_alloc_dma_buf)
>   #define FASTRPC_IOCTL_FREE_DMA_BUFF	_IOWR('R', 2, __u32)
>   #define FASTRPC_IOCTL_INVOKE		_IOWR('R', 3, struct fastrpc_invoke)
> -/* This ioctl is only supported with secure device nodes */
>   #define FASTRPC_IOCTL_INIT_ATTACH	_IO('R', 4)
>   #define FASTRPC_IOCTL_INIT_CREATE	_IOWR('R', 5, struct fastrpc_init_create)
>   #define FASTRPC_IOCTL_MMAP		_IOWR('R', 6, struct fastrpc_req_mmap)
>   #define FASTRPC_IOCTL_MUNMAP		_IOWR('R', 7, struct fastrpc_req_munmap)
> -/* This ioctl is only supported with secure device nodes */
>   #define FASTRPC_IOCTL_INIT_ATTACH_SNS	_IO('R', 8)
> -/* This ioctl is only supported with secure device nodes */
>   #define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 9, struct fastrpc_init_create_static)
>   #define FASTRPC_IOCTL_MEM_MAP		_IOWR('R', 10, struct fastrpc_mem_map)
>   #define FASTRPC_IOCTL_MEM_UNMAP		_IOWR('R', 11, struct fastrpc_mem_unmap)