libcamera2ndk_vendor: Fix potential use after free of camera_metadata_t

Bug: 131566406

Test: Use libcamera2ndk_vendor multiple times without seeing logs /
      assertions indicating null metadata / corrupted metadata in
      allocateACaptureRequest.

Change-Id: I2154a83bb97a4dd945f15328769b811e9485a0ac
Signed-off-by: Jayant Chowdhary <jchowdhary@google.com>
gugelfrei
Jayant Chowdhary 5 years ago
parent 4714c214a0
commit b61a9d4cde

@ -262,12 +262,12 @@ camera_status_t CameraDevice::isSessionConfigurationSupported(
void CameraDevice::addRequestSettingsMetadata(ACaptureRequest *aCaptureRequest, void CameraDevice::addRequestSettingsMetadata(ACaptureRequest *aCaptureRequest,
sp<CaptureRequest> &req) { sp<CaptureRequest> &req) {
CameraMetadata metadataCopy = aCaptureRequest->settings->getInternalData(); CameraMetadata metadataCopy = aCaptureRequest->settings->getInternalData();
const camera_metadata_t *camera_metadata = metadataCopy.getAndLock(); camera_metadata_t *camera_metadata = metadataCopy.release();
HCameraMetadata hCameraMetadata; HCameraMetadata hCameraMetadata;
utils::convertToHidl(camera_metadata, &hCameraMetadata); utils::convertToHidl(camera_metadata, &hCameraMetadata, true);
metadataCopy.unlock(camera_metadata);
req->mPhysicalCameraSettings.resize(1); req->mPhysicalCameraSettings.resize(1);
req->mPhysicalCameraSettings[0].settings.metadata(std::move(hCameraMetadata)); req->mPhysicalCameraSettings[0].settings.metadata(std::move(hCameraMetadata));
req->mPhysicalCameraSettings[0].id = getId();
} }
camera_status_t CameraDevice::updateOutputConfigurationLocked(ACaptureSessionOutput *output) { camera_status_t CameraDevice::updateOutputConfigurationLocked(ACaptureSessionOutput *output) {
@ -398,10 +398,9 @@ void CameraDevice::allocateOneCaptureRequestMetadata(
cameraSettings.id = id; cameraSettings.id = id;
// TODO: Do we really need to copy the metadata here ? // TODO: Do we really need to copy the metadata here ?
CameraMetadata metadataCopy = metadata->getInternalData(); CameraMetadata metadataCopy = metadata->getInternalData();
const camera_metadata_t *cameraMetadata = metadataCopy.getAndLock(); camera_metadata_t *cameraMetadata = metadataCopy.release();
HCameraMetadata hCameraMetadata; HCameraMetadata hCameraMetadata;
utils::convertToHidl(cameraMetadata, &hCameraMetadata); utils::convertToHidl(cameraMetadata, &hCameraMetadata, true);
metadataCopy.unlock(cameraMetadata);
if (metadata != nullptr) { if (metadata != nullptr) {
if (hCameraMetadata.data() != nullptr && if (hCameraMetadata.data() != nullptr &&
mCaptureRequestMetadataQueue != nullptr && mCaptureRequestMetadataQueue != nullptr &&
@ -426,11 +425,12 @@ CameraDevice::allocateACaptureRequest(sp<CaptureRequest>& req, const char* devic
const std::string& id = req->mPhysicalCameraSettings[i].id; const std::string& id = req->mPhysicalCameraSettings[i].id;
CameraMetadata clone; CameraMetadata clone;
utils::convertFromHidlCloned(req->mPhysicalCameraSettings[i].settings.metadata(), &clone); utils::convertFromHidlCloned(req->mPhysicalCameraSettings[i].settings.metadata(), &clone);
camera_metadata_t *clonep = clone.release();
if (id == deviceId) { if (id == deviceId) {
pRequest->settings = new ACameraMetadata(clone.release(), ACameraMetadata::ACM_REQUEST); pRequest->settings = new ACameraMetadata(clonep, ACameraMetadata::ACM_REQUEST);
} else { } else {
pRequest->physicalSettings[req->mPhysicalCameraSettings[i].id] = pRequest->physicalSettings[req->mPhysicalCameraSettings[i].id] =
new ACameraMetadata(clone.release(), ACameraMetadata::ACM_REQUEST); new ACameraMetadata(clonep, ACameraMetadata::ACM_REQUEST);
} }
} }
pRequest->targets = new ACameraOutputTargets(); pRequest->targets = new ACameraOutputTargets();

@ -64,13 +64,14 @@ bool convertFromHidlCloned(const HCameraMetadata &metadata, CameraMetadata *rawM
return true; return true;
} }
// Note: existing data in dst will be gone. Caller still owns the memory of src // Note: existing data in dst will be gone. dst owns memory if shouldOwn is set
void convertToHidl(const camera_metadata_t *src, HCameraMetadata* dst) { // to true.
void convertToHidl(const camera_metadata_t *src, HCameraMetadata* dst, bool shouldOwn) {
if (src == nullptr) { if (src == nullptr) {
return; return;
} }
size_t size = get_camera_metadata_size(src); size_t size = get_camera_metadata_size(src);
dst->setToExternal((uint8_t *) src, size); dst->setToExternal((uint8_t *) src, size, shouldOwn);
return; return;
} }

@ -168,8 +168,8 @@ HRotation convertToHidl(int rotation);
bool convertFromHidlCloned(const HCameraMetadata &metadata, CameraMetadata *rawMetadata); bool convertFromHidlCloned(const HCameraMetadata &metadata, CameraMetadata *rawMetadata);
// Note: existing data in dst will be gone. Caller still owns the memory of src // Note: existing data in dst will be gone.
void convertToHidl(const camera_metadata_t *src, HCameraMetadata* dst); void convertToHidl(const camera_metadata_t *src, HCameraMetadata* dst, bool shouldOwn = false);
TemplateId convertToHidl(ACameraDevice_request_template templateId); TemplateId convertToHidl(ACameraDevice_request_template templateId);

Loading…
Cancel
Save