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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 4/22/21 12:55 PM, Pavel Skripkin wrote:
Hi!

On Thu, 22 Apr 2021 21:37:42 +0530
Atul Gopinathan <atulgopinathan@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
Suggested-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
Reported-by: syzbot+990626a4ef6f043ed4cd@xxxxxxxxxxxxxxxxxxxxxxxxx
Tested-by: syzbot+990626a4ef6f043ed4cd@xxxxxxxxxxxxxxxxxxxxxxxxx
Signed-off-by: Atul Gopinathan <atulgopinathan@xxxxxxxxx>

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


Pavel,

Does the above handle the other drivers hdcs_1x00/1020 and pb01000?

Atul's patch handles those cases. If thoese code paths need to be fixes,
Atul could do a patch on top of yours perhaps?

thanks,
-- Shuah





[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux