linux-kernel - Re: [PATCH v1 3/3] misc: fastrpc: Skip reference for DMA handles

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: <7q7rar7ssvzlkol46e5e4yecgt6n4b4oqueam4ywlxjeasx2dl@oydthy337t6i>
Date: Fri, 30 Aug 2024 12:33:50 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
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 v1 3/3] misc: fastrpc: Skip reference for DMA handles

On Thu, Aug 22, 2024 at 04:29:33PM GMT, Ekansh Gupta wrote:
> If multiple dma handles are passed with same fd over a remote call
> the kernel driver takes a reference and expects that put for the
> map will be called as many times to free the map.

> But DSP only
> updates the fd one time in the fd list when the DSP refcount
> goes to zero

I'm sorry, I couldn't understand this phrase. Could you plese clarify
what do you mean here?

> and hence kernel make put call only once for the
> fd. This can cause SMMU fault issue as the same fd can be used
> in future for some other call.
> 
> Fixes: 35a82b87135d ("misc: fastrpc: Add dma handle implementation")
> Cc: stable <stable@...nel.org>
> Signed-off-by: Ekansh Gupta <quic_ekangupt@...cinc.com>
> ---
>  drivers/misc/fastrpc.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index ebe828770a8d..ad56e918e1f8 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -755,7 +755,7 @@ static const struct dma_buf_ops fastrpc_dma_buf_ops = {
>  
>  static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
>  				u64 va, u64 len, u32 attr,
> -				struct fastrpc_map **ppmap)
> +				struct fastrpc_map **ppmap, bool take_ref)
>  {
>  	struct fastrpc_session_ctx *sess = fl->sctx;
>  	struct fastrpc_map *map = NULL;
> @@ -763,7 +763,7 @@ static int fastrpc_map_create(struct fastrpc_user *fl, int fd,
>  	struct scatterlist *sgl = NULL;
>  	int err = 0, sgl_index = 0;
>  
> -	if (!fastrpc_map_lookup(fl, fd, va, len, ppmap, true))
> +	if (!fastrpc_map_lookup(fl, fd, va, len, ppmap, take_ref))
>  		return 0;
>  
>  	map = kzalloc(sizeof(*map), GFP_KERNEL);
> @@ -917,14 +917,17 @@ static int fastrpc_create_maps(struct fastrpc_invoke_ctx *ctx)
>  	int i, err;
>  
>  	for (i = 0; i < ctx->nscalars; ++i) {
> +		bool take_ref = true;
>  
>  		if (ctx->args[i].fd == 0 || ctx->args[i].fd == -1 ||
>  		    ctx->args[i].length == 0)
>  			continue;
>  
> +		if (i >= ctx->nbufs)
> +			take_ref = false;

Please clarify too.

>  		err = fastrpc_map_create(ctx->fl, ctx->args[i].fd,
>  				(u64)ctx->args[i].ptr, ctx->args[i].length,
> -				ctx->args[i].attr, &ctx->maps[i]);
> +				ctx->args[i].attr, &ctx->maps[i], take_ref);
>  		if (err) {
>  			dev_err(dev, "Error Creating map %d\n", err);
>  			return -EINVAL;
> @@ -1417,7 +1420,7 @@ static int fastrpc_init_create_process(struct fastrpc_user *fl,
>  
>  	if (init.filelen && init.filefd) {
>  		err = fastrpc_map_create(fl, init.filefd, init.file,
> -				init.filelen, 0, &map);
> +				init.filelen, 0, &map, true);
>  		if (err)
>  			goto err;
>  	}
> @@ -2040,7 +2043,7 @@ static int fastrpc_req_mem_map(struct fastrpc_user *fl, char __user *argp)
>  
>  	/* create SMMU mapping */
>  	err = fastrpc_map_create(fl, req.fd, req.vaddrin, req.length,
> -			0, &map);
> +			0, &map, true);
>  	if (err) {
>  		dev_err(dev, "failed to map buffer, fd = %d\n", req.fd);
>  		return err;
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry