Fix potential double close in IMediaMetadataRetriever::setDataSource
IMediaMetadataRetriever::setDataSource(fd, offset, length) takes the ownership
of |fd| on the direct invocation, and doesn't take the ownership on invocation
from Binder. This is inconsintent to other similar methods like
IMediaPlayer::setDataSource, and causes potential double close of |fd|.
This CL changes the caller and implementations to leave the ownership to make
them consistent.
Also, fixes a double close in IMediaPlayerService::setDataSource in an error
case.
Change-Id: Id551a1e725c4392b0fe6b7293871212eb101c0a5
diff --git a/media/libmedia/IMediaMetadataRetriever.cpp b/media/libmedia/IMediaMetadataRetriever.cpp
index aa2665a..cc52540 100644
--- a/media/libmedia/IMediaMetadataRetriever.cpp
+++ b/media/libmedia/IMediaMetadataRetriever.cpp
@@ -229,7 +229,7 @@
} break;
case SET_DATA_SOURCE_FD: {
CHECK_INTERFACE(IMediaMetadataRetriever, data, reply);
- int fd = dup(data.readFileDescriptor());
+ int fd = data.readFileDescriptor();
int64_t offset = data.readInt64();
int64_t length = data.readInt64();
reply->writeInt32(setDataSource(fd, offset, length));
diff --git a/media/libmediaplayerservice/MediaPlayerService.cpp b/media/libmediaplayerservice/MediaPlayerService.cpp
index 694f1a4..46b8b18 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -748,7 +748,6 @@
if (offset >= sb.st_size) {
ALOGE("offset error");
- ::close(fd);
return UNKNOWN_ERROR;
}
if (offset + length > sb.st_size) {
diff --git a/media/libmediaplayerservice/MetadataRetrieverClient.cpp b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
index 715cc0c..98e6974 100644
--- a/media/libmediaplayerservice/MetadataRetrieverClient.cpp
+++ b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
@@ -148,7 +148,6 @@
if (offset >= sb.st_size) {
ALOGE("offset (%lld) bigger than file size (%llu)", offset, sb.st_size);
- ::close(fd);
return BAD_VALUE;
}
if (offset + length > sb.st_size) {
@@ -164,12 +163,10 @@
ALOGV("player type = %d", playerType);
sp<MediaMetadataRetrieverBase> p = createRetriever(playerType);
if (p == NULL) {
- ::close(fd);
return NO_INIT;
}
status_t status = p->setDataSource(fd, offset, length);
if (status == NO_ERROR) mRetriever = p;
- ::close(fd);
return status;
}
diff --git a/media/libmediaplayerservice/StagefrightPlayer.cpp b/media/libmediaplayerservice/StagefrightPlayer.cpp
index b37aee3..51dbf83 100644
--- a/media/libmediaplayerservice/StagefrightPlayer.cpp
+++ b/media/libmediaplayerservice/StagefrightPlayer.cpp
@@ -64,7 +64,7 @@
// the method returns, if you want to keep it, dup it!
status_t StagefrightPlayer::setDataSource(int fd, int64_t offset, int64_t length) {
ALOGV("setDataSource(%d, %lld, %lld)", fd, offset, length);
- return mPlayer->setDataSource(dup(fd), offset, length);
+ return mPlayer->setDataSource(fd, offset, length);
}
status_t StagefrightPlayer::setDataSource(const sp<IStreamSource> &source) {
diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp
index 87eef1e..10c2e59 100644
--- a/media/libstagefright/AwesomePlayer.cpp
+++ b/media/libstagefright/AwesomePlayer.cpp
@@ -341,6 +341,7 @@
reset_l();
+ fd = dup(fd);
sp<DataSource> dataSource = new FileSource(fd, offset, length);
status_t err = dataSource->initCheck();