From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Keith Mok Date: Thu, 31 Aug 2023 00:31:35 +0000 Subject: [PATCH] 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 --- libcutils/ashmem-dev.cpp | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/libcutils/ashmem-dev.cpp b/libcutils/ashmem-dev.cpp index e67b45808..a081837e5 100644 --- a/libcutils/ashmem-dev.cpp +++ b/libcutils/ashmem-dev.cpp @@ -360,6 +360,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()); } @@ -411,14 +417,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; }