| Message ID | 1539453759-29976-2-git-send-email-akinobu.mita@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: video-i2c: support changing frame interval and runtime PM | expand |
On Sat, Oct 13, 2018 at 11:02 AM Akinobu Mita <akinobu.mita@gmail.com> wrote: > > The video device release() callback for video-i2c driver frees the whole > struct video_i2c_data. If there is no user left for the video device > when video_unregister_device() is called, the release callback is executed. > > However, in video_i2c_remove() some fields (v4l2_dev, lock, and queue_lock) > in struct video_i2c_data are still accessed after video_unregister_device() > is called. > > This fixes the use after free by moving the code from video_i2c_remove() > to the release() callback. > > Fixes: 5cebaac60974 ("media: video-i2c: add video-i2c driver") > Cc: Matt Ranostay <matt.ranostay@konsulko.com> Reviewed-by: Matt Ranostay <matt.ranostay@konsulko.com> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Hans Verkuil <hansverk@cisco.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> > --- > * v3 > - Move the code causing use-after-free from video_i2c_remove() to the > video device release() callback. > - Remove Acked-by line as there are enough changes from previous version > > drivers/media/i2c/video-i2c.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c > index 06d29d8..f27d294 100644 > --- a/drivers/media/i2c/video-i2c.c > +++ b/drivers/media/i2c/video-i2c.c > @@ -510,7 +510,12 @@ static const struct v4l2_ioctl_ops video_i2c_ioctl_ops = { > > static void video_i2c_release(struct video_device *vdev) > { > - kfree(video_get_drvdata(vdev)); > + struct video_i2c_data *data = video_get_drvdata(vdev); > + > + v4l2_device_unregister(&data->v4l2_dev); > + mutex_destroy(&data->lock); > + mutex_destroy(&data->queue_lock); > + kfree(data); > } > > static int video_i2c_probe(struct i2c_client *client, > @@ -608,10 +613,6 @@ static int video_i2c_remove(struct i2c_client *client) > struct video_i2c_data *data = i2c_get_clientdata(client); > > video_unregister_device(&data->vdev); > - v4l2_device_unregister(&data->v4l2_dev); > - > - mutex_destroy(&data->lock); > - mutex_destroy(&data->queue_lock); > > return 0; > } > -- > 2.7.4 >
diff --git a/drivers/media/i2c/video-i2c.c b/drivers/media/i2c/video-i2c.c index 06d29d8..f27d294 100644 --- a/drivers/media/i2c/video-i2c.c +++ b/drivers/media/i2c/video-i2c.c @@ -510,7 +510,12 @@ static const struct v4l2_ioctl_ops video_i2c_ioctl_ops = { static void video_i2c_release(struct video_device *vdev) { - kfree(video_get_drvdata(vdev)); + struct video_i2c_data *data = video_get_drvdata(vdev); + + v4l2_device_unregister(&data->v4l2_dev); + mutex_destroy(&data->lock); + mutex_destroy(&data->queue_lock); + kfree(data); } static int video_i2c_probe(struct i2c_client *client, @@ -608,10 +613,6 @@ static int video_i2c_remove(struct i2c_client *client) struct video_i2c_data *data = i2c_get_clientdata(client); video_unregister_device(&data->vdev); - v4l2_device_unregister(&data->v4l2_dev); - - mutex_destroy(&data->lock); - mutex_destroy(&data->queue_lock); return 0; }
The video device release() callback for video-i2c driver frees the whole struct video_i2c_data. If there is no user left for the video device when video_unregister_device() is called, the release callback is executed. However, in video_i2c_remove() some fields (v4l2_dev, lock, and queue_lock) in struct video_i2c_data are still accessed after video_unregister_device() is called. This fixes the use after free by moving the code from video_i2c_remove() to the release() callback. Fixes: 5cebaac60974 ("media: video-i2c: add video-i2c driver") Cc: Matt Ranostay <matt.ranostay@konsulko.com> Cc: Sakari Ailus <sakari.ailus@linux.intel.com> Cc: Hans Verkuil <hansverk@cisco.com> Cc: Mauro Carvalho Chehab <mchehab@kernel.org> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- * v3 - Move the code causing use-after-free from video_i2c_remove() to the video device release() callback. - Remove Acked-by line as there are enough changes from previous version drivers/media/i2c/video-i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)