Bug 1479960 - Make AutoMemMap not fstat() the mapped object if it doesn't need to. r=kmag

One problem with using shared memory instead of files for MemMapSnapshot
is that AutoMemMap was trying to use fstat() to obtain the object size;
that doesn't work with ashmem on Android and was causing problems with
the Mac sandbox, but it's not necessary, because we already know the
size.  This patch changes it to not do that.

Depends on D26743

Differential Revision: https://phabricator.services.mozilla.com/D26744

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Jed Davis 2019-08-14 22:48:36 +00:00
parent 0c5b23b47d
commit a2d06f8ed8
2 changed files with 22 additions and 19 deletions

View File

@ -39,8 +39,7 @@ Result<Ok, nsresult> AutoMemMap::init(nsIFile* file, int flags, int mode,
} }
Result<Ok, nsresult> AutoMemMap::init(const FileDescriptor& file, Result<Ok, nsresult> AutoMemMap::init(const FileDescriptor& file,
PRFileMapProtect prot, PRFileMapProtect prot, size_t maybeSize) {
size_t expectedSize) {
MOZ_ASSERT(!fd); MOZ_ASSERT(!fd);
if (!file.IsValid()) { if (!file.IsValid()) {
return Err(NS_ERROR_INVALID_ARG); return Err(NS_ERROR_INVALID_ARG);
@ -54,19 +53,29 @@ Result<Ok, nsresult> AutoMemMap::init(const FileDescriptor& file,
} }
Unused << handle.release(); Unused << handle.release();
return initInternal(prot, expectedSize); return initInternal(prot, maybeSize);
} }
Result<Ok, nsresult> AutoMemMap::initInternal(PRFileMapProtect prot, Result<Ok, nsresult> AutoMemMap::initInternal(PRFileMapProtect prot,
size_t expectedSize) { size_t maybeSize) {
MOZ_ASSERT(!fileMap); MOZ_ASSERT(!fileMap);
MOZ_ASSERT(!addr); MOZ_ASSERT(!addr);
PRFileInfo64 fileInfo; if (maybeSize > 0) {
MOZ_TRY(PR_GetOpenFileInfo64(fd.get(), &fileInfo)); // Some OSes' shared memory objects can't be stat()ed, either at
// all (Android) or without loosening the sandbox (Mac) so just
// use the size.
size_ = maybeSize;
} else {
// But if we don't have the size, assume it's a regular file and
// ask for it.
PRFileInfo64 fileInfo;
MOZ_TRY(PR_GetOpenFileInfo64(fd.get(), &fileInfo));
if (fileInfo.size > UINT32_MAX) { if (fileInfo.size > UINT32_MAX) {
return Err(NS_ERROR_INVALID_ARG); return Err(NS_ERROR_INVALID_ARG);
}
size_ = fileInfo.size;
} }
fileMap = PR_CreateFileMap(fd, 0, prot); fileMap = PR_CreateFileMap(fd, 0, prot);
@ -74,13 +83,6 @@ Result<Ok, nsresult> AutoMemMap::initInternal(PRFileMapProtect prot,
return Err(NS_ERROR_FAILURE); return Err(NS_ERROR_FAILURE);
} }
size_ = fileInfo.size;
// The memory region size passed in certain IPC messages isn't necessary on
// Unix-like systems, since we can always stat the file descriptor to
// determine it accurately. But since we have it, anyway, sanity check that
// it matches the size returned by the stat.
MOZ_ASSERT_IF(expectedSize > 0, size_ == expectedSize);
addr = PR_MemMap(fileMap, 0, size_); addr = PR_MemMap(fileMap, 0, size_);
if (!addr) { if (!addr) {
return Err(NS_ERROR_FAILURE); return Err(NS_ERROR_FAILURE);
@ -122,7 +124,8 @@ FileDescriptor AutoMemMap::cloneHandle() const {
Result<Ok, nsresult> AutoMemMap::initWithHandle(const FileDescriptor& file, Result<Ok, nsresult> AutoMemMap::initWithHandle(const FileDescriptor& file,
size_t size, size_t size,
PRFileMapProtect prot) { PRFileMapProtect prot) {
return init(file, prot); MOZ_DIAGNOSTIC_ASSERT(size > 0);
return init(file, prot, size);
} }
FileDescriptor AutoMemMap::cloneHandle() const { return cloneFileDescriptor(); } FileDescriptor AutoMemMap::cloneHandle() const { return cloneFileDescriptor(); }

View File

@ -36,7 +36,7 @@ class AutoMemMap {
Result<Ok, nsresult> init(const FileDescriptor& file, Result<Ok, nsresult> init(const FileDescriptor& file,
PRFileMapProtect prot = PR_PROT_READONLY, PRFileMapProtect prot = PR_PROT_READONLY,
size_t expectedSize = 0); size_t maybeSize = 0);
// Initializes the mapped memory with a shared memory handle. On // Initializes the mapped memory with a shared memory handle. On
// Unix-like systems, this is identical to the above init() method. On // Unix-like systems, this is identical to the above init() method. On
@ -73,8 +73,8 @@ class AutoMemMap {
void setPersistent() { persistent_ = true; } void setPersistent() { persistent_ = true; }
private: private:
Result<Ok, nsresult> initInternal(PRFileMapProtect prot = PR_PROT_READONLY, Result<Ok, nsresult> initInternal(PRFileMapProtect prot,
size_t expectedSize = 0); size_t maybeSize = 0);
AutoFDClose fd; AutoFDClose fd;
PRFileMap* fileMap = nullptr; PRFileMap* fileMap = nullptr;