init: give bootconfig priority over kernel cmdline
Inspired by:
79058486 ("init: handle more bootconfig parameters")
On the one hand,
there is a new bootconfig approach (since Android 12 [1]) to pass
androidboot.* parameters from bootloader to initd. Documentation
encourages migration, stating that parameters must be passed only via
bootconfig and never via kernel cmdline [1].
But on the other hand,
current implementation gives priority to cmdline. This has subtle
side-effect. Initd is interested of androidboot.* parameters only,
which become 'ro.*' properties, hence they are write-once properties.
During parsing the first encountered androidboot.* parameter takes an
effect (a.k.a. left-most parsing rule).
Hence:
* when device owner has fully supported bootconfig feature w.r.t. [1]
* and when for one reason or another there is some androidboot.*
parameter in the kernel cmdline, duplicating the one in bootconfig
the legacy approach (kernel cmdline) overwrites parameter from
bootconfig. This looks like misbehavior.
=== Evidence of misbehavior
The intended priority order is documented in:
1) 79058486 ("init: handle more bootconfig parameters")
2) the fs_mgr_get_boot_config() function [2], which explicitly states
the priority should be device tree, properties, bootconfig, then kernel
cmdline
Ambiguity was introduced in:
a4ef15be ("Support bootconfig in first stage init and fs_mgr")
Patch resolves this by changing the execution order.
=== Tested
This issue was disclosed during security audit and involves U-Boot.
U-boot has been fully migrated to bootconfig feature, but attacker
could set androidboot.some_flag in bootargs, leading to overwriting the
same one from bootconfig during initd parsing.
To reproduce:
1) Set in U-Boot shell the following:
```
# System settings:
$ env set bootconfig ${bootconfig} androidboot.some_flag=true
# Emulate attacker's modifications:
$ env set bootargs ${bootargs} androidboot.some_flag=false
$ env save
$ reset # reboot in Android
```
2) After reboot, the following will be in dmesg:
```
init: Init cannot set 'ro.boot.some_flag' to 'true': Read-only \
property was already set
```
Links:
[1] https://source.android.com/docs/core/architecture/bootloader/implementing-bootconfig#incremental-implementation
[2] https://cs.android.com/android/platform/superproject/main/+/main:system/core/fs_mgr/libfstab/boot_config.cpp?q=fs_mgr_get_boot_config
Signed-off-by:
Evgeny Bachinin <EABachinin@salutedevices.com>
(cherry picked from https://android-review.googlesource.com/q/commit:d2d95ea846a2eb88a74f1c5d0764d7aab90918c0)
Merged-In: I98f281a12d5f5f5620a7cca68674ac57de0da2cc
Change-Id: I98f281a12d5f5f5620a7cca68674ac57de0da2cc
Loading
Please register or sign in to comment