On 4/23/21 2:44 PM, Pavel Skripkin wrote:
Hi!
On Fri, 23 Apr 2021 14:19:15 -0600
Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> wrote:
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
It's not my patch. I've sent a patch sometime ago, but it was reject
by Mauro (we had a small discussion on linux-media mailing-list), then
Hans wrote the patch based on my leak discoverage.
Yes my bad. :)
I added Hans to CC, maybe, he will help :)
Will wait for Hans to take a look.
thanks,
-- Shuah
