From b1911401e0008c2b12a3fe6c9dc34528bf7710a5 Mon Sep 17 00:00:00 2001 From: Adam Lesinski <adamlesinski@google.com> Date: Wed, 9 Mar 2016 17:13:09 -0800 Subject: [PATCH] libziparchive: ensure ReadAtOffset is atomic ag/880725 modified ReadAtOffset to seek then read from the open file descriptor. Previously pread64 was used to provide atomic behaviour. This causes races when multiple threads are trying to access data from the file. This is supported, so this change reverts the relevant parts of the above CL to restore the old behaviour. Bug:27563413 Change-Id: I7bffd78da8c558745dfc3c072ba9691b1b15bb5b --- include/ziparchive/zip_archive.h | 3 +++ libziparchive/zip_archive.cc | 10 ++++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/include/ziparchive/zip_archive.h b/include/ziparchive/zip_archive.h index 3591a6beba..7dc60aed20 100644 --- a/include/ziparchive/zip_archive.h +++ b/include/ziparchive/zip_archive.h @@ -152,6 +152,9 @@ void CloseArchive(ZipArchiveHandle handle); * if this file entry contains a data descriptor footer. To verify crc32s * and length, a call to VerifyCrcAndLengths must be made after entry data * has been processed. + * + * On non-Windows platforms this method does not modify internal state and + * can be called concurrently. */ int32_t FindEntry(const ZipArchiveHandle handle, const ZipString& entryName, ZipEntry* data); diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index a2d6fccd1d..1f2750047f 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -495,14 +495,20 @@ static int32_t UpdateEntryFromDataDescriptor(int fd, } // Attempts to read |len| bytes into |buf| at offset |off|. -// Callers should not rely on the |fd| offset being incremented -// as a side effect of this call. +// On non-Windows platforms, callers are guaranteed that the |fd| +// offset is unchanged and there is no side effect to this call. +// +// On Windows platforms this is not thread-safe. static inline bool ReadAtOffset(int fd, uint8_t* buf, size_t len, off64_t off) { +#if !defined(_WIN32) + return TEMP_FAILURE_RETRY(pread64(fd, buf, len, off)); +#else if (lseek64(fd, off, SEEK_SET) != off) { ALOGW("Zip: failed seek to offset %" PRId64, off); return false; } return android::base::ReadFully(fd, buf, len); +#endif } static int32_t FindEntry(const ZipArchive* archive, const int ent, -- GitLab