| Message ID | 20240815094920.8242-1-griffin@kroah.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Revert "misc: fastrpc: Restrict untrusted app to attach to privileged PD" | expand |
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@kernel.org> > Cc: Ekansh Gupta <quic_ekangupt@quicinc.com> > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Reported-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> > Link: https://lore.kernel.org/r/9a9f5646-a554-4b65-8122-d212bb665c81@umsystem.edu > Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.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@linaro.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)
On Thu, Aug 15, 2024 at 03:55:01PM GMT, Srinivas Kandagatla wrote: > > > 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@kernel.org> > > Cc: Ekansh Gupta <quic_ekangupt@quicinc.com> > > Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > Reported-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> > > Link: https://lore.kernel.org/r/9a9f5646-a554-4b65-8122-d212bb665c81@umsystem.edu > > Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.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. It is not possible. We must remain compatible with existing DTs. > > As this patch is only doing part of the job and breaking existing platforms, > > Acked-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > --srini > > >
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)
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@kernel.org> Cc: Ekansh Gupta <quic_ekangupt@quicinc.com> Cc: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Cc: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reported-by: Joel Selvaraj <joelselvaraj.oss@gmail.com> Link: https://lore.kernel.org/r/9a9f5646-a554-4b65-8122-d212bb665c81@umsystem.edu Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com> --- drivers/misc/fastrpc.c | 22 +++------------------- include/uapi/misc/fastrpc.h | 3 --- 2 files changed, 3 insertions(+), 22 deletions(-)