Camera NDK: fix bug in lock order

AImage::close() can be called by AImageReader while holding
AImageReader::mLock.
Also fix ACaptureFailure double free issue.

Test: AR test app + CTS stress
Bug: 68885255
Change-Id: I17037e3e30e0f53b35ca538a3f321693c539cbdc
Merged-In: I17037e3e30e0f53b35ca538a3f321693c539cbdc
gugelfrei
Yin-Chia Yeh 7 years ago
parent f6c6cc0d1d
commit a10ab5bfd1

@ -941,7 +941,6 @@ void CameraDevice::CallbackHandler::onMessageReceived(
ACaptureRequest* request = allocateACaptureRequest(requestSp); ACaptureRequest* request = allocateACaptureRequest(requestSp);
(*onFail)(context, session.get(), request, failure); (*onFail)(context, session.get(), request, failure);
freeACaptureRequest(request); freeACaptureRequest(request);
delete failure;
break; break;
} }
case kWhatCaptureSeqEnd: case kWhatCaptureSeqEnd:

@ -53,7 +53,6 @@ AImage::isClosed() const {
void void
AImage::close(int releaseFenceFd) { AImage::close(int releaseFenceFd) {
lockReader();
Mutex::Autolock _l(mLock); Mutex::Autolock _l(mLock);
if (mIsClosed) { if (mIsClosed) {
return; return;
@ -71,7 +70,6 @@ AImage::close(int releaseFenceFd) {
mBuffer = nullptr; mBuffer = nullptr;
mLockedBuffer = nullptr; mLockedBuffer = nullptr;
mIsClosed = true; mIsClosed = true;
unlockReader();
} }
void void
@ -622,7 +620,9 @@ EXPORT
void AImage_deleteAsync(AImage* image, int releaseFenceFd) { void AImage_deleteAsync(AImage* image, int releaseFenceFd) {
ALOGV("%s", __FUNCTION__); ALOGV("%s", __FUNCTION__);
if (image != nullptr) { if (image != nullptr) {
image->lockReader();
image->close(releaseFenceFd); image->close(releaseFenceFd);
image->unlockReader();
if (!image->isClosed()) { if (!image->isClosed()) {
LOG_ALWAYS_FATAL("Image close failed!"); LOG_ALWAYS_FATAL("Image close failed!");
} }

@ -349,8 +349,8 @@ AImageReader::~AImageReader() {
for (auto it = mAcquiredImages.begin(); for (auto it = mAcquiredImages.begin();
it != mAcquiredImages.end(); it++) { it != mAcquiredImages.end(); it++) {
AImage* image = *it; AImage* image = *it;
Mutex::Autolock _l(image->mLock);
releaseImageLocked(image, /*releaseFenceFd*/-1); releaseImageLocked(image, /*releaseFenceFd*/-1);
image->close();
} }
// Delete Buffer Items // Delete Buffer Items
@ -502,6 +502,8 @@ AImageReader::releaseImageLocked(AImage* image, int releaseFenceFd) {
mBufferItemConsumer->releaseBuffer(*buffer, bufferFence); mBufferItemConsumer->releaseBuffer(*buffer, bufferFence);
returnBufferItemLocked(buffer); returnBufferItemLocked(buffer);
image->mBuffer = nullptr; image->mBuffer = nullptr;
image->mLockedBuffer = nullptr;
image->mIsClosed = true;
bool found = false; bool found = false;
// cleanup acquired image list // cleanup acquired image list

@ -85,7 +85,7 @@ struct AImageReader : public RefBase {
// Called by AImageReader_acquireXXX to acquire a Buffer and setup AImage. // Called by AImageReader_acquireXXX to acquire a Buffer and setup AImage.
media_status_t acquireImageLocked(/*out*/AImage** image, /*out*/int* fenceFd); media_status_t acquireImageLocked(/*out*/AImage** image, /*out*/int* fenceFd);
// Called by AImage to close image // Called by AImage/~AImageReader to close image. Caller is responsible to grab AImage::mLock
void releaseImageLocked(AImage* image, int releaseFenceFd); void releaseImageLocked(AImage* image, int releaseFenceFd);
static int getBufferWidth(BufferItem* buffer); static int getBufferWidth(BufferItem* buffer);

Loading…
Cancel
Save