Re: [PATCH v2] cxl/mem: Fix potential memory leak

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Sat, 20 Feb 2021 19:58:46 -0800
Ben Widawsky <ben.widawsky@intel.com> wrote:

> When submitting a command for userspace, input and output payload bounce
> buffers are allocated. For a given command, both input and output
> buffers may exist and so when allocation of the input buffer fails, the
> output buffer must be freed too.
> 
> As far as I can tell, userspace can't easily exploit the leak to OOM a
> machine unless the machine was already near OOM state.
> 
> Fixes: 583fa5e71cae ("cxl/mem: Add basic IOCTL interface")
> Reported-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Signed-off-by: Ben Widawsky <ben.widawsky@intel.com>

I'd probably have used a goto and label but I guess this works fine a well.
FWIW on such a small patch.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/mem.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index df895bcca63a..244cb7d89678 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -514,8 +514,10 @@ static int handle_mailbox_cmd_from_user(struct cxl_mem *cxlm,
>  	if (cmd->info.size_in) {
>  		mbox_cmd.payload_in = vmemdup_user(u64_to_user_ptr(in_payload),
>  						   cmd->info.size_in);
> -		if (IS_ERR(mbox_cmd.payload_in))
> +		if (IS_ERR(mbox_cmd.payload_in)) {
> +			kvfree(mbox_cmd.payload_out);
>  			return PTR_ERR(mbox_cmd.payload_in);
> +		}
>  	}
>  
>  	rc = cxl_mem_mbox_get(cxlm);