Bug#1033176: linux: Future Android/Waydroid support
On Thursday, 30 March 2023 00:27:41 CET Diederik de Haas wrote: > The patch description which turned the module from a `bool` into a > `tristate` explicitly mentioned security as a reason so that the module > would ONLY be loaded when needed, instead of always for everyone ... for > security reasons. Kees Cook tooted about and GKH boosted the following link: https://lore.kernel.org/lkml/20231101-rust-binder-v1-0-08ba9197f...@google.com/ Titled "Setting up Binder for the future" which is a patch set rewriting the implementation of Binder with Rust. The cover page ofc describes the patch set and also contains the following: "We have left the binderfs filesystem component in C. Rewriting it in Rust would be a large amount of work and requires a lot of bindings to the file system interfaces. Binderfs has not historically had the same challenges with security and complexity, so rewriting binderfs seems to have lower value than the rest of Binder." signature.asc Description: This is a digitally signed message part.
Bug#1033176: linux: Future Android/Waydroid support
I've worked on Waydroid support in Mobian and tried to keep our wiki article (https://wiki.mobian-project.org/doku.php?id=waydroid) updated with installation instructions from source. In my opinion there are two main considerations regarding the binder modules: 1. Waydroid really wants to use binderfs as a replacement for the now legacy method of predefining binder interfaces. From what I understand this isn't really within the control of Waydroid but rather what Android expects of the underlying kernel, but predefining the right ones when loading the module works for now. I would expect this latter part could change in the future without much notice from upstream (upstream in this case would be AOSP). 2. Until Waydroid is packaged for Debian we probably can't be expected to make specific accommodations for Waydroid, given it's a very niche use case. On the other hand Waydroid is almost essential for some Mobian users. Since Mobian aims to reintegrate into Debian it's a problem we'll have to address sooner or later. Both of these considerations would be easily solved by packaging Waydroid in Debian. With official Debian packaging it would be possible to keep changes in sync as they happen, in this case updating the binder interface names when required by new versions of Waydroid. With official Debian packaging it would also be easily justifiable to include the binder module in the Debian kernel tree. Debian packaging of Waydroid and its dependencies mostly exists upstream but with some trivial errors preventing easy builds. With that in mind, until Waydroid is in the Debian repos I see two somewhat readily available options: 1. Drop the "binder" string from CONFIG_ANDROID_BINDER_DEVICES and leave the default value (which then becomes "binder,hwbinder,vndbinder"). This way we're reducing the delta between the default linux config and makes Waydroid work out of the box (for now). Supposedly this wasn't needed for some users where Waydroid had explicitly loaded the module with the right device names, but that does not match my experience on amd64 where I had to add 'options binder_linux devices="binder,hwbinder,vndbinder"' to "/etc/modprobe.d/binder-linux.conf". 2. Essentially option 1 but with binderfs enabled. This would ensure compatibility with newer versions of Waydroid (with newer AOSP), at least for the foreseeable future. Option 2 is better in my opinion but currently "CONFIG_ANDROID_BINDERFS=y" breaks compilation of "binder_linux" (modpost says "put_ipc_ns" and "init_ipc_ns" is undefined). I don't know how easy of a fix this would be and I would be more comfortable with having the modular build patch upstreamed to ensure this is widely tested. Ideally there would be a third option if the kernel could drop the legacy device names entirely and only allow creation of devices through binderfs. In that case I wouldn't see any reason to have this compiled as a module at all, since there would be no devices to exploit security holes in without first creating them through binderfs (and if you can create them without going through Waydroid you probably have access to other exploitable code as well). For reference, Mobian has both the current legacy device names in CONFIG_ANDROID_BINDER_DEVICES and binderfs enabled. Finally, it would be great if we could find someone who can take the role of a Waydroid package maintainer. Updating the Mobian wiki on and off has proved to me that I don't have the time to properly maintain the package, but I can assist someone else in packaging it to the best of my abilities. I don't have enough experience with Debian packaging to clean up the existing package without assistance either but I think I have a decently good grasp of the Waydroid internals at this point.
Bug#1033176: linux: Future Android/Waydroid support
On Wednesday, 29 March 2023 22:59:13 CEST Diederik de Haas wrote: > Control: reopen -1 > > On Tuesday, 21 March 2023 14:21:05 CEST Debian Bug Tracking System wrote: > > > In https://github.com/waydroid/waydroid/issues/811 I asked 'upstream' > > > for recommendations on which/how/what kernel modules to enable. > > > > In https://git.kernel.org/linus/9e18d0c82f0c07f2a41898d4adbb698a953403ee > > the default value of BINDER_DEVICES was changed to > > "binder,hwbinder,vndbinder" whereas the Debian kernel only has "binder". > > Also in other places they use the 3 values, but I haven't been able to > > find out *why*. > > As I wouldn't be able to make the argument to change it to something else > > and the most important thing is to enable ANDROID_BINDER_IPC, which is > > already the case, there's no need to keep this bug open. > > > > If someone *can* substantiate why and how things should be changed, they > > should file a bug report (themselves) making the case. > > Just received an (unexpected) extra response, which DOES answer the Q I had: > > "BINDER_DEVICES defines which binder device names should appear in /dev. > Different android versions required different devices to be available: the > latest android version uses "binder,hwbinder,vndbinder" meaning /dev/binder, > / dev/hwbinder, /dev/vndbinder. > At some point Google realized that it's inconvenient to have this change at > compile time, so they introduced BINDERFS: you can mount a filesystem of > type binder somewhere and use IOCTLs on it to allocate new device names. > > So if you use BINDERFS it doesn't matter which value of BINDER_DEVICES you > set because you can add (or remove) devices at runtime. In this case > BINDER_DEVICES is just the set of initial devices present in the root of a > binderfs. > > For mainline linux, BINDERFS is heavily recomended. Waydroid will use it to > create all the binder devices it needs. You can leave BINDER_DEVICES empty > or unchanged or whatever you prefer." > > I don't really understand how or what `ioctl`s are, but I'm pretty sure > others do and can now make an informed choice if and how things could be > improved. I'm pretty sure that'll be for Trixie, which has the nice benefit > that it can be extensively tested. Possibly unintentional, but another reply came in with a valid point AFAICT: "That caught me by surprise because I knew Waydroid worked on bullseye already, and it has the same CONFIGs. Then I finally realized... When you compile binder as a module (rather than built-in), when inserting the module you can provide the binder device names you want in the module parameters! So in this case `CONFIG_BINDER_DEVICES="binder"` is not a limitation." Is this an intended feature or a security concern? The patch description which turned the module from a `bool` into a `tristate` explicitly mentioned security as a reason so that the module would ONLY be loaded when needed, instead of always for everyone ... for security reasons. signature.asc Description: This is a digitally signed message part.
Bug#1033176: linux: Future Android/Waydroid support
Source: linux Version: Future Android/Waydroid support Severity: wishlist Forwarded: https://github.com/waydroid/waydroid/issues/811 -BEGIN PGP SIGNED MESSAGE- Hash: SHA256 In https://salsa.debian.org/kernel-team/linux/-/merge_requests/651 I had initially removed the 2 Android related patches for the following reason: Drop patches: - debian/android-enable-building-ashmem-and-binder-as-modules.patch - debian/export-symbols-needed-by-android-drivers.patch After https://bugs.debian.org/901492 the preceding 2 patches were created for anbox support. However in kernel 5.18 `ashmem` was removed from the upstream kernel and since then, anbox has not been working as reported in https://bugs.debian.org/1014329. Then in https://bugs.debian.org/1032304, titled "RM: anbox -- ROM; Upstream discontinued", the anbox package has been removed from the Debian archive. And on Anbox's GH page one can see the following: "It's development has however stalled in the past years and it's only fair to say that now in 2023 it's no longer actively developed." So it's of no use to continue carrying these patches. Even though anbox is removed from the Debian archive and upstream more or less 'dead', it turns out that Waydroid (= ~ anbox's successor) could probably benefit from support in the Debian kernel too. So I undid/reverted the dropping of those patches. Removal of Android support should be done separately and ideally based on a bug report in the BTS, hence this bug report. In https://github.com/waydroid/waydroid/issues/811 I asked 'upstream' for recommendations on which/how/what kernel modules to enable. - -- System Information: Debian Release: 12.0 APT prefers unstable-debug APT policy: (500, 'unstable-debug'), (500, 'stable-security'), (500, 'unstable'), (500, 'testing'), (500, 'stable'), (101, 'experimental') Architecture: amd64 (x86_64) Kernel: Linux 6.1.0-6-amd64 (SMP w/16 CPU threads; PREEMPT) Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US:en Shell: /bin/sh linked to /usr/bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled -BEGIN PGP SIGNATURE- iHUEARYIAB0WIQT1sUPBYsyGmi4usy/XblvOeH7bbgUCZBZPHwAKCRDXblvOeH7b bpbCAPsFqbmYhJzizpispzGdw+ksgNnm59ZQDtmSMSYcNk5S5gD9FcHWbGx6XtJ2 5YefJ1PVNv1BbtdAkofzw2Nz5gLLDgE= =pyYQ -END PGP SIGNATURE-
