Re: [PATCH for-rc 1/6] IB/hfi1: Restore allocated resources on failed copyout

[Leon Romanovsky <leon@kernel.org>]

On Mon, Jan 09, 2023 at 12:31:06PM -0500, Dennis Dalessandro wrote:
> From: Dean Luick <dean.luick@cornelisnetworks.com>
> 
> Fix a resource leak if an error occurs.
> 
> Fixes: f404ca4c7ea8 ("IB/hfi1: Refactor hfi_user_exp_rcv_setup() IOCTL")
> Signed-off-by: Dean Luick <dean.luick@cornelisnetworks.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
> ---
>  drivers/infiniband/hw/hfi1/file_ops.c |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
> index f5f9269fdc16..c9fc913db00c 100644
> --- a/drivers/infiniband/hw/hfi1/file_ops.c
> +++ b/drivers/infiniband/hw/hfi1/file_ops.c
> @@ -1318,12 +1318,15 @@ static int user_exp_rcv_setup(struct hfi1_filedata *fd, unsigned long arg,
>  		addr = arg + offsetof(struct hfi1_tid_info, tidcnt);
>  		if (copy_to_user((void __user *)addr, &tinfo.tidcnt,
>  				 sizeof(tinfo.tidcnt)))
> -			return -EFAULT;
> +			ret = -EFAULT;

I don't think that it is right to continue to next copy_to_user() if
first one failed.

Thanks

>  
>  		addr = arg + offsetof(struct hfi1_tid_info, length);
>  		if (copy_to_user((void __user *)addr, &tinfo.length,
>  				 sizeof(tinfo.length)))
>  			ret = -EFAULT;
> +
> +		if (ret)
> +			hfi1_user_exp_rcv_invalid(fd, &tinfo);
>  	}
>  
>  	return ret;
> 
>