[PATCH] media: gspca: m5602: fix NULL sensor deref on disconnect

Farhad Alemi posted 1 patch 2 weeks ago
[PATCH] media: gspca: m5602: fix NULL sensor deref on disconnect
Posted by Farhad Alemi 2 weeks ago
The m5602 sensor ->disconnect handlers store NULL into sd->sensor, and
m5602_disconnect() runs them before gspca_disconnect() stops streaming,
so two paths then fault on that pointer: m5602_stop_transfer() reads
sd->sensor->stop when gspca_stream_off() runs the stopN callback at
release, and m5602_write_sensor() reads sd->sensor->i2c_regW when a
read() already inside gspca_init_transfer() reaches it through the
->start dispatch. The six sensor descriptors are static const and are
never freed, so the store guards nothing: drop it from every handler,
and remove the three handlers, their prototypes and their .disconnect
initializers that are then left with nothing to do.
m5602_stop_transfer() also runs arbitrarily later than disconnect, by
which time gspca_dev->dev is freed because gspca takes no reference on
the usb_device, so return early there when gspca_dev->present is clear.

Closes: https://lore.kernel.org/all/CA+0ovCh63ei3SWria1JexWc0r9ckhj=XVNhXSzgHQFMUEj+ypA@mail.gmail.com/
Signed-off-by: Farhad Alemi <farhad.alemi@berkeley.edu>
---
The device was emulated.

--- a/drivers/media/usb/gspca/m5602/m5602_core.c
+++ b/drivers/media/usb/gspca/m5602/m5602_core.c
@@ -356,6 +356,12 @@ static void m5602_stop_transfer(struct gspca_dev
*gspca_dev)
 {
 	struct sd *sd = (struct sd *) gspca_dev;

+	/* gspca_stream_off() runs this even after disconnect, when the USB
+	 * device every sensor register access goes through is already freed.
+	 */
+	if (!gspca_dev->present)
+		return;
+
 	/* Run the sensor specific end transfer sequence */
 	if (sd->sensor->stop)
 		sd->sensor->stop(sd);
--- a/drivers/media/usb/gspca/m5602/m5602_mt9m111.c
+++ b/drivers/media/usb/gspca/m5602/m5602_mt9m111.c
@@ -383,11 +383,6 @@ int mt9m111_start(struct sd *sd)
 	return err;
 }

-void mt9m111_disconnect(struct sd *sd)
-{
-	sd->sensor = NULL;
-}
-
 static int mt9m111_set_hvflip(struct gspca_dev *gspca_dev)
 {
 	int err;
--- a/drivers/media/usb/gspca/m5602/m5602_mt9m111.h
+++ b/drivers/media/usb/gspca/m5602/m5602_mt9m111.h
@@ -108,7 +108,6 @@ int mt9m111_probe(struct sd *sd);
 int mt9m111_init(struct sd *sd);
 int mt9m111_init_controls(struct sd *sd);
 int mt9m111_start(struct sd *sd);
-void mt9m111_disconnect(struct sd *sd);

 static const struct m5602_sensor mt9m111 = {
 	.name = "MT9M111",
@@ -119,7 +118,6 @@ static const struct m5602_sensor mt9m111 = {
 	.probe = mt9m111_probe,
 	.init = mt9m111_init,
 	.init_controls = mt9m111_init_controls,
-	.disconnect = mt9m111_disconnect,
 	.start = mt9m111_start,
 };
 #endif
--- a/drivers/media/usb/gspca/m5602/m5602_ov7660.c
+++ b/drivers/media/usb/gspca/m5602/m5602_ov7660.c
@@ -316,8 +316,6 @@ int ov7660_stop(struct sd *sd)
 void ov7660_disconnect(struct sd *sd)
 {
 	ov7660_stop(sd);
-
-	sd->sensor = NULL;
 }

 static int ov7660_set_gain(struct gspca_dev *gspca_dev, __s32 val)
--- a/drivers/media/usb/gspca/m5602/m5602_ov9650.c
+++ b/drivers/media/usb/gspca/m5602/m5602_ov9650.c
@@ -544,8 +544,6 @@ int ov9650_stop(struct sd *sd)
 void ov9650_disconnect(struct sd *sd)
 {
 	ov9650_stop(sd);
-
-	sd->sensor = NULL;
 }

 static int ov9650_set_exposure(struct gspca_dev *gspca_dev, __s32 val)
--- a/drivers/media/usb/gspca/m5602/m5602_po1030.c
+++ b/drivers/media/usb/gspca/m5602/m5602_po1030.c
@@ -543,11 +543,6 @@ static int po1030_set_auto_exposure(struct
gspca_dev *gspca_dev,
 	return m5602_write_sensor(sd, PO1030_AUTOCTRL1, &i2c_data, 1);
 }

-void po1030_disconnect(struct sd *sd)
-{
-	sd->sensor = NULL;
-}
-
 static int po1030_s_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct gspca_dev *gspca_dev =
--- a/drivers/media/usb/gspca/m5602/m5602_po1030.h
+++ b/drivers/media/usb/gspca/m5602/m5602_po1030.h
@@ -149,7 +149,6 @@ int po1030_probe(struct sd *sd);
 int po1030_init(struct sd *sd);
 int po1030_init_controls(struct sd *sd);
 int po1030_start(struct sd *sd);
-void po1030_disconnect(struct sd *sd);

 static const struct m5602_sensor po1030 = {
 	.name = "PO1030",
@@ -161,6 +160,5 @@ static const struct m5602_sensor po1030 = {
 	.init = po1030_init,
 	.init_controls = po1030_init_controls,
 	.start = po1030_start,
-	.disconnect = po1030_disconnect,
 };
 #endif
--- a/drivers/media/usb/gspca/m5602/m5602_s5k4aa.c
+++ b/drivers/media/usb/gspca/m5602/m5602_s5k4aa.c
@@ -709,11 +709,6 @@ static int s5k4aa_s_ctrl(struct v4l2_ctrl *ctrl)
 	return err;
 }

-void s5k4aa_disconnect(struct sd *sd)
-{
-	sd->sensor = NULL;
-}
-
 static void s5k4aa_dump_registers(struct sd *sd)
 {
 	int address;
--- a/drivers/media/usb/gspca/m5602/m5602_s5k4aa.h
+++ b/drivers/media/usb/gspca/m5602/m5602_s5k4aa.h
@@ -67,7 +67,6 @@ int s5k4aa_probe(struct sd *sd);
 int s5k4aa_init(struct sd *sd);
 int s5k4aa_init_controls(struct sd *sd);
 int s5k4aa_start(struct sd *sd);
-void s5k4aa_disconnect(struct sd *sd);

 static const struct m5602_sensor s5k4aa = {
 	.name = "S5K4AA",
@@ -78,7 +77,6 @@ static const struct m5602_sensor s5k4aa = {
 	.init = s5k4aa_init,
 	.init_controls = s5k4aa_init_controls,
 	.start = s5k4aa_start,
-	.disconnect = s5k4aa_disconnect,
 };

 #endif
--- a/drivers/media/usb/gspca/m5602/m5602_s5k83a.c
+++ b/drivers/media/usb/gspca/m5602/m5602_s5k83a.c
@@ -374,8 +374,6 @@ int s5k83a_stop(struct sd *sd)
 void s5k83a_disconnect(struct sd *sd)
 {
 	s5k83a_stop(sd);
-
-	sd->sensor = NULL;
 }

 static int s5k83a_set_gain(struct gspca_dev *gspca_dev, __s32 val)