From patchwork Tue Sep 8 07:54:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 9530 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 469C1BDB1D for ; Tue, 8 Sep 2020 07:54:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 135C762BBA; Tue, 8 Sep 2020 09:54:56 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="d5ke+veZ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D329F60534 for ; Tue, 8 Sep 2020 09:54:54 +0200 (CEST) Received: from pendragon.lan (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5621239; Tue, 8 Sep 2020 09:54:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1599551694; bh=DeETnSJKCwRHsa2r3Nv54I3xOIUmCszwDKnaOPsyXMI=; h=From:To:Cc:Subject:Date:From; b=d5ke+veZb/Fq/npUHnoPWK8X2P7N7FPMn675T2TPJySb2YeJVBy3EnVDRIJjrrg4y HK8fsbf38BFT1uUQariVmpevdPVht2caiAb8aqZgJj66OXHANjlnCyOsLjIHCtaXK2 1GnCIs2nxa3z0DRA+LgDnpxxwYTIXX533WwbMtsc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 8 Sep 2020 10:54:24 +0300 Message-Id: <20200908075424.14101-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] android: Protect against null callbacks X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" According to the Android camera HAL C interface documentation, the camera service is supposed to set callbacks after initializing the HAL and calling get_number_of_cameras(), before any other calls to the module. We rely on this behaviour and use callbacks unconditionally, which would lead to a crash if the camera service behaved incorrectly. While the camera service isn't supposed to behave incorrectly, gracefully handling the error when opening cameras isn't costly, and provides better diagnostic than a crash. While at it, removed an unneeded [[maybe_unused]] attribute. Signed-off-by: Laurent Pinchart Reported-by: Coverity CID=298638 Reviewed-by: Niklas Söderlund Reviewed-by: Umang Jain Reviewed-by: Kieran Bingham --- src/android/camera3_hal.cpp | 2 +- src/android/camera_hal_manager.cpp | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp index 67a497b8c829..d6e04af21470 100644 --- a/src/android/camera3_hal.cpp +++ b/src/android/camera3_hal.cpp @@ -32,7 +32,7 @@ static int hal_get_camera_info(int id, struct camera_info *info) return cameraManager.getCameraInfo(id, info); } -static int hal_set_callbacks([[maybe_unused]] const camera_module_callbacks_t *callbacks) +static int hal_set_callbacks(const camera_module_callbacks_t *callbacks) { cameraManager.setCallbacks(callbacks); diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp index a1805ebccf24..05b474010b1d 100644 --- a/src/android/camera_hal_manager.cpp +++ b/src/android/camera_hal_manager.cpp @@ -29,7 +29,7 @@ LOG_DECLARE_CATEGORY(HAL); */ CameraHalManager::CameraHalManager() - : cameraManager_(nullptr), numInternalCameras_(0), + : cameraManager_(nullptr), callbacks_(nullptr), numInternalCameras_(0), nextExternalCameraId_(firstExternalCameraId_) { } @@ -70,6 +70,11 @@ CameraDevice *CameraHalManager::open(unsigned int id, { MutexLocker locker(mutex_); + if (!callbacks_) { + LOG(HAL, Error) << "Can't open camera before callbacks are set"; + return nullptr; + } + CameraDevice *camera = cameraDeviceFromHalId(id); if (!camera) { LOG(HAL, Error) << "Invalid camera id '" << id << "'";