> It's an 8-bit counter... just allocate a fixed 256 entries and be done with it. That reads like code written by people without any embedded/low-level experien

> It's an 8-bit counter... just allocate a fixed 256 entries and be done with it. That reads like code written by people without any embedded/low-level experience. I guess you are talking about the first commit I linked. The problem here seems to be that some events of the kind HCI_READ_RMT_EXT_FEATURES_COMP_EVT can be shorter than the assumed 13 bytes. The code contains no check for that and if the events are shorter, it would read data from after the allocation. It would use that data to index inside arrays, etc. Now, if you just allocate a buffer of 256 entries but don't do anything else, it wouldn't read data from outside the allocation, yes, but it would still read uninitialized data, as nothing would be written after the end of the valid data. That uninitialized data could e.g. come from previous freed allocations. This would hardly be an improvement. You'd have to allocate and zero-initialize it, and then you'd still have the problem whether zero is invalid data or part of the allocation... Even if code would figure that out, it would be extremely smelly code and I'd never merge it in any projects I maintain. The approach done by the patch to just check the length is much much better. The length is sent as part of the event. > Keeping code simple and without unnecessary abstraction is a far more valuable skill than $safe-language-trend-of-the-day. This code almost directly maps to the bluetooth host controller interface which is part of the published Bluetooth standard. So you can't change the core concepts of it. There are a few abstraction layers which copy the data for some reason from a new/delete managed hidl_vec to a malloc/free managed array (check hciEventReceived function in hci/src/hci_layer_android.cc). Yes, I'd say that some of those layers are indeed unnecessary. But those abstraction layers are not where the vulnerability occurs. It occurs in the code that parses the message, and the bug is that the code does not check the length of the input data. This is a classic bug that can occur in C/C++ codebases. Safe Rust prevents OOB writes/reads by performing bounds checks when you index into a slice. The issue with languages like C is that verifying that code is safe is extremely hard, even harder than writing it in the first place. This codebase seems to have not been written by Google but by Broadcom, so Google would have to verify whether what Broadcom wrote is actually safe. With Rust, such verification is easy. If your code makes little use of unsafe, and most code doesn't actually have to, it's easy to verify its safety (at least for the classes of bugs that Rust eliminates). Due to the strong typing, other types of bugs are made harder to write as well.