Bromite: [Android 9] AImageReader video playback crash

[Namit-Nayan]

Bromite version

Version: 87.0.4280.67
Arch: arm64
Android version: 9.0(Lineage OS 16)
Device model: Asus-X00TD

Is this bug about the SystemWebView?

No

Is the bug reproducible with latest version?

Yes

Can the bug be reproduced with corresponding Chromium version?

Yes

Describe the bug

Can't play some YouTube videos. And bromite keeps stopping popup on every video page.

Steps to reproduce the bug

Steps to reproduce the bug:

1. Go to https://m.youtube.com/watch?v=Pvv_yfoEvcM
2. You will see a popup bromite keeps stopping , the browser is still running, though the page reloads (or it seems to).
3. Try to play the video
4. Video won't play. (Some other videos played, 2. Point happened there too)
   (I guessed this is due to autoplay blocking, but when I tried to toggle that setting, it is stuck on blocked) Also later found it to be reproducible on latest chromium version.

Expected behavior

Video plays and no bromite keeps stopping popup.

Screenshots

[uazo]

bug confirmed, here its tombstone
https://gist.github.com/uazo/471e19b4785b04be8646937370e6a9e3

[uazo]

other info

11-20 08:11:57.133  2777  2816 W VideoCapabilities: Unrecognized profile/level 0/0 for video/mpeg2
11-20 08:11:57.133  2777  2816 W VideoCapabilities: Unrecognized profile/level 0/2 for video/mpeg2
11-20 08:11:57.133  2777  2816 W VideoCapabilities: Unrecognized profile/level 0/3 for video/mpeg2
11-20 08:11:57.136  2777  2816 W VideoCapabilities: Unrecognized profile/level 32768/2 for video/mp4v-es
11-20 08:11:57.154  2777  2816 W VideoCapabilities: Unrecognized profile/level 1/32 for video/mp4v-es
11-20 08:11:57.154  2777  2816 I VideoCapabilities: Unsupported profile 16384 for video/mp4v-es
11-20 08:11:57.154  2777  2816 I VideoCapabilities: Unsupported profile 16384 for video/mp4v-es
11-20 08:11:57.163  2777  2816 I VideoCapabilities: Unsupported profile 4 for video/mp4v-es
11-20 08:11:57.173  2777  2816 W cr_MediaCodecUtil: HW encoder for video/avc is not available on this device.

11-20 08:10:13.081  2577  2742 I app_process64: frameworks/av/media/ndk/NdkImageReader.cpp:238:13: runtime error: control flow integrity check for type 'void (void *, AImageReader *)' failed during indirect function call
11-20 08:10:13.081  2577  2742 I app_process64:
11-20 08:10:13.085  2577  2597 D NdkImageReader: acquireImageLocked: Overriding buffer format YUV_420_888 to 0x300.
11-20 08:10:13.139  2577  2742 I app_process64: (/data/app/org.chromium.chrome.stable-7gyJ7xQDm3QLarRXDrI5VQ==/base.apk+0x9590200): note: (unknown) defined here
11-20 08:10:13.139  2577  2742 I app_process64:
11-20 08:10:13.287  2577  2742 F libc    : Fatal signal 6 (SIGABRT), code -6 (SI_TKILL) in tid 2742 (ImageReader-1x1), pid 2577 (ileged_process0)
11-20 08:10:13.287  2754  2754 E chromium: [1120/081013.286528:ERROR:socket.cc(182)] incorrect payload size 0
11-20 08:10:13.355  2758  2758 I crash_dump64: obtaining output fd from tombstoned, type: kDebuggerdTombstone

Crash is here
frameworks/av/media/ndk/NdkImageReader.cpp:238:13: runtime error: control flow integrity check for type 'void (void *, AImageReader *)

[uazo]

maybe related

https://github.com/bromite/bromite/issues/445
https://groups.google.com/a/chromium.org/g/chromium-dev/c/EbaVVYVaSA8/m/9Hnc-4w_CAAJ
https://bugs.chromium.org/p/chromium/issues/detail?id=977583
https://github.com/GrapheneOS/platform_frameworks_av/commit/7f7b234d7c05c3bdb304d886a317fd895e18021e

[oiao]

I can confirm the same behavior as in #455 after the update to v87. Was working fine in v86

[csagan5]

The last time we ran tests for this issue was in #755; unfortunately it is a recurring issue.

[csagan5]

Can the bug be reproduced with corresponding Chromium version?

Yes

This is not a bug in Bromite; it is a bug in Chromium. We have introduced patches in the past to counteract it, they seem to not be effective anymore.

@uazo @oiao please report your Android version, device model and the version you installed (arm or arm64).

@uazo the problem is that chrome://gpu should contain disable_aimagereader, will check why that is not happening, might be easy to fix.

[csagan5]

This bug was extensively discussed in #445; @uazo if you wonder, the patch is this: https://github.com/bromite/bromite/blob/87.0.4280.67/build/patches/Restore-GPU-bug-blacklist-for-AImageReader-on-ARM-CPUs.patch
The patch is limited to ARM GL vendor but now the bug is reproducible with other vendors as well.

Until a new version of Bromite is released with the fix everybody can go to chrome://flags and disable AImageReader manually. This crash affects only Android 9 and ARM64, as far as I am aware.

If we get enough reports about the model we can adjust the patch accordingly.

[uazo]

please report your Android version, device model and the version you installed (arm or arm64).

'HUAWEI/BLA-L29/HWBLA:8.0.0/HUAWEIBLA-L29S/137(C432):user/release-keys'
hw is a BLN-L22, arm64, Android 9, but it's a custom build I used only for tests.
In my Android 10 (Lineage 17.1) works perfectly.

The patch is limited to ARM GL vendor but now the bug is reproducible with other vendors as well.

I was trying to understand why.
the error entry point is this:

https://source.chromium.org/chromium/chromium/src/+/master:gpu/command_buffer/service/image_reader_gl_owner.cc;l=175

among other things, putting it as a comment everything works perfectly but I don't understand why. some idea?

do you know how the CFI works? can i imagine the problem could be a race-condition that makes this (https://cs.android.com/android/_/android/platform/frameworks/av/+/e2eba97ec5b0bdeb128eaed3dac00bbaca2ff56e:media/ndk/NdkImageReader.cpp;l=252;bpv=1;bpt=0) point to a null pointer? or maybe listener_ is it released before releasing that pointer?

EDIT: ah, I've tried also this https://chromium-review.googlesource.com/c/chromium/src/+/2537782 but it doesn't seem to me to solve it, the only thing they solve is to comment on that line

[h3ndrik]

Android Version: 9 (crDroid 5.12)
Model: ZTE A2017X
Version: arm64
also affected. Disabling AImageReader in chrome://flags works.

[csagan5]

I was trying to understand why.
the error entry point is this:

https://source.chromium.org/chromium/chromium/src/+/master:gpu/command_buffer/service/image_reader_gl_owner.cc;l=175

among other things, putting it as a comment everything works perfectly but I don't understand why. some idea?

do you know how the CFI works? can i imagine the problem could be a race-condition that makes this (https://cs.android.com/android/_/android/platform/frameworks/av/+/e2eba97ec5b0bdeb128eaed3dac00bbaca2ff56e:media/ndk/NdkImageReader.cpp;l=252;bpv=1;bpt=0) point to a null pointer? or maybe listener_ is it released before releasing that pointer?

EDIT: ah, I've tried also this https://chromium-review.googlesource.com/c/chromium/src/+/2537782 but it doesn't seem to me to solve it, the only thing they solve is to comment on that line

@uazo see this comment on the upstream issue (by @thestinger); we have discussed it in the past, see his explanation here.

How it works: call paths are tagged by the compiler and then verified at runtime for integrity; the code you linked is about dynamic loading, never a pretty business.

Why it is now happening for Qualcomm chipsets on Android 9: I do not know, my guess is that something changed in Android 9 itself and now AImageReader hits the faulty paths.

The solution is to disable AImageReader for Qualcomm as well, I will release a fixed version later; I wonder if other chipsets are affected too.

[uazo]

@thestinger said

but we haven't yet identified the function being called and how it violates CFI. It's a bit painful with how it crosses the boundaries of these programs

So, the function is here https://source.chromium.org/chromium/chromium/src/+/master:gpu/command_buffer/service/image_reader_gl_owner.cc;l=175
sure enough because taking it off works, and among other things, I still have to understand why everything works anyway.

the code you linked is about dynamic loading, never a pretty business.

out of pure curiosity I try to understand something about CFI and see if it can be solved outside of aosp

my guess is that something changed in Android 9 itself and now AImageReader hits the faulty paths.

so i guess there is no sw that can use that library, at least as dynamic loading. possible?

The solution is to disable AImageReader for Qualcomm as well, I will release a fixed version later; I wonder if other chipsets are affected too.

Ok so you're disabling accelerated video decode, for those gpu's, right?

[csagan5]

so i guess there is no sw that can use that library, at least as dynamic loading. possible?

If this is a hardware-accelerated function then the actual function lives in a vendor-provided binary blob; on some devices that function might work, on others not. But my understanding is partial, maybe @thestinger knows better.

Ok so you're disabling accelerated video decode, for those gpu's, right?

Yes, until now only for ARM/ARM64 GPUs on Android9; other related features flags are disabled for Android10+. I will disable AImageReader for Qualcomm chipsets on Android9 and below.

The best would be to test on various hardware and Android versions combinations, but sadly that is not possible.

@uazo one possible improvement is to expose flags for these 2 features here (AImageReaderMediaPlayer and AImageReaderVideoOutput): https://github.com/bromite/bromite/blob/master/build/patches/Disable-AImageReader-for-ARM64-P-and-ARM64-Q.patch

So that they can be tested on the various combinations of hardware/software; if you are interested you could submit a patch for that after next release.

[thestinger]

This will presumably get fixed when Chromium finally moves to preferring 64-bit by default. This could be why they haven't moved to it yet. They were going to do it for Chromium 86 (I think) and reverted it.

[uazo]

one possible improvement is to expose flags for these 2 features here

in the settings? yes, sure, i will try, can you open a new issue for that?

[csagan5]

in the settings? yes, sure, i will try, can you open a new issue for that?

A flag in chrome://flags, see #818

[csagan5]

Fixed in 87.0.4280.68.

[uazo]

sorry, a thought.
In your opinion, is it fair to imagine that if I load a stub library compiled for P+ here (https://source.chromium.org/chromium/chromium/src/+/master:base/android/android_image_reader_compat.cc;l=61) that calls the NDK in turn, with thestinger's patch in the callback, it should work?
not that I'm capable of it, but I wanted to ask you more experts, before trying.

[csagan5]

@uazo I don't think so, otherwise it would be a security issue. Security-wise I prefer to disable the acceleration rather than disabling CFI.

[uazo]

I don't think so

instead I think so, but only if the .so is a physical file.
let's see if I can explain myself.

from what I understand, in android / linux there is something (maybe a runtime linker or it is the same llvm that inserts specific instructions, I ignore this) that verifies CFI.
in calls between shared libraries the mechanism is documented here http://clang.llvm.org/docs/ControlFlowIntegrityDesign.html#shared-library-support
but the problem is that in chromium modern there are no physical libraries because the linker integrated in the code generates a unique file (crazy.libchrome.so) which remains in the apk and is not unpacked (references https://chromium.googlesource.com/chromium/src/+/master/docs/android_native_libraries.md) so libmediandk.so is unable to extract the CallSiteTypeId and verify it (read https://struct.github.io/cross_dso_cfi.html)
this explains why with component_build = true it would work (ref https://bugs.chromium.org/p/chromium/issues/detail?id=977583#c12).

therefore, if we wanted to solve it or just to verify it, we should try to re-enable the component_build in bromite, which is broken, or just switch to monochrome or thricrome (and so I understood why he said to switch to 64 bit).

tell me if I've said nonsense things

Security-wise I prefer to disable the acceleration rather than disabling CFI.

ok

[csagan5]

this explains why with component_build = true it would work

Crashes were happening also before the deprecation of component builds, see #445.

Reopening this because bug does not happen with the AImageReader flag disabled but does still happen even with latest Bromite patch.

[slowthgt]

Not sure if it still helps, but can confirm the playback crash bug on mine. Disabling AImageReader flag solves it.
Model: OnePlus 3 (arm64)
OS: Android 9 (OxygenOS 9.0.6)
Bromite version: 87.0.4280.68

[csagan5]

Fixed in 87.0.4280.81. AImageReader will always be disabled for ARM64; it can be enabled through the corresponding flag.