From b048c33d5bdaec747195dfedf971d4d9155f5000 Mon Sep 17 00:00:00 2001 From: Christopher Tate Date: Fri, 21 Feb 2014 12:50:21 -0800 Subject: [PATCH] Don't assume that we're at start of file at ctor time BackupDataReader / BackupDataWriter were implicitly assuming that when instantiated, the underlying fd was positioned at start-of-file. If one tried to e.g. open an existing data stream to append further data to it, things might randomly fail (at read time, possibly when consuming the stream later) due to incorrect alignment of the data entities: the appending writer would assume that no padding was needed to achieve correct alignment, and this might easily be false. Now the underlying native reader/writer helpers recognize the true position within the file when constructed, and as a result it's now safe to e.g. construct a BackupDataOutput for an existing file and then append to it. Change-Id: If0484921e687852f923a4b4efabff573a6c16981 --- .../internal/backup/LocalTransport.java | 27 +++++++++++++++++-- libs/androidfw/BackupData.cpp | 6 +++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/core/java/com/android/internal/backup/LocalTransport.java b/core/java/com/android/internal/backup/LocalTransport.java index a3323e94a17b2..2fe2494bb347e 100644 --- a/core/java/com/android/internal/backup/LocalTransport.java +++ b/core/java/com/android/internal/backup/LocalTransport.java @@ -35,6 +35,11 @@ import java.io.FileInputStream; import java.io.FileOutputStream; import java.io.IOException; +import libcore.io.ErrnoException; +import libcore.io.Libcore; +import libcore.io.StructStat; +import static libcore.io.OsConstants.*; + /** * Backup transport for stashing stuff into a known location on disk, and * later restoring from there. For testing only. @@ -96,7 +101,16 @@ public class LocalTransport extends IBackupTransport.Stub { } public int performBackup(PackageInfo packageInfo, ParcelFileDescriptor data) { - if (DEBUG) Log.v(TAG, "performBackup() pkg=" + packageInfo.packageName); + if (DEBUG) { + try { + StructStat ss = Libcore.os.fstat(data.getFileDescriptor()); + Log.v(TAG, "performBackup() pkg=" + packageInfo.packageName + + " size=" + ss.st_size); + } catch (ErrnoException e) { + Log.w(TAG, "Unable to stat input file in performBackup() on " + + packageInfo.packageName); + } + } File packageDir = new File(mDataDir, packageInfo.packageName); packageDir.mkdirs(); @@ -130,7 +144,16 @@ public class LocalTransport extends IBackupTransport.Stub { buf = new byte[bufSize]; } changeSet.readEntityData(buf, 0, dataSize); - if (DEBUG) Log.v(TAG, " data size " + dataSize); + if (DEBUG) { + try { + long cur = Libcore.os.lseek(data.getFileDescriptor(), 0, SEEK_CUR); + Log.v(TAG, " read entity data; new pos=" + cur); + } + catch (ErrnoException e) { + Log.w(TAG, "Unable to stat input file in performBackup() on " + + packageInfo.packageName); + } + } try { entity.write(buf, 0, dataSize); diff --git a/libs/androidfw/BackupData.cpp b/libs/androidfw/BackupData.cpp index 4e3b522011106..1a5c55ce8edaa 100644 --- a/libs/androidfw/BackupData.cpp +++ b/libs/androidfw/BackupData.cpp @@ -59,9 +59,10 @@ padding_extra(size_t n) BackupDataWriter::BackupDataWriter(int fd) :m_fd(fd), m_status(NO_ERROR), - m_pos(0), m_entityCount(0) { + m_pos = (ssize_t) lseek(fd, 0, SEEK_CUR); + if (DEBUG) ALOGI("BackupDataWriter(%d) @ %ld", fd, (long)m_pos); } BackupDataWriter::~BackupDataWriter() @@ -184,10 +185,11 @@ BackupDataReader::BackupDataReader(int fd) :m_fd(fd), m_done(false), m_status(NO_ERROR), - m_pos(0), m_entityCount(0) { memset(&m_header, 0, sizeof(m_header)); + m_pos = (ssize_t) lseek(fd, 0, SEEK_CUR); + if (DEBUG) ALOGI("BackupDataReader(%d) @ %ld", fd, (long)m_pos); } BackupDataReader::~BackupDataReader()