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
