Ungoogled-chromium: version 79+: patch causes crash on startup when both enable_widevine and is_official_build are set

Created on 18 Jan 2020 · 4Comments · Source: Eloston/ungoogled-chromium

There is an issue with the following patch: https://github.com/Eloston/ungoogled-chromium/blob/master/patches/extra/debian_buster/fixes/widevine-enable-version-string.patch

When built with the gn flag enable_widevine=true, it causes GetBundledWidevine() to always be called if ENABLE_WIDEVINE_CDM_COMPONENT is defined. This function makes the following call:

CHECK(base::PathService::Get(chrome::DIR_BUNDLED_WIDEVINE_CDM,
                                     &install_dir));

It doesn't patch the base::PathService::Get function, so it always returns false unless BUNDLE_WIDEVINE_CDM is defined - which only happens on official Google builds.

The CHECK() macro will normally just print an error and then continue. However, if built with the gn flag is_official_build=true, it will insert an illegal instruction which will hard-crash the application. Note that is_official_build is NOT the "official Google Chrome build" flag that enables BUNDLE_WIDEVINE_CDM, that's a separate Google-only flag.

Although this patch won't cause any user-visible problems unless is_official_build=true is set, it also doesn't seem to actually do anything in chromium 79+, because GetBundledWidevine() will always fail.

The issue was fixed in the Gentoo ebuild by removing that specific patch: https://github.com/PF4Public/gentoo-overlay/issues/24

At least for Gentoo, the "bundled" Widevine isn't necessary; it includes a separate chrome-binary-plugins package which works with the new chromium-79 "component-updated" Widevine packaging. I'm not sure if something in this patch is necessary for other distributions/builds, but at the very least, it needs to patch the part of base::PathService::Get to get the DIR_BUNDLED_WIDEVINE_CDM directory correctly.

If the "bundled" Widevine code isn't necessary for all distros, would it be better to remove this patch and let each individual build apply the correct patch if necessary?

bug

Source

thubble

🚀1

All 4 comments

In a nutshell, it appears that either the patch needs to be removed, or is_official_build disabled. (@thubble , did I get it right?)

I did a quick look through ungoogled repos and turns out that is_official_build=true is not widely used. Mac repo uses it at least.

I personally kinda _inherited_ it from the times, when Ian was maintaining Gentoo ebuild. So if somebody knowledgeable could provide details on is_official_build=true (its implications and usage etc.), that would help decide, whether either the patch or is_official_build stays in the favor of the other.

PF4Public on 19 Jan 2020

Disabling is_official_build would fix the crash, but I still think there's a problem with this patch. It seems to be incomplete, in that it patches some code to always call base::PathService::Get, but doesn't patch base::PathService::Get itself to handle the chrome::DIR_BUNDLED_WIDEVINE_CDM parameter properly. So it ends up always failing to get the bundled Widevine directory (returning false), which crashes with is_official_build but simply doesn't do anything otherwise.

I think what the patch is intended to do is allow the Widevine DLLs to be installed in whatever the "bundled" directory is, and behave how it would in previous Chromium versions. Because it's a Debian patch, I suspect it may have something to do with how they bundle the DLLs. Gentoo (and possibly other distros) works fine with the new default behaviour in 79.

It does appear that this patch isn't in the current official Debian patches. The only 2 patches they apply are:

The closest I could find to this patch is from Ubuntu's "snap" builds, so maybe it's intended for that?

https://git.launchpad.net/~chromium-team/chromium-browser/+git/snap-from-source/tree/build/patches/widevine-enable-version-string.patch?h=dev
https://git.launchpad.net/~chromium-team/chromium-browser/+git/snap-from-source/tree/build/patches/widevine-other-locations?h=dev (NOTE - this looks like it might contain a partial fix for this issue, but it still assumes the snap-specific paths for Widevine).

thubble on 19 Jan 2020

👍1

I encountered this problem in Debian a little while back, and I fixed it by using a different widevine-locations.patch: https://github.com/ungoogled-software/ungoogled-chromium-debian/blob/fd302d290a7ad787159033799096f61b501bc6a9/debian/patches/fixes/widevine-locations.patch. I have confirmed that Widevine works fine when installed to ~/.local/lib

Both enable_widevine and is_official_build are pretty platform-specific flags, so we should probably move all Widevine patches to the platform repos.