linux-kernel - Re: [PATCH] media: airspy: fix memory leak in airspy probe

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] [day] [month] [year] [list]

Date:   Thu, 11 Aug 2022 15:42:48 +0200
From:   Tommaso Merciai <tommaso.merciai@...rulasolutions.com>
To:     Dongliang Mu <dzm91@...t.edu.cn>
Cc:     Antti Palosaari <crope@....fi>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Hans Verkuil <hverkuil-cisco@...all.nl>,
        Oliver Neukum <oneukum@...e.com>,
        Dongliang Mu <mudongliangabcd@...il.com>,
        syzbot+bb25f85e5aa482864dc0@...kaller.appspotmail.com,
        linux-media@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] media: airspy: fix memory leak in airspy probe

Hi Dongliang,

On Thu, Aug 11, 2022 at 12:57:00PM +0800, Dongliang Mu wrote:
> From: Dongliang Mu <mudongliangabcd@...il.com>
> 
> The commit ca9dc8d06ab6 ("media: airspy: respect the DMA coherency
>  rules") moves variable buf from stack to heap, however, it only frees
> buf in the error handling code, missing deallocation in the success
> path.
> 
> Fix this by freeing buf in the success path since this variable does not
> have any references in other code.
> 
> Fixes: ca9dc8d06ab6 ("media: airspy: respect the DMA coherency rules")
> Reported-by: syzbot+bb25f85e5aa482864dc0@...kaller.appspotmail.com
> Signed-off-by: Dongliang Mu <mudongliangabcd@...il.com>
> ---
>  drivers/media/usb/airspy/airspy.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/media/usb/airspy/airspy.c b/drivers/media/usb/airspy/airspy.c
> index 240a7cc56777..7b1c40132555 100644
> --- a/drivers/media/usb/airspy/airspy.c
> +++ b/drivers/media/usb/airspy/airspy.c
> @@ -1070,6 +1070,10 @@ static int airspy_probe(struct usb_interface *intf,
>  				ret);
>  		goto err_free_controls;
>  	}
> +
> +	/* Free buf if success*/
> +	kfree(buf);
> +

buf var is only used to print the fw version during the probe.
I think you are right we need to free this.

Reviewed-by: Tommaso Merciai <tommaso.merciai@...rulasolution.com>

>  	dev_info(s->dev, "Registered as %s\n",
>  			video_device_node_name(&s->vdev));
>  	dev_notice(s->dev, "SDR API is still slightly experimental and functionality changes may follow\n");
> -- 
> 2.35.1
> 

Regards,
Tommaso

-- 
Tommaso Merciai
Embedded Linux Engineer
tommaso.merciai@...rulasolutions.com
__________________________________

Amarula Solutions SRL
Via Le Canevare 30, 31100 Treviso, Veneto, IT
T. +39 042 243 5310
info@...rulasolutions.com
www.amarulasolutions.com