linux-kernel - Re: [PATCH] media: gspca: stv06xx: Fix memleak in stv06xx subdrivers

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]

Date:   Thu, 22 Apr 2021 21:55:11 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Atul Gopinathan <atulgopinathan@...il.com>
Cc:     mchehab@...nel.org,
        syzbot+990626a4ef6f043ed4cd@...kaller.appspotmail.com,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        linux-kernel-mentees@...ts.linuxfoundation.org,
        linux-media@...r.kernel.org
Subject: Re: [PATCH] media: gspca: stv06xx: Fix memleak in stv06xx
 subdrivers

Hi!

On Thu, 22 Apr 2021 21:37:42 +0530
Atul Gopinathan <atulgopinathan@...il.com> wrote:
> During probing phase of a gspca driver in "gspca_dev_probe2()", the
> stv06xx subdrivers have certain sensor variants (namely, hdcs_1x00,
> hdcs_1020 and pb_0100) that allocate memory for their respective
> sensor which is passed to the "sd->sensor_priv" field. During the
> same probe routine, after "sensor_priv" allocation, there are chances
> of later functions invoked to fail which result in the probing
> routine to end immediately via "goto out" path. While doing so, the
> memory allocated earlier for the sensor isn't taken care of resulting
> in memory leak.
> 
> Fix this by adding operations to the gspca, stv06xx and down to the
> sensor levels to free this allocated memory during gspca probe
> failure.
> 
> -
> The current level of hierarchy looks something like this:
> 
> 	gspca (main driver) represented by struct gspca_dev
> 	   |
> ___________|_____________________________________
> |	|	|	|	|		| (subdrivers)
> 			|			  represented
>  			stv06xx			  by "struct
> sd" |
>  	 _______________|_______________
>  	 |	|	|	|	|  (sensors)
> 	 	|			|
>  		hdcs_1x00/1020		pb01000
> 			|_________________|
> 				|
> 			These three sensor variants
> 			allocate memory for
> 			"sd->sensor_priv" field.
> 
> Here, "struct gspca_dev" is the representation used in the top level.
> In the sub-driver levels, "gspca_dev" pointer is cast to "struct sd*",
> something like this:
> 
> 	struct sd *sd = (struct sd *)gspca_dev;
> 
> This is possible because the first field of "struct sd" is
> "gspca_dev":
> 
> 	struct sd {
> 		struct gspca_dev;
> 		.
> 		.
> 	}
> 
> Therefore, to deallocate the "sd->sensor_priv" fields from
> "gspca_dev_probe2()" which is at the top level, the patch creates
> operations for the subdrivers and sensors to be invoked from the gspca
> driver levels. These operations essentially free the "sd->sensor_priv"
> which were allocated by the "config" and "init_controls" operations in
> the case of stv06xx sub-drivers and the sensor levels.
> 
> This patch doesn't affect other sub-drivers or even sensors who never
> allocate memory to "sensor_priv". It has also been tested by syzbot
> and it returned an "OK" result.
> 
> https://syzkaller.appspot.com/bug?id=ab69427f2911374e5f0b347d0d7795bfe384016c
> -
> 
> Fixes: 4c98834addfe ("V4L/DVB (10048): gspca - stv06xx: New
> subdriver.") Cc: stable@...r.kernel.org
> Suggested-by: Shuah Khan <skhan@...uxfoundation.org>
> Reported-by: syzbot+990626a4ef6f043ed4cd@...kaller.appspotmail.com
> Tested-by: syzbot+990626a4ef6f043ed4cd@...kaller.appspotmail.com
> Signed-off-by: Atul Gopinathan <atulgopinathan@...il.com>

AFAIK, something similar is already applied to linux-media tree
https://git.linuxtv.org/media_tree.git/commit/?id=4f4e6644cd876c844cdb3bea2dd7051787d5ae25

-- 
With regards,
Pavel Skripkin