Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ea30826b authored by Avichal Rakesh's avatar Avichal Rakesh Committed by Android Build Cherrypicker Worker
Browse files

ExternalCameraHAL: dup fd when creating AIDL NativeHandle 

AIDL's NativeHandle do not have a concept of unowned file
descriptors. If a NativeHandle object is created with an
fd, NativeHandle implicitly assumes ownership of the fd.
When passing fds over binder, ExternalCameraHAL used
makeToAidl which which accidentally transferred ownership
to the AIDL objects. Additionally, NativeHandles close
owned fds on destruction, which led to multiple closure
of fences.

This CL changes the logic to use dupToAidl to ensure that
NativeHandle objects are given ownership of a duped fds
and don't interfere with any of the fds used for internal
bookkeeping.

Bug: 313115623
Test: Verified by partner that ExternalCameraHAL no longer double closes fds.
Merged-In: Ic406634de6f22a290abb414e80a7747927368b68
Change-Id: Ic406634de6f22a290abb414e80a7747927368b68
parent a41ff513

camera/device/default/ExternalCameraDeviceSession.cpp

+16 −8
Original line number Diff line number Diff line
@@ -789,8 +789,10 @@ Status ExternalCameraDeviceSession::switchToOffline( 
                outputBuffer.bufferId = buffer.bufferId;

                outputBuffer.status = BufferStatus::ERROR;

                if (buffer.acquireFence >= 0) {

                    outputBuffer.releaseFence.fds.resize(1);

                    outputBuffer.releaseFence.fds.at(0).set(buffer.acquireFence);

                    native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

                    handle->data[0] = buffer.acquireFence;

                    outputBuffer.releaseFence = android::dupToAidl(handle);

                    native_handle_delete(handle);

                }

            } else {

                offlineBuffers.push_back(buffer);
@@ -1768,8 +1770,10 @@ Status ExternalCameraDeviceSession::processCaptureRequestError( 
        result.outputBuffers[i].bufferId = req->buffers[i].bufferId;

        result.outputBuffers[i].status = BufferStatus::ERROR;

        if (req->buffers[i].acquireFence >= 0) {

            result.outputBuffers[i].releaseFence.fds.resize(1);

            result.outputBuffers[i].releaseFence.fds.at(0).set(req->buffers[i].acquireFence);

            native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

            handle->data[0] = req->buffers[i].acquireFence;

            result.outputBuffers[i].releaseFence = android::dupToAidl(handle);

            native_handle_delete(handle);

        }

    }
@@ -1813,16 +1817,20 @@ Status ExternalCameraDeviceSession::processCaptureResult(std::shared_ptr<HalRequ 
        if (req->buffers[i].fenceTimeout) {

            result.outputBuffers[i].status = BufferStatus::ERROR;

            if (req->buffers[i].acquireFence >= 0) {

                result.outputBuffers[i].releaseFence.fds.resize(1);

                result.outputBuffers[i].releaseFence.fds.at(0).set(req->buffers[i].acquireFence);

                native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

                handle->data[0] = req->buffers[i].acquireFence;

                result.outputBuffers[i].releaseFence = android::dupToAidl(handle);

                native_handle_delete(handle);

            }

            notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);

        } else {

            result.outputBuffers[i].status = BufferStatus::OK;

            // TODO: refactor

            if (req->buffers[i].acquireFence >= 0) {

                result.outputBuffers[i].releaseFence.fds.resize(1);

                result.outputBuffers[i].releaseFence.fds.at(0).set(req->buffers[i].acquireFence);

                native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

                handle->data[0] = req->buffers[i].acquireFence;

                result.outputBuffers[i].releaseFence = android::dupToAidl(handle);

                native_handle_delete(handle);

            }

        }

    }

camera/device/default/ExternalCameraOfflineSession.cpp

+3 −3
Original line number Diff line number Diff line
@@ -110,7 +110,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptr<HalReq 
            if (req->buffers[i].acquireFence >= 0) {

                native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

                handle->data[0] = req->buffers[i].acquireFence;

                result.outputBuffers[i].releaseFence = android::makeToAidl(handle);

                result.outputBuffers[i].releaseFence = android::dupToAidl(handle);

            }

            notifyError(req->frameNumber, req->buffers[i].streamId, ErrorCode::ERROR_BUFFER);

        } else {
@@ -119,7 +119,7 @@ Status ExternalCameraOfflineSession::processCaptureResult(std::shared_ptr<HalReq 
            if (req->buffers[i].acquireFence >= 0) {

                native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

                handle->data[0] = req->buffers[i].acquireFence;

                outputBuffer.releaseFence = android::makeToAidl(handle);

                outputBuffer.releaseFence = android::dupToAidl(handle);

            }

        }

    }
@@ -247,7 +247,7 @@ Status ExternalCameraOfflineSession::processCaptureRequestError( 
        if (req->buffers[i].acquireFence >= 0) {

            native_handle_t* handle = native_handle_create(/*numFds*/ 1, /*numInts*/ 0);

            handle->data[0] = req->buffers[i].acquireFence;

            outputBuffer.releaseFence = makeToAidl(handle);

            outputBuffer.releaseFence = dupToAidl(handle);

        }

    }

camera/device/default/ExternalCameraUtils.cpp

+5 −11
Original line number Diff line number Diff line
@@ -750,18 +750,12 @@ Size getMaxThumbnailResolution(const common::V1_0::helper::CameraMetadata& chars 


void freeReleaseFences(std::vector<CaptureResult>& results) {

    for (auto& result : results) {

        native_handle_t* inputReleaseFence =

                ::android::makeFromAidl(result.inputBuffer.releaseFence);

        if (inputReleaseFence != nullptr) {

            native_handle_close(inputReleaseFence);

            native_handle_delete(inputReleaseFence);

        }

        // NativeHandles free fd's on desctruction. Simply delete the objects!

        result.inputBuffer.releaseFence.fds.clear();  // Implicitly closes fds

        result.inputBuffer.releaseFence.ints.clear();

        for (auto& buf : result.outputBuffers) {

            native_handle_t* outReleaseFence = ::android::makeFromAidl(buf.releaseFence);

            if (outReleaseFence != nullptr) {

                native_handle_close(outReleaseFence);

                native_handle_delete(outReleaseFence);

            }

            buf.releaseFence.fds.clear();  // Implicitly closes fds

            buf.releaseFence.ints.clear();

        }

    }

}