)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":36810,"name":"S","email":"jqssun@gmail.com","username":"jqssun","avatars":[{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"5796e6e8deec874561dd19ab6eb47af1ae0eb4b6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"976d0231_0fdd717f","updated":"2026-03-18 17:15:40.000000000","message":"This topic fixes a bug that causes reboot loop and UB when slot suffix is absent (e.g., on non-A/B builds). \n\nAccessing index [1] of an empty std::string is undefined behavior. Since std::string::empty() returns false for a length-1 string, the fallback to mSlots.front() is never reached. The `grubenv_abootctrl` file becomes effectively broken, making subsequent call to `getCurrentSlot()` return COMMAND_FAILED (-2).\n\nBecause `markBootSuccessful()` validates the slot before proceeding, it returns `COMMAND_FAILED` immediately. `vold`\u0027s checkpoint commit fails and the device immediately reboots with the following in `logcat`\n\n```\n03-18 14:16:35.097   234   234 I vold    : Using AIDL version of IBootControl\n03-18 14:16:35.097   234   234 E vold    : MarkBootSuccessful failed Status(-8, EX_SERVICE_SPECIFIC): \u0027-2: Operation failed\u0027\n03-18 14:16:35.097   234   234 E Checkpoint: Error marking booted successfully: Status(-8, EX_SERVICE_SPECIFIC): \u0027-2: Operation failed\u0027\n03-18 14:16:35.697   680  1336 I ShutdownThread: Rebooting, reason: Checkpoint commit failed\n```\nThis creates an infinite reboot loop on any non-A/B build that includes the GRUB boot control HAL. Fixed by reading the slot suffix into a temporary string, and only extracting suffix when it is long enough; otherwise it falls back to the mSlots.front() default. Boot loop resolved with the fix.","commit_id":"f2f42f27559cd0affeb7bd15702aa783982a633d"},{"author":{"_account_id":32245,"name":"0xCAFEBABE","email":"me.cafebabe@gmail.com","username":"me-cafebabe","avatars":[{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}],"status":"nya~"},"change_message_id":"e89d8531d4dd0cf25a8511eecf738b81af7dea8b","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":1,"id":"3009f210_d4713363","in_reply_to":"976d0231_0fdd717f","updated":"2026-03-19 00:28:52.000000000","message":"sure, but why do you want to include boot control HAL on non-A/B builds though...","commit_id":"f2f42f27559cd0affeb7bd15702aa783982a633d"},{"author":{"_account_id":36810,"name":"S","email":"jqssun@gmail.com","username":"jqssun","avatars":[{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"3bbe7822133ffb8ed2e7118e5bf6ed2443741438","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"9640b096_18a765db","updated":"2026-03-19 01:24:28.000000000","message":"I\u0027ve had a look at `GrubBootControl.cpp` and thought it seemed it was designed to be compatible with single slot systems too. There\u0027s an empty slot fallback included `if (current_slot.empty()) current_slot \u003d mSlots.front();` which appeared to be there to explicitly support A-only devices. Additionally, there\u0027s a dedicated `getNumberSlots()`/`mSlots.size()` method and `IsValidSlot(slot)` guards already. This made me believe that it was intended to support single slot builds too.\n\nTesting shows the HAL works correctly on non-A/B builds bar that specific problematic line. At first glance that line seemed like a UB bug regardless, so I figured I should just patch it. Context of all this was I was getting boot failures with `AB_OTA_UPDATER\u003dfalse` on `virtio_*` targets. I guess I could potentially patch it downstream but this seems to be the cleanest approach.","commit_id":"a959257d5740726af39deb0725cbfcf3c051aa18"},{"author":{"_account_id":36810,"name":"S","email":"jqssun@gmail.com","username":"jqssun","avatars":[{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"4ac9bca8299818dcc84d72dacd8dafa860f467a5","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"6e707c05_608a16d1","in_reply_to":"86d476b2_b96bf94c","updated":"2026-03-19 02:11:08.000000000","message":"Honestly... not much. It\u0027s more so we don\u0027t need a separate file that handles the same thing. (The `is_bootable` and `is_successful` keys are quite helpful, and the rest of `grubenv` can be useful even though not that\u0027s not strictly `abootctrl`)\n\nFor context this is the list of keys it adds on a non-A/B device (after the patch). To the rest of the system the A-only setup is similar to certain OEMs where the second slot can technically be there but just empty. If at any point they want to switch to an A/B layout they should be able to do that relatively easily.\n\n```\nabootctrl_global_active_slot\u003da\nabootctrl_global_current_slot\u003da\nabootctrl_global_no_auto_slot_switch\u003dfalse\nabootctrl_global_snapshot_merge_status\u003dnone\nabootctrl_slot_a_is_bootable\u003dtrue\nabootctrl_slot_a_is_successful\u003dtrue\nabootctrl_slot_a_retry_count\u003d\n```","commit_id":"a959257d5740726af39deb0725cbfcf3c051aa18"},{"author":{"_account_id":32245,"name":"0xCAFEBABE","email":"me.cafebabe@gmail.com","username":"me-cafebabe","avatars":[{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}],"status":"nya~"},"change_message_id":"a3aa539e24be40820b88e7ec3dc4aa7051bf14aa","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"86d476b2_b96bf94c","in_reply_to":"9640b096_18a765db","updated":"2026-03-19 01:58:41.000000000","message":"what is the use of boot control HAL on non-A/B which does not have slot switching?","commit_id":"a959257d5740726af39deb0725cbfcf3c051aa18"}],"grub/libgrub_boot_control/GrubBootControl.cpp":[{"author":{"_account_id":32245,"name":"0xCAFEBABE","email":"me.cafebabe@gmail.com","username":"me-cafebabe","avatars":[{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/579a599284aa0f2454acf08453b0f261.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}],"status":"nya~"},"change_message_id":"1169515d8089747453479c1adcd79a0efa8ec0b8","unresolved":true,"context_lines":[{"line_number":84,"context_line":"    // Global: Active slot and Current slot"},{"line_number":85,"context_line":"    string current_slot;"},{"line_number":86,"context_line":"#if defined(__ANDROID_VENDOR__) || defined(__ANDROID_RECOVERY__) || defined(__ANDROID_APEX__)"},{"line_number":87,"context_line":"    current_slot \u003d GetProperty(\"ro.boot.slot_suffix\", \"\")[1];"},{"line_number":88,"context_line":"#endif"},{"line_number":89,"context_line":"    if (current_slot.empty()) current_slot \u003d mSlots.front();"},{"line_number":90,"context_line":"    SetItemValueForGlobal(kItemGlobalActiveSlot, current_slot, false);"}],"source_content_type":"text/x-c++src","patch_set":1,"id":"03c707fd_39f97bff","side":"PARENT","line":87,"updated":"2026-03-23 15:14:33.000000000","message":"wait, what about this instead...\n```\n    current_slot \u003d GetProperty(\"ro.boot.slot_suffix\", \"_\" + mSlots.front())[1];\n\n```\nso that in case of the prop is empty, GetProperty() will return \"_a\"\n\neven if that doesn\u0027t compile, I believe there\u0027s still better solutions","commit_id":"a041424e50ef9304092f0663a60b906925000d29"},{"author":{"_account_id":36810,"name":"S","email":"jqssun@gmail.com","username":"jqssun","avatars":[{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d32","height":32},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d56","height":56},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d100","height":100},{"url":"https://www.gravatar.com/avatar/c0a151a62c3685fc32c360b019856fdd.jpg?d\u003didenticon\u0026r\u003dpg\u0026s\u003d120","height":120}]},"change_message_id":"6361044f6815666e66095f4379d7ed4590509bc1","unresolved":true,"context_lines":[{"line_number":84,"context_line":"    // Global: Active slot and Current slot"},{"line_number":85,"context_line":"    string current_slot;"},{"line_number":86,"context_line":"#if defined(__ANDROID_VENDOR__) || defined(__ANDROID_RECOVERY__) || defined(__ANDROID_APEX__)"},{"line_number":87,"context_line":"    current_slot \u003d GetProperty(\"ro.boot.slot_suffix\", \"\")[1];"},{"line_number":88,"context_line":"#endif"},{"line_number":89,"context_line":"    if (current_slot.empty()) current_slot \u003d mSlots.front();"},{"line_number":90,"context_line":"    SetItemValueForGlobal(kItemGlobalActiveSlot, current_slot, false);"}],"source_content_type":"text/x-c++src","patch_set":1,"id":"a09563a1_0b883ff3","side":"PARENT","line":87,"in_reply_to":"03c707fd_39f97bff","updated":"2026-03-23 19:59:48.000000000","message":"That might work, although it still feels somewhat unsafe in that if \"a\" or any other string was passed instead of \"_a\" we still get a UB. Maybe this is cleaner?\n```\nif (auto s \u003d GetProperty(\"ro.boot.slot_suffix\", \"\"); s.size() \u003e\u003d 2) current_slot \u003d s[1];\n```\n\nI think a length check for safety is needed regardless, but maybe better if we combine that with an underscore check?","commit_id":"a041424e50ef9304092f0663a60b906925000d29"}]}
