On Thu, Apr 22, 2021 at 09:55:11PM +0300, 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
Oh, my bad. Thanks for pointing it out. The syzkaller page of this bug
hasn't been updated. I will send an e-mail and mark it as fixed.
Regards,
Atul
