summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorTaiju Tsuiki <tzik@google.com>2015-04-21 17:36:22 +0900
committerSteve Kondik <steve@cyngn.com>2015-12-18 17:30:09 -0500
commitf36321997a15edce6ac88414c22efd07da9eb8dc (patch)
treec9a1fd89c4926aa4b1bfadf399418307a280c9dd
parentab9b1e936c2da51d138b3989e90c774ef9bdef7f (diff)
downloadframeworks_av-f36321997a15edce6ac88414c22efd07da9eb8dc.zip
frameworks_av-f36321997a15edce6ac88414c22efd07da9eb8dc.tar.gz
frameworks_av-f36321997a15edce6ac88414c22efd07da9eb8dc.tar.bz2
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
-rw-r--r--media/libmedia/IMediaMetadataRetriever.cpp2
-rw-r--r--media/libmediaplayerservice/MediaPlayerService.cpp1
-rw-r--r--media/libmediaplayerservice/MetadataRetrieverClient.cpp3
-rw-r--r--media/libstagefright/AwesomePlayer.cpp1
4 files changed, 2 insertions, 5 deletions
diff --git a/media/libmedia/IMediaMetadataRetriever.cpp b/media/libmedia/IMediaMetadataRetriever.cpp
index 9765f0d..dbf524e 100644
--- a/media/libmedia/IMediaMetadataRetriever.cpp
+++ b/media/libmedia/IMediaMetadataRetriever.cpp
@@ -240,7 +240,7 @@ status_t BnMediaMetadataRetriever::onTransact(
} 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 de51b3c..da5bb9a 100644
--- a/media/libmediaplayerservice/MediaPlayerService.cpp
+++ b/media/libmediaplayerservice/MediaPlayerService.cpp
@@ -749,7 +749,6 @@ status_t MediaPlayerService::Client::setDataSource(int fd, int64_t offset, int64
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 894a855..7bd99cc 100644
--- a/media/libmediaplayerservice/MetadataRetrieverClient.cpp
+++ b/media/libmediaplayerservice/MetadataRetrieverClient.cpp
@@ -149,7 +149,6 @@ status_t MetadataRetrieverClient::setDataSource(int fd, int64_t offset, int64_t
if (offset >= sb.st_size) {
ALOGE("offset (%" PRId64 ") bigger than file size (%" PRIu64 ")", offset, sb.st_size);
- ::close(fd);
return BAD_VALUE;
}
if (offset + length > sb.st_size) {
@@ -165,12 +164,10 @@ status_t MetadataRetrieverClient::setDataSource(int fd, int64_t offset, int64_t
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/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp
index 5a43caf..933f241 100644
--- a/media/libstagefright/AwesomePlayer.cpp
+++ b/media/libstagefright/AwesomePlayer.cpp
@@ -344,6 +344,7 @@ status_t AwesomePlayer::setDataSource(
reset_l();
+ fd = dup(fd);
sp<DataSource> dataSource = new FileSource(fd, offset, length);
status_t err = dataSource->initCheck();