| Message ID | 1540045588-9091-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 20, 2018 at 11:26:23PM +0900, Akinobu Mita 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> > Cc: Sakari Ailus <sakari.ailus@linux.intel.com> > Cc: Hans Verkuil <hansverk@cisco.com> > Cc: Mauro Carvalho Chehab <mchehab@kernel.org> > Reviewed-by: Matt Ranostay <matt.ranostay@konsulko.com> > Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
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> Reviewed-by: Matt Ranostay <matt.ranostay@konsulko.com> Signed-off-by: Akinobu Mita <akinobu.mita@gmail.com> --- * v4 - Add Reviewed-by line drivers/media/i2c/video-i2c.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)