linux-kernel - Re: [PATCH v3] misc: fastrpc: Increase max user PD initmem size

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: <ZoMcM+F8r58tmE7g@hu-bjorande-lv.qualcomm.com>
Date: Mon, 1 Jul 2024 14:14:27 -0700
From: Bjorn Andersson <quic_bjorande@...cinc.com>
To: Ekansh Gupta <quic_ekangupt@...cinc.com>
CC: <srinivas.kandagatla@...aro.org>, <linux-arm-msm@...r.kernel.org>,
        <gregkh@...uxfoundation.org>, <quic_bkumar@...cinc.com>,
        <linux-kernel@...r.kernel.org>, <quic_chennak@...cinc.com>,
        <dri-devel@...ts.freedesktop.org>, <arnd@...db.de>,
        stable
	<stable@...nel.org>
Subject: Re: [PATCH v3] misc: fastrpc: Increase max user PD initmem size

On Mon, Jul 01, 2024 at 05:22:37PM +0530, Ekansh Gupta wrote:
> For user PD initialization, initmem is allocated and sent to DSP for
> initial memory requirements like shell loading. This size is passed
> by user space and is checked against a max size.

Why does fastrpc_init_create_process() allocate 4x the passed value and
why is the value rounded up to INIT_FILELEN_MAX?

> For unsigned PD
> offloading requirement, more than 2MB size could be passed by user
> which would result in PD initialization failure. Increase the maximum
> size that can be passed py user for user PD initmem allocation.

Sounds good, but why not 2.1MB or a rounder arbitrary value of 8 or 16?

What is actually expected to fit in this initial memory? Is it the shell
that has grown beyond 2MB?

Also, s/py/by

> Any
> additional memory sent to DSP during PD init is used as the PD heap.
> 

Does this mean that the initmem is used for shell loading and initial
heap, and if more heap is needed after that the DSP can request more
memory? Related to the question in v2, how is this memory accounted for?

What would it mean that init.filefd != 0 in
fastrpc_init_create_process(), will that pre-allocated memory (which was
allocated without any size checks afaict) then be used for the same
purpose? Why is a buffer of 4x the size of initfd then also allocated
and mapped to the DSP?

Could you please send a patch adding some comments documenting these
things, the steps taken to create a new process, and what the 6
arguments built in fastrpc_init_create_process() actually means?

Perhaps I'm just failing to read the answers already in the
implementation, if so I'm sorry.

Regards,
Bjorn

> Fixes: 7f1f481263c3 ("misc: fastrpc: check before loading process to the DSP")
> Cc: stable <stable@...nel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.com>
> ---
> Changes in v2:
>   - Modified commit text.
>   - Removed size check instead of updating max file size.
> Changes in v3:
>   - Added bound check again with a higher max size definition.
>   - Modified commit text accordingly.
> 
>  drivers/misc/fastrpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 5204fda51da3..11a230af0b10 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -38,7 +38,7 @@
>  #define FASTRPC_INIT_HANDLE	1
>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
>  #define FASTRPC_CTXID_MASK (0xFF0)
> -#define INIT_FILELEN_MAX (2 * 1024 * 1024)
> +#define INIT_FILELEN_MAX (5 * 1024 * 1024)
>  #define INIT_FILE_NAMELEN_MAX (128)
>  #define FASTRPC_DEVICE_NAME	"fastrpc"
>  
> -- 
> 2.34.1
> 
> 
>