Chromium 117 and Control Flow Integrity (CFI) crashing on Android

[t-m-w]

@uazo, You probably already know that this is an issue in 117, but just in case, I wanted to let you know that we were encountering SIGTRAP crashes shortly after startup in our fork of 117 for CalyxOS (Android) that took us a while to track down and caused some confusion. When we figured out that the crashes were only happening with is_cfi=true, we briefly used use_cfi_diag=true to try to track down the crashes, but after it looked like a can of worms, we eventually we decided to just turn off CFI in our build for now.

Unfortunately I didn't document it extensively, but one of the issues was a failure to link with FastCheckoutAccessibilityService, another similar for AutofillKeyboardAccessoryAdapter, and I gave up at a cast to unsupported type issue with libunwind.

If you have already built Cromite 117 for Android and are not experiencing crashes or have any tips, we'd love to know!

Thanks as always for all your work on Cromite.

[uazo]

and are not experiencing crashes or have any tips

unfortunately I confirm the crash, I had not checked the arm64 version, thanks for the report.

When we figured out that the crashes were only happening with is_cfi=true

it has already happened.
To understand that, I had compared all the clang flags in the output of ninja between releases, hopefully it will be that simple again.

Unfortunately I didn't document it extensively, but one of the issues was a failure to link with FastCheckoutAccessibilityService, another similar for AutofillKeyboardAccessoryAdapter, and I gave up at a cast to unsupported type issue with libunwind.
but after it looked like a can of worms

never mind, in my naivety, when the same problem had occurred, I had thought of a security problem, which is why I took the trouble to verify it.
in addition to the cfi diagnostics, it had been necessary to activate the component build, I had lost a lot of time, and then realised that it was out of my hands, as well as of little interest to the google team (but I can understand them, who knows how many bother them).

we eventually we decided to just turn off CFI in our build for now.

Look, this is something I've been thinking about for a while.
may I ask a favour of you, imagining that you are much faster?
would you try dropping 00Remove-experimental-relative-c---abi-vtables.patch and activating use_relative_vtables_abi without CFI?
can be a viable alternative to CFI, obviously without support for shared libraries.

[t-m-w]

Wow, it sounds like you have put quite a lot of time into this already!

would you try dropping 00Remove-experimental-relative-c---abi-vtables.patch and activating use_relative_vtables_abi without CFI?

This appears to work fine, at least with the patches we are using in our 117.0.5938.60 build. We hadn't been using that patch, and when setting is_cfi=false, we did go ahead and set use_relative_vtables_abi=true, no crashes or anything noticed yet.

We can try with all the Cromite patches too, if that would help.

can be a viable alternative to CFI, obviously without support for shared libraries.

Not so obvious here, I only learned of these terms recently 🙃

[uazo]

this is the stack trace:

Build fingerprint: 'Android/sdk_phone64_x86_64/emulator64_x86_64:12/SE1A.220826.006.A1/10064458:userdebug/test-keys'
Revision: '0'
pid: 25626, tid: 25626, name: cromite.cromite  >>> org.cromite.cromite <<<
signal 4 (SIGILL), code 2 (ILL_ILLOPN), fault addr 0x75e72dff79b7

Stack Trace:
  RELADDR   FUNCTION                                                                          FILE:LINE
  0000000004f729b7  __unw_init_local                                                                  ../../third_party/libunwind/src/src/libunwind.cpp:0:0
  0000000004f7aad7  _Unwind_Backtrace                                                                 ../../third_party/libunwind/src/src/UnwindLevel1-gcc-ext.c:137:3
  00000000053aa16a  base::debug::CollectStackTrace(void**, unsigned long)                             ../../base/debug/stack_trace_android.cc:80:3
  v------>  base::debug::StackTrace::StackTrace(unsigned long)                                ../../base/debug/stack_trace.cc:221:12
  00000000053bb19d  base::debug::StackTrace::StackTrace()                                             ../../base/debug/stack_trace.cc:218:28
  00000000037754f2  content::RenderFrameHostImpl::RenderFrameHostImpl(content::SiteInstance*, scoped_refptr<content::RenderViewHostImpl>, content::RenderFrameHostDelegate*, content::FrameTree*, content::FrameTreeNode*, int, mojo::PendingAssociatedRemote<content::mojom::Frame>, base::TokenType<blink::LocalFrameTokenTypeMarker> const&, base::TokenType<blink::DocumentTokenTypeMarker> const&, base::UnguessableToken, bool, content::RenderFrameHostImpl::LifecycleStateImpl, scoped_refptr<content::BrowsingContextState>, blink::FrameOwnerElementType, content::RenderFrameHostImpl*, content::RenderFrameHostImpl::FencedFrameStatus)  ../../content/browser/renderer_host/render_frame_host_impl.cc:1505:22
  0000000003773dcd  content::RenderFrameHostFactory::Create(content::SiteInstance*, scoped_refptr<content::RenderViewHostImpl>, content::RenderFrameHostDelegate*, content::FrameTree*, content::FrameTreeNode*, int, mojo::PendingAssociatedRemote<content::mojom::Frame>, base::TokenType<blink::LocalFrameTokenTypeMarker> const&, base::TokenType<blink::DocumentTokenTypeMarker> const&, base::UnguessableToken, bool, content::RenderFrameHostImpl::LifecycleStateImpl, scoped_refptr<content::BrowsingContextState>)  ../../content/browser/renderer_host/render_frame_host_factory.cc:39:31
  00000000037c190d  content::RenderFrameHostManager::CreateRenderFrameHost(content::RenderFrameHostManager::CreateFrameCase, content::SiteInstanceImpl*, int, mojo::PendingAssociatedRemote<content::mojom::Frame>, base::TokenType<blink::LocalFrameTokenTypeMarker> const&, base::TokenType<blink::DocumentTokenTypeMarker> const&, base::UnguessableToken, bool, scoped_refptr<content::BrowsingContextState>)  ../../content/browser/renderer_host/render_frame_host_manager.cc:3568:10
  00000000037c146e  content::RenderFrameHostManager::InitRoot(content::SiteInstanceImpl*, bool, blink::FramePolicy, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, base::UnguessableToken const&)  ../../content/browser/renderer_host/render_frame_host_manager.cc:523:22
  00000000036b1389  content::FrameTree::Init(content::SiteInstanceImpl*, bool, std::__Cr::basic_string<char, std::__Cr::char_traits<char>, std::__Cr::allocator<char>> const&, content::RenderFrameHostImpl*, blink::FramePolicy const&, base::UnguessableToken const&)  ../../content/browser/renderer_host/frame_tree.cc:888:27
  00000000039388f5  content::WebContentsImpl::Init(content::WebContents::CreateParams const&, blink::FramePolicy)  ../../content/browser/web_contents/web_contents_impl.cc:3324:23
  000000000392bd2f  content::WebContentsImpl::CreateWithOpener(content::WebContents::CreateParams const&, content::RenderFrameHostImpl*)  ../../content/browser/web_contents/web_contents_impl.cc:1260:17
  v------>  JNI_WebContentsFactory_CreateWebContents(_JNIEnv*, base::android::JavaParamRef<_jobject*> const&, unsigned char, unsigned char, base::android::JavaParamRef<_jthrowable*> const&)  ../../chrome/browser/android/web_contents_factory.cc:32:10
  0

the crash is a ctor of RenderFrameHostImpl, absolutely no point in continuing the investigation.
I'll try to check the clang flags.

[t-m-w]

doesn't say much to me... any ideas?

Oh, interesting. I don't know anything about libunwind myself... I didn't even realize it was optional. It's for backtraces? I wonder what happens without it, then. (I do know that the backtrace info from CFI-related crashes was pretty useless without use_cfi_diag=true but that may be another story.)

[uazo]

I didn't even realize it was optional.

no, it is not optional.

It's for backtraces?

precisely

I wonder what happens without it, then.

The stack dump would probably not work, and I cannot afford it.

we decided to just turn off CFI in our build for now.

I did the same, tomorrow I will try to activate an experimental flag that uses android's native unwinding stack instead of libunwind.
but I have to decide whether to reactivate support for cfi in production for Android, considering that it is not officially supported.

[uazo]

I will try to activate an experimental flag that uses android's native unwinding stack instead of libunwind.

no difference, the crash continues.
I will try compiling chromium vanilla for android with cfi, in the hope that it will also crash.
otherwise, it's a big problem.

[t-m-w]

I should have mentioned earlier, yes, vanilla does crash on Android too the same way.

[uazo]

vanilla does crash on Android too the same way.

aahh, that's a relief, you don't know how happy that makes me!
:=)

[peternrdstrm]

Will this not be a huge negative impact on security?

[uazo]

huge... depends on your threat model, but a little something yes.
The problem is that reactivating the CFI requires skills that I do not have, and it is not directly supported by chromium for android, so I can't ask them either.

what I understood: CFI protects calls, that use vtables, which are, to put it simply, the pointing of methods of c++ objects. For relocation and position-independent-code to work, the vtable is read-write (at runtime linking time) and protection is by verification between the caller and the called.
Instead, the relative vtables is enabled, which supports relocation and PIC and does not need to have the vtable in read-write at linking time, in fact the part of memory in which the vtables are stored is read-only and therefore not modifiable and read directly from the elf.

what is effectively lost is the checking of derived casts (cast from base to derived to the wrong dynamic type) and unrelated casts (cast from void* or another unrelated type to the wrong dynamic type). in the code I have written is never present. in the chromium one I expect them to do the verification. I realise that I minimise the risk, but I have no alternative.

CFI cross-DSO support seems to be unstable.

But I'm not a security technician, I know what I read.

[ghost]

Does Vanadium not use CFI?

[thestinger]

Cromite isn't allowed to use changes from Vanadium anymore due to it using GPLv3 while Vanadium is GPLv2-only. We were forbidden from using their code by a licensing change to only using GPLv3 so we did the same thing in response. It predates Cromite. This thread was opened by someone who is part of an organization repeatedly engaging in extreme harassment towards me, spreading misinformation about GrapheneOS and plagiarizing work from us. We expect them to try to launder our work on CFI support as their own, as they have done in several other important cases. @uazo is well aware of these issues and if they want to work things out with us they know how to contact us.

[uazo]

Cromite isn't allowed to use changes from Vanadium anymore due to it using GPLv3 while Vanadium is GPLv2-only.

I am aware of it

We were forbidden from using their code by a licensing change to only using GPLv3 so we did the same thing in response.

in bromite there has never been any change to the license, I think the history of git may be helpful in removing that doubt.
the patches I wrote are gpl-2+, vanadium is free to use them, even without my explicit consent, if, as I think, the license makes it possible.
The previous ones (bromite), I'm sorry, but I won't edit them out of respect for the author's original choice, and as far as I know carl has never denied to vanadium the use of any patches. maybe I did without intending it, but not carl, but I think I've already fixed that.

This thread was opened by someone who is part of an organization repeatedly engaging in extreme harassment towards me

I'm sorry to hear that, also the first time I read it years ago, but please, I think there is no need to write it in this repo. i guess there are other better ways to assert your interests, and anyway i couldn't help you.
everyone is welcome here if they prove to be cooperative. the same goes for the vanadium team, I hope you do not think otherwise.

We expect them to try to launder our work on CFI support as their own, as they have done in several other important cases

as above, please tell them without using this repo as a middleman

is well aware of these issues and if they want to work things out with us they know how to contact us.

I think I don't have anything to work out

[thestinger]

Vanadium used to have licensing fully compatible with Chromium upstream. It changed to GPLv2-with-exceptions due to what happened with Bromite. There's an exception for the Apache 2 license that's otherwise incompatible with it and a WebView linking exception to permit using the Vanadium WebView in non-GPLv2-compatible apps. Apache 2 is not GPLv2 compatible so GPLv2-or-later is unusable without making an exception for that license. Similarly, not clear how WebView can possibly be legal if it's GPL. Libraries like this need LGPL or a runtime exception. Vanadium is a WebView provider, not only an application, and the WebView is a library used by apps in addition to having OS processes spawned separately for those apps to use. GPLv2 is unusable without the 2 exceptions we added. We'd be willing to go back to permissive licensing if we didn't feel the need to temporarily defend ourselves this way for Vanadium. We otherwise only use GPLv2 for the Linux kernel but we're open to using it more based on what continues to happen.

the patches I wrote are gpl-2+, vanadium is free to use them, even without my explicit consent, if, as I think, the license makes it possible.

GPLv2 is not usable without the 2 exceptions. We reluctantly use GPLv2 with these 2 exceptions making it usable because we feel it's necessary, especially now that we have a lot more resources we plan to start dedicating to Vanadium and other projects. Everyone else is doing this kind of thing instead of using the permissive Chromium licensing, so we are now too, but ours is legal.

The previous ones (bromite), I'm sorry, but I won't edit them out of respect for the author's original choice, and as far as I know carl has never denied to vanadium the use of any patches. maybe I did without intending it, but not carl, but I think I've already fixed that.

That is how it came across. We were welcome to use the patches and then at some point we weren't anymore. It became unclear what we were allowed to use. We never ended up taking any significant work despite significantly contributing whether or not that is acknowledged. People who contributed to Vanadium also contributed to Bromite because we viewed it as working with us and some features were shipped there made by people working on Vanadium despite not ever shipping them in Vanadium due to lack of resources to get it completed.

We even helped defend Bromite from repeated attacks by people trying to harm it because they saw the collaboration with us including an impersonation attack used to take over the Telegram room. Some of the people involved in that are now welcome here.

Despite that, we weren't allowed to have a private conversation with Carl but rather were forced to do it publicly and then have that used as a pile on by people misrepresenting our statements/actions and lying about us. We were blocked from responding or even making edits with clarifications. It was continued in a whole other thread attacking us. That led to a substantial amount of harassment directed towards me and was directly cited by the CalyxOS community member who repeatedly tried to have me killed by the police via swatting attacks. It continues today. It's not over.

I'm sorry to hear that, also the first time I read it years ago, but please, I think there is no need to write it in this repo. i guess there are other better ways to assert your interests, and anyway i couldn't help you.

It doesn't feel like years ago because the harassment directed towards me from the Bromite project continues today. Bromite and the Bromite developers share significant responsibility for the swatting attacks targeting me in April this year and other escalations. You made yourselves a significant part of it.

everyone is welcome here if they prove to be cooperative. the same goes for the vanadium team, I hope you do not think otherwise.

People involved in extreme abuse towards GrapheneOS and our community being welcome here means that GrapheneOS developers and our community are not welcome. You talk about collaboration in the README but yet the history of this project is tainted by what your community and the people you collaborate with have done.

as above, please tell them without using this repo as a middleman

I don't intend to communicate with them. Someone mentioned Vanadium here and I responded explaining that yes, we do use CFI and we've had to implement numerous fixes for it over the years. We also use MTE, which is also broken in Chromium without changes but they might accept our fix upstream. Despite mostly not being willing to contribute upstream anymore to avoid helping abusive people attacking us and because Google continues to give them a platform on YouTube and elsewhere, we're still reluctantly making some contributions when it's important. We'll just make our own state partitioning, content filtering and the rest with the substantial money we have available to hire people. Quite silly having each project redo all this work but it seems that's how it will be.

I think I don't have anything to work out

There's a substantial unresolved conflict and the Bromite repository is still being used to direct attacks towards us based on misrepresentations and lies we weren't allowed to respond to anymore. Bromite may be inactive in terms of development but the attacks on us are ongoing. This project as the successor to Bromite by one of the main developers who was part of that incident has inherited the baggage of it.

[peternrdstrm]

@uazo With all due respect, I think that collaboration with vanadium could do wonders for both projects.

[uazo]

Vanadium is a WebView provider, not only an application
GPLv2 is not usable without the 2 exceptions.

I don't know the licensing issues of integrating webview into an s.o., but I think it's a condition related to the fact that you release grapheneos, I shouldn't have those problems. what the user does with their device is the user's problem, I think.
but, in any case, I'm not releasing any webview for now since I wouldn't know how to test it.
if you find a way to use my patches and make them compatible with your environment, go ahead and open an issue with what you think I should do, so we can discuss it.

That is how it came across. We were welcome to use the patches and then at some point we weren't anymore. It became unclear what we were allowed to use. We never ended up taking any significant work despite significantly contributing whether or not that is acknowledged. People who contributed to Vanadium also contributed to Bromite because we viewed it as working with us and some features were shipped there made by people working on Vanadium despite not ever shipping them in Vanadium due to lack of resources to get it completed.
We even helped defend Bromite from repeated attacks by people trying to harm it because they saw the collaboration with us including an impersonation attack used to take over the Telegram room. Some of the people involved in that are now welcome here.
Despite that, we weren't allowed to have a private conversation with Carl but rather were forced to do it publicly and then have that used as a pile on by people misrepresenting our statements/actions and lying about us. We were blocked from responding or even making edits with clarifications. It was continued in a whole other thread attacking us. That led to a substantial amount of harassment directed towards me and was directly cited by the CalyxOS community member who repeatedly tried to have me killed by the police via swatting attacks. It continues today. It's not over.

I tell you again, I can't help you.

It doesn't feel like years ago because the harassment directed towards me from the Bromite project continues today. Bromite and the Bromite developers share significant responsibility for the swatting attacks targeting me in April this year and other escalations. You made yourselves a significant part of it.

I've never done anything, and anyone who knows me, knows that I would never do these things. hope that's enough for you.
bromite died a year ago

There's a substantial unresolved conflict and the Bromite repository is still being used to direct attacks towards us based on misrepresentations and lies we weren't allowed to respond to anymore. Bromite may be inactive in terms of development but the attacks on us are ongoing. This project as the successor to Bromite by one of the main developers who was part of that incident has inherited the baggage of it.

this is not bromite and I don't feel responsible for anything, so I don't know how to help you.

[bfelbo]

Thanks for the really thorough investigation! Does anyone know if this has been fixed in the recently-released Chromium version 118?

[ghost]

Any updates?

[uazo]

I have already taken my position, what updates are you talking about?