summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKeith Mok <keithmok@google.com>2023-08-31 00:31:35 +0000
committerAndroid Build Coastguard Worker <android-build-coastguard-worker@google.com>2023-12-18 10:54:41 +0000
commitb03923cf962739c53f96eac551cb9c6d20d897c0 (patch)
treeb0ebbebdbfade71b93d369791d58cf21a9847838
parentb92fc9a6a12c27ae83c95ff94f0a32b5e9401d63 (diff)
downloadcore-b03923cf962739c53f96eac551cb9c6d20d897c0.tar.gz
Add seal if ashmem-dev is backed by memfd
Need to seal the buffer size in align with ashmem if set to PROT_READ only to prevent untrusted remote process to shrink the buffer size and crash it. Bug: 294609150 Test: build Ignore-AOSP-First: Security (cherry picked from commit f83c5c8fecf89d9315945368aa20350c2f235cc0) (cherry picked from https://googleplex-android-review.googlesource.com/q/commit:61a2897733e15a12b7aa2dfd99957e83cbe59351) Merged-In: I9288cf30b41e84ad8d3247c204e20482912bff69 Change-Id: I9288cf30b41e84ad8d3247c204e20482912bff69
-rw-r--r--libcutils/ashmem-dev.cpp29
1 files changed, 25 insertions, 4 deletions
diff --git a/libcutils/ashmem-dev.cpp b/libcutils/ashmem-dev.cpp
index 6a27f9a20..56d68759f 100644
--- a/libcutils/ashmem-dev.cpp
+++ b/libcutils/ashmem-dev.cpp
@@ -349,6 +349,12 @@ static int memfd_create_region(const char* name, size_t size) {
return -1;
}
+ // forbid size changes to match ashmem behaviour
+ if (fcntl(fd, F_ADD_SEALS, F_SEAL_GROW | F_SEAL_SHRINK) == -1) {
+ ALOGE("memfd_create(%s, %zd) F_ADD_SEALS failed: %m", name, size);
+ return -1;
+ }
+
if (debug_log) {
ALOGE("memfd_create(%s, %zd) success. fd=%d\n", name, size, fd.get());
}
@@ -400,14 +406,29 @@ error:
}
static int memfd_set_prot_region(int fd, int prot) {
- /* Only proceed if an fd needs to be write-protected */
+ int seals = fcntl(fd, F_GET_SEALS);
+ if (seals == -1) {
+ ALOGE("memfd_set_prot_region(%d, %d): F_GET_SEALS failed: %s\n", fd, prot, strerror(errno));
+ return -1;
+ }
+
if (prot & PROT_WRITE) {
+ /* Now we want the buffer to be read-write, let's check if the buffer
+ * has been previously marked as read-only before, if so return error
+ */
+ if (seals & F_SEAL_FUTURE_WRITE) {
+ ALOGE("memfd_set_prot_region(%d, %d): region is write protected\n", fd, prot);
+ errno = EINVAL; // inline with ashmem error code, if already in
+ // read-only mode
+ return -1;
+ }
return 0;
}
- if (fcntl(fd, F_ADD_SEALS, F_SEAL_FUTURE_WRITE) == -1) {
- ALOGE("memfd_set_prot_region(%d, %d): F_SEAL_FUTURE_WRITE seal failed: %s\n", fd, prot,
- strerror(errno));
+ /* We would only allow read-only for any future file operations */
+ if (fcntl(fd, F_ADD_SEALS, F_SEAL_FUTURE_WRITE | F_SEAL_SEAL) == -1) {
+ ALOGE("memfd_set_prot_region(%d, %d): F_SEAL_FUTURE_WRITE | F_SEAL_SEAL seal failed: %s\n",
+ fd, prot, strerror(errno));
return -1;
}