| Message ID | 20211001211525.1423725-1-javierm@redhat.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Javier, Thank you for the patch. On Fri, Oct 01, 2021 at 11:15:24PM +0200, Javier Martinez Canillas wrote: > Currently, the gst_libcamera_device_new() function assumes that a call to > Camera::generateConfiguration() will always succeed, but that may not be > the case and the return value must to be checked. > > Otherwise, this could lead to a NULL pointer dereference if the pipeline > handler fails to generate a config for the VideoRecording stream role. > > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com> > Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I'll push this patch independently of 2/2 as there's an open question there. > --- > > Changes in v2: > - Fix typos in commit message as pointed out by Nicolas Dufresne. > - Add Nicolas Dufresne's Reviewed-by tag. > > src/gstreamer/gstlibcameraprovider.cpp | 14 +++++++++++++- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp > index aee6f9a2be2..6eb0a0eb0c4 100644 > --- a/src/gstreamer/gstlibcameraprovider.cpp > +++ b/src/gstreamer/gstlibcameraprovider.cpp > @@ -132,6 +132,10 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera) > > roles.push_back(StreamRole::VideoRecording); > std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles); > + if (!config || config->size() != roles.size()) { > + GST_ERROR("Failed to generate a default configuration for %s", name); > + return nullptr; > + } > > for (const StreamConfiguration &stream_cfg : *config) { > GstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats()); > @@ -189,8 +193,16 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider) > > for (const std::shared_ptr<Camera> &camera : cm->cameras()) { > GST_INFO_OBJECT(self, "Found camera '%s'", camera->id().c_str()); > + > + GstDevice *dev = gst_libcamera_device_new(camera); > + if (!dev) { > + GST_ERROR_OBJECT(self, "Failed to add camera '%s'", > + camera->id().c_str()); > + return nullptr; > + } > + > devices = g_list_append(devices, > - g_object_ref_sink(gst_libcamera_device_new(camera))); > + g_object_ref_sink(dev)); > } > > return devices;
diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp index aee6f9a2be2..6eb0a0eb0c4 100644 --- a/src/gstreamer/gstlibcameraprovider.cpp +++ b/src/gstreamer/gstlibcameraprovider.cpp @@ -132,6 +132,10 @@ gst_libcamera_device_new(const std::shared_ptr<Camera> &camera) roles.push_back(StreamRole::VideoRecording); std::unique_ptr<CameraConfiguration> config = camera->generateConfiguration(roles); + if (!config || config->size() != roles.size()) { + GST_ERROR("Failed to generate a default configuration for %s", name); + return nullptr; + } for (const StreamConfiguration &stream_cfg : *config) { GstCaps *sub_caps = gst_libcamera_stream_formats_to_caps(stream_cfg.formats()); @@ -189,8 +193,16 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider) for (const std::shared_ptr<Camera> &camera : cm->cameras()) { GST_INFO_OBJECT(self, "Found camera '%s'", camera->id().c_str()); + + GstDevice *dev = gst_libcamera_device_new(camera); + if (!dev) { + GST_ERROR_OBJECT(self, "Failed to add camera '%s'", + camera->id().c_str()); + return nullptr; + } + devices = g_list_append(devices, - g_object_ref_sink(gst_libcamera_device_new(camera))); + g_object_ref_sink(dev)); } return devices;