It's a valuable skill for sure, but there apparently isn't enough people with this skill on the market, which is why $safe-language-trend-of-the-day is being developed and gains momentum.

As someone who both writes C and Rust I don't think the two contradict each other. Rust is very nice to write stable, fast and well tested libraries in that can interface with C code in.

Learning Rust and with its concepts improved my C code. Even if Rust would vanish over night I wouldn't regret learning it.

People who know how to do that are expensive, and they still make mistakes. Rust enables junior-level JavaScript programmers to write kernel/bare-metal level code without fear of making these kinds of errors. If you've spent a career programming in C and you're complaining about Rust, you're right. Rust isn't for you, it's for your replacement.

> 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.

I appreciate this comment, because it demonstrates a solid approach to simplify the code.

That said, regarding:

> safe-language-trend-of-the-day.

I agree that rust advocacy can sometimes be a bit misguided and over-enthusiastic - however how often is an out of bounds write not a bug (or a too clever by far hack)?

We've had pretty efficient ways to deal with this in c like languages for a long time (eg Pascal, Ada).

(c-like in the sense of being relatively low-overhead, close to the hardware wrt memory layout etc).

The entire raison d'etre of Rust is that it can be safer in exactly these cases where C or C++ is unsafe, and aims towards their eventual replacement.

This will take a couple of decades, but it's a worthwhile effort.

This is userspace code, it's a legit point to criticise use of a memory-unsafe language here. It's 2020 after all, 24 years after "Smashing the stack for fun and profit" and 24 years into the golden age of C exploits while safer practical systems languages have existed. And also a legit point to promote Rust, even if it's a little too new for this codebase.

I mean, they did link to a bunch of source code spots... so it's not completely unconstructive.

...especially when the large majority of HN readers are already aware of rust "thanks" to the rust evangelism strikeforce.

I've said nowhere that all of Android should be written in Rust. I've only said that had this specific component been written in Rust, the issues wouldn't have shown up. Of course the code base is old and predates Rust.

But I think this vulnerability serves as an important lesson about which language to choose for new projects in the embedded area. Thus I'm very glad that Google uses Rust for its new OpenSK security key firmware. I hope that future versions of Android will adopt Rust, at least in newly written components. Some Google developed Android related projects are already using Rust, like Cuttlefish which uses crosvm.

Since Android includes a deeply integrated Java implementation that they've put a lot of work into, which includes an AOT compiler, why wouldn't they write services like this one in Java?

The point is that the sane, correct choice is also the easy, default choice in Rust. We've been able to implement (ptr,len) buffer types in C for as long as we've had C, but uint8_t* is both baked into many APIs, and the path of least resistance.

We've spent decades pushing the limits of security improvements we can get through asking people to please try harder and do better with C, but we still see a high rate of high-impact errors like this.

Rust's safety model isn't the only valuable thing about Rust. Another big valuable part of Rust is that instead of giving the programmer a box of unsafe tools and a post-it reminding them to be careful, Rust provides sane, safe default tools that have been built based on what we've learned from the past several decades.

The argument isn't "Only Rust can save you", but that Rust is a good choice that both meets the same performance requirements, and avoids these problems by default.

If you've got a better solution to persuade C and C++ developers to consistently and reliably always wrap their use of pointers from other APIs into (ptr,len) buffer types, I'd love to hear it!

With comments like this, I sometimes feel like many developers are much more interested in smugly dismissing a group that's made significant real-world progress in making it easy to do the right thing than they are in actually helping real developers to reliably make safer software today.