__strlen_aarch64_mte frequently triggered problem with MTE [298530285]

Infeasible

Bug

[AOSP] assigned

Status Update

No update yet.

Description

h8...@gmail.com

created issue #1

Sep 1, 2023 02:52AM

To avoid the possibility of sharing private information, please share bugreports and screenshots from Google Drive. Share files with android-bugreport@google.com and include only Google drive links in your bug. If attaching bug reports or other sensitive data directly to issues, please mark the attachment(s) as “Restricted (https://developers.google.com/issue-tracker/concepts/restricted-content)” when creating a bug or adding a comment. Restricting or deleting your comment or attachment can also be done using the overflow menu after submitting.

Disclaimer: Please note, by submitting this bug report, you acknowledge that Google may use information included in the bug report to diagnose technical issues and to improve our products and services, in accordance with our Privacy Policy (https://policies.google.com/privacy), and you agree that we may share it with engineering partners potentially impacted by your issue.

Description of issue (what needs changing):
From our practical experiences, so many Android Applications could trigger the MTE syncerrors and bring collapses only if we open the MTE, since the Android standard libc would use the MTE version strlen by default. The JNI interfaces which are used to pass strings bewteen native and JVM with env->GetStringUTFChars or env->ReleaseStringUTFChars could aggravate that.

#00 pc 00000000000937cc /apex/com.android.runtime/lib64/bionic/libc.so
(__strlen_aarch64_mte+12)

The developers' coding style or the compiler optimizations could make the string objects free ahead and of course, it is theoretically UAF (use after free). Something like the case below, in which the string object would be free because of some Return Value Optimization techniques of C++.

vector.push_back(toCppString(**).c_str())

I could understand the implementation consideration for this MTE version strlen. It is needed by completeness of theory and the MTE team may request the coverage of all dereerences from heap opeartions.

But I still have no idea whether it is necessary to bring MTE to the strlen? I think the stirng object is greatly management inherently and there is no need to operate memory explicitly. Although there are definetely certain UAF or other possible memory safety problems like OOB of string object caused by strlen, the real world impact maybe limited, especially with the Scudo's memory safety mitigations. Of course, this is just a sensory intuition and a concrete and rational analysis is essential.

So, is it possible to take some remedial actions or just disable the MTE strlen for better and more practical usage of MTE Android please?

Comments

ra...@google.com <ra...@google.com> #2Sep 1, 2023 11:05AM

Assigned to ra...@google.com.

We have shared this with our product and engineering team and will update this issue with more information as it becomes available.

vi...@google.com <vi...@google.com> #3Oct 4, 2023 04:17PM

Thanks again for the feedback! Our product and engineering teams have evaluated the request and responded:

Hello,

An MTE-specific strlen (and all string.h family of functions) is necessary and desirable to detect buffer-overflow bugs.

The example you mention, vector.push_back(toCppString(**).c_str()), is a great example of the type of footgun that exists in C++. Our other tools even specifically mention this as an example of a use-after-free bug that we want to catch.

Some of those specific examples of use-after-free bugs are begnign, however they're still undefined behaviour. MTE catching them is the desired functionality.

we...@vivo.com <we...@vivo.com> #4Dec 21, 2023 08:35AM

Thanks for your reply. I can understand that and I found a new case that the __memchr_aarch64_mte called by strstr could give a MTE error if the input pointer has not been aligned by 16 bytes. I think it is a bug,right? test case:

    char *data = (char*)calloc(128,1);
    memset(data, 'A', 128);
    char *str2 = "BBbbbaa";
    char *res = strstr(data+2, str2);

we...@vivo.com <we...@vivo.com> #5Dec 22, 2023 02:11AM

The backtrace for the test case:

signal 11 (SIGSEGV), code 9 (SEGV_MTESERR), fault addr 0x0200007b9b4e67e0
Cause: [MTE]: Buffer Overflow, 0 bytes right of a 128-byte allocation at 0x7b9b4e6760
***
backtrace:
     #00 pc 0000000000092fe0  /apex/com.android.runtime/lib64/bionic/libc.so (__strchr_aarch64_mte+96) (BuildId: cfc293be733954571ce0dc79a9917039)
     #01 pc 00000000000e8f88  /apex/com.android.runtime/lib64/bionic/libc.so (strstr+24) (BuildId: cfc293be733954571ce0dc79a9917039)
     #02 pc 00000000000017d8  /data/local/tmp/a.out (main+116)

Oh,sorry that the case I give above is for the __strchr_aarch64_mte, and the case for __memchr_aarch64_mte is below.

   char *data = (char*)calloc(128,1);
    memset(data, 'A', 128);
    char *str2 = "ABbbbAa";
    char *res = strstr(data+2, str2);

and the backtrace of the __memchr_aarch64_mte case:

signal 11 (SIGSEGV), code 9 (SEGV_MTESERR), fault addr 0x0f000074794803b0
Cause: [MTE]: Buffer Overflow, 0 bytes right of a 128-byte allocation at 0x7479480330
***
backtrace:
      #00 pc 000000000009268c  /apex/com.android.runtime/lib64/bionic/libc.so (__memchr_aarch64_mte+12) (BuildId: cfc293be733954571ce0dc79a9917039)
      #01 pc 00000000000e92c0  /apex/com.android.runtime/lib64/bionic/libc.so (twoway_strstr+464) (BuildId: cfc293be733954571ce0dc79a9917039)

I think the __strchr_aarch64_mte and __memchr_aarch64_mte has the same problem that they preread 16 bytes and if there are two MTE tag in a single bytes chunk, a MTE exception would raise. And that is caused without any wrong operations by developer.

h8...@gmail.com <h8...@gmail.com> #6Dec 22, 2023 02:17AM

Sorry and I give another issue for that. 317403451

ra...@google.com <ra...@google.com> #7Dec 22, 2023 04:27AM

Status: Won't Fix (Infeasible)

Thanks for information. Closing this issue as you have raised new bug.
For further update please see

bug 317403451

Issue 298530285