Em Sat, 27 Feb 2021 02:37:31 +0300 Pavel Skripkin <paskripkin@xxxxxxxxx> escreveu: > Syzbot reported memory leak in hdcs_probe_1x00()[1]. > hdcs_probe_1x00() allocates memory for struct hdcs, but if hdcs_init() fails in gspca_dev_probe2() > this memory becomes leaked. > > int gspca_dev_probe2(struct usb_interface *intf, > const struct usb_device_id *id, > const struct sd_desc *sd_desc, > int dev_size, > struct module *module) > { > ... > > ret = sd_desc->config(gspca_dev, id); > if (ret < 0) > goto out; > ret = sd_desc->init(gspca_dev); > if (ret < 0) > goto out; > ... > out: > if (gspca_dev->input_dev) > input_unregister_device(gspca_dev->input_dev); > v4l2_ctrl_handler_free(gspca_dev->vdev.ctrl_handler); > v4l2_device_unregister(&gspca_dev->v4l2_dev); > kfree(gspca_dev->usb_buf); > kfree(gspca_dev); > return ret; > } > > Reported-by: syzbot+e7f4c64a4248a0340c37@xxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Pavel Skripkin <paskripkin@xxxxxxxxx> > Change-Id: Ia198671177ee346de61780813025110c7c491d7a > --- > drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c > index 5a47dcbf1c8e..24df13b33a02 100644 > --- a/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c > +++ b/drivers/media/usb/gspca/stv06xx/stv06xx_hdcs.c > @@ -485,7 +485,7 @@ static int hdcs_init(struct sd *sd) > stv_bridge_init[i][1]); > } > if (err < 0) > - return err; > + goto error; > > /* sensor soft reset */ > hdcs_reset(sd); > @@ -496,12 +496,12 @@ static int hdcs_init(struct sd *sd) > stv_sensor_init[i][1]); > } > if (err < 0) > - return err; > + goto error; > > /* Enable continuous frame capture, bit 2: stop when frame complete */ > err = stv06xx_write_sensor(sd, HDCS_REG_CONFIG(sd), BIT(3)); > if (err < 0) > - return err; > + goto error; > > /* Set PGA sample duration > (was 0x7E for the STV602, but caused slow framerate with HDCS-1020) */ > @@ -512,9 +512,13 @@ static int hdcs_init(struct sd *sd) > err = stv06xx_write_sensor(sd, HDCS_TCTRL, > (HDCS_ADC_START_SIG_DUR << 5) | hdcs->psmp); > if (err < 0) > - return err; > + goto error; > > return hdcs_set_size(sd, hdcs->array.width, hdcs->array.height); > + > +error: > + kfree(hdcs); > + return err; > } This doesn't seem the right fix here, as it is not the _init function that allocates it. Also, when the device is disconnected, a memory leak will happen. I suspect that the right fix would be to move this: hdcs = kmalloc(sizeof(struct hdcs), GFP_KERNEL); if (!hdcs) return -ENOMEM; To the main driver (stv06xx.c) - probably replacing it by kzalloc(), and then handle the free code both both sd_probe() and sd_disconnect(). Thanks, Mauro