| Message ID | 20200616131244.70308-14-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, On Tue, Jun 16, 2020 at 10:12:42PM +0900, Paul Elder wrote: > V4L2 allows buffer queueing before streamon while libcamera does not. > The compatibility layer thus saves these buffers in a pending queue > until streamon, and then automatically queues them. However, this > pending queue is not cleared when the buffers a freed, so if buffers are s/a freed/are freed. > queued, the stream is not started, buffers are freed, more buffers are > queued, and the stream is finally started, then the first set of buffers > will be used-after-free. Fix this by clearing the pending quest queue > upon the buffers being freed. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> Should this happen at streamoff time as well ? Got a bit lost :) Otherwise Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > --- > src/v4l2/v4l2_camera.cpp | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > index f0ec54b..bae270a 100644 > --- a/src/v4l2/v4l2_camera.cpp > +++ b/src/v4l2/v4l2_camera.cpp > @@ -155,6 +155,7 @@ void V4L2Camera::freeBuffers() > Stream *stream = *camera_->streams().begin(); > > bufferAllocator_->free(stream); > + pendingRequests_.clear(); > } > > FileDescriptor V4L2Camera::getBufferFd(unsigned int index) > -- > 2.27.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
On Wed, Jun 17, 2020 at 05:40:54PM +0200, Jacopo Mondi wrote: > Hi Paul, > > On Tue, Jun 16, 2020 at 10:12:42PM +0900, Paul Elder wrote: > > V4L2 allows buffer queueing before streamon while libcamera does not. > > The compatibility layer thus saves these buffers in a pending queue > > until streamon, and then automatically queues them. However, this > > pending queue is not cleared when the buffers a freed, so if buffers are > > s/a freed/are freed. > > > queued, the stream is not started, buffers are freed, more buffers are > > queued, and the stream is finally started, then the first set of buffers > > will be used-after-free. Fix this by clearing the pending quest queue > > upon the buffers being freed. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > Should this happen at streamoff time as well ? Oh you're right yeah, it should. > Got a bit lost :) I'll try to clarify it. > Otherwise > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > Thanks, Paul > > --- > > src/v4l2/v4l2_camera.cpp | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp > > index f0ec54b..bae270a 100644 > > --- a/src/v4l2/v4l2_camera.cpp > > +++ b/src/v4l2/v4l2_camera.cpp > > @@ -155,6 +155,7 @@ void V4L2Camera::freeBuffers() > > Stream *stream = *camera_->streams().begin(); > > > > bufferAllocator_->free(stream); > > + pendingRequests_.clear(); > > } > > > > FileDescriptor V4L2Camera::getBufferFd(unsigned int index) > > -- > > 2.27.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index f0ec54b..bae270a 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -155,6 +155,7 @@ void V4L2Camera::freeBuffers() Stream *stream = *camera_->streams().begin(); bufferAllocator_->free(stream); + pendingRequests_.clear(); } FileDescriptor V4L2Camera::getBufferFd(unsigned int index)
V4L2 allows buffer queueing before streamon while libcamera does not. The compatibility layer thus saves these buffers in a pending queue until streamon, and then automatically queues them. However, this pending queue is not cleared when the buffers a freed, so if buffers are queued, the stream is not started, buffers are freed, more buffers are queued, and the stream is finally started, then the first set of buffers will be used-after-free. Fix this by clearing the pending quest queue upon the buffers being freed. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/v4l2/v4l2_camera.cpp | 1 + 1 file changed, 1 insertion(+)