From c2dd82bfd6d7aea5b6760efc0712ae11d7a52e6b Mon Sep 17 00:00:00 2001 From: Marco Nelissen Date: Tue, 20 Sep 2016 13:36:40 -0700 Subject: Check mprotect result mprotect can theoretically fail, which could then let one exploit a vulnerable codec if one exists on the device. Bug: 31350239 Change-Id: I7b99c190619f0fb2eb93119596e6da0d2deb8ba5 (cherry picked from commit 866c800c0624bb13eee44973cc8a2ecd0012de6e) --- media/libmedia/IOMX.cpp | 52 ++++++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/media/libmedia/IOMX.cpp b/media/libmedia/IOMX.cpp index c28eac8..365d9ac 100644 --- a/media/libmedia/IOMX.cpp +++ b/media/libmedia/IOMX.cpp @@ -733,31 +733,35 @@ status_t BnOMX::onTransact( // mark the last page as inaccessible, to avoid exploitation // of codecs that access past the end of the allocation because // they didn't check the size - mprotect((char*)params + allocSize - pageSize, pageSize, PROT_NONE); - switch (code) { - case GET_PARAMETER: - err = getParameter(node, index, params, size); - break; - case SET_PARAMETER: - err = setParameter(node, index, params, size); - break; - case GET_CONFIG: - err = getConfig(node, index, params, size); - break; - case SET_CONFIG: - err = setConfig(node, index, params, size); - break; - case SET_INTERNAL_OPTION: - { - InternalOptionType type = - (InternalOptionType)data.readInt32(); - - err = setInternalOption(node, index, type, params, size); - break; + if (mprotect((char*)params + allocSize - pageSize, pageSize, + PROT_NONE) != 0) { + ALOGE("mprotect failed: %s", strerror(errno)); + } else { + switch (code) { + case GET_PARAMETER: + err = getParameter(node, index, params, size); + break; + case SET_PARAMETER: + err = setParameter(node, index, params, size); + break; + case GET_CONFIG: + err = getConfig(node, index, params, size); + break; + case SET_CONFIG: + err = setConfig(node, index, params, size); + break; + case SET_INTERNAL_OPTION: + { + InternalOptionType type = + (InternalOptionType)data.readInt32(); + + err = setInternalOption(node, index, type, params, size); + break; + } + + default: + TRESPASS(); } - - default: - TRESPASS(); } } } -- cgit v1.1