| Message ID | 20190718042805.26595-1-kieran.bingham@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | 1a6b80e4a0fa7b2be8fa6c4a25a33f92ce66f563 |
| Headers | show |
| Series |
|
| Related | show |
Hi Kieran, Thank you for the patch. On Thu, Jul 18, 2019 at 05:28:05AM +0100, Kieran Bingham wrote: > The FrameSink::cleanup() call checks if video_ is set before cleaning up > and then deleting the object. > > If the cleanup() call is called twice for any reason, this will > encounter a use-after-free as the video_ pointer is not cleared after > deletion. > > Whilst cleanup() is not currently called twice consecutively, to prevent > errors in the future, make it explicit that the object has been deleted > by clearing the stale pointer. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> It's test code so it doesn't matter much, but it doesn't hurt either, so Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > test/camera/buffer_import.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp > index d6e4fd5bf6ad..400d02b350c1 100644 > --- a/test/camera/buffer_import.cpp > +++ b/test/camera/buffer_import.cpp > @@ -109,7 +109,9 @@ public: > video_->streamOff(); > video_->releaseBuffers(); > video_->close(); > + > delete video_; > + video_ = nullptr; > } > > if (media_)
Hi Kieran, Thanks for your work. On 2019-07-18 05:28:05 +0100, Kieran Bingham wrote: > The FrameSink::cleanup() call checks if video_ is set before cleaning up > and then deleting the object. > > If the cleanup() call is called twice for any reason, this will > encounter a use-after-free as the video_ pointer is not cleared after > deletion. > > Whilst cleanup() is not currently called twice consecutively, to prevent > errors in the future, make it explicit that the object has been deleted > by clearing the stale pointer. > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > test/camera/buffer_import.cpp | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp > index d6e4fd5bf6ad..400d02b350c1 100644 > --- a/test/camera/buffer_import.cpp > +++ b/test/camera/buffer_import.cpp > @@ -109,7 +109,9 @@ public: > video_->streamOff(); > video_->releaseBuffers(); > video_->close(); > + > delete video_; > + video_ = nullptr; > } > > if (media_) > -- > 2.20.1 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index d6e4fd5bf6ad..400d02b350c1 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -109,7 +109,9 @@ public: video_->streamOff(); video_->releaseBuffers(); video_->close(); + delete video_; + video_ = nullptr; } if (media_)
The FrameSink::cleanup() call checks if video_ is set before cleaning up and then deleting the object. If the cleanup() call is called twice for any reason, this will encounter a use-after-free as the video_ pointer is not cleared after deletion. Whilst cleanup() is not currently called twice consecutively, to prevent errors in the future, make it explicit that the object has been deleted by clearing the stale pointer. Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com> --- test/camera/buffer_import.cpp | 2 ++ 1 file changed, 2 insertions(+)