5 ms·

> which is a pointer miscalculation in network backend of QEMU Out of interest does Rust prevent this kind of mistake?

by tus88 7y ago

lewisinc 7y ago
Possibly, by default yeah, but with `unsafe` blocks you can dereference raw pointers, which can theoretically land you in this same situation. But you'd have to opt into that possibility, and you'd be very aware of that.
jchw 7y ago
Yes: in fact, any memory-safe language can guarantee that a “safe” program would not have this class of bug. In languages without pointer arithmetic and with bounds checking, this type of bug just isn’t possible. Not only does this include Rust but also Java, Go, and a fairly large list of other programming languages. Most of what Rust improves on is related to memory ownership and concurrency - we’ve had ways to prevent this class of problem for a long time, and in fact there are even C variants that can do that too.
geofft 7y ago
Yes. At a surface level, the bug is in doing raw pointer arithmetic: determining the size of some value by subtracting two pointers from each other, incorrectly assuming that both pointers are within the same object. There's a codepath where one is not, and therefore this size computation is incorrect. Later, that size is added to another pointer, allowing for out-of-bounds access, overwriting other variables. Rust doesn't let you subtract two pointers from each other. Even unsafe Rust does not; you'd have to cast the pointers to integers, first, because finding the difference between two unrelated pointers is a fundamentally meaningless operation. (Indeed, it's undefined behavior in C, and Rust compiles through LLVM and would inherit the same optimization passes that wish to consider things UB, so it doesn't pass a request through to LLVM that's going to be undefined.) And safe Rust doesn't let you index to an arbitrary spot in an array / buffer without a bounds check, so even if you got a nonsense offset, it would crash instead of overwriting unrelated values. At a slightly higher level, it seems like the underlying issue here (if I'm reading the article right) is that struct mbuf has two ways of representing the data: the array member m_dat and the pointer m_ext. Which one you're supposed to use is represented by a flag. The code correctly kept track of which one to use in all cases except one. Entirely apart from the memory safety stuff, Rust gives you tagged enums (enums with data, aka "sum types" in functional programming) with the property that you can only access data inside a particular enum variant if the variable you're looking at is actually of that variant. So, for instance, you could have something roughly like: enum MData { Internal(buffer: [u8; 32]), External(ptr: &[u8]), } and syntactically there's no way to get a ptr out of an Internal or a buffer out of an External, so you couldn't have the logic confusion that led up to the memory unsafety. Even if you could do raw pointer arithmetic in Rust, you'd still get it right: let delta = match mbuf.data { Internal(buffer) => q - &buffer, External(ptr) => q - ptr, } so it's impossible to forget to check the flag. (In this case they do check the flag but it sounds like they're not checking the right flag or something? Or the flag is set too early? I don't totally follow the description, but if it's something like that, using a Rust enum would guarantee that the "flag" accurately matches whatever you're looking at.) The memory safety stuff is great, but I really think that having a richer type system like this is more fundamentally what prevents bugs, compared to C where all you have is numbers, pointers, structures, and structures-where-things-overlap. (Another good use of this is nullable pointers that force you to do null checks before dereferencing them, and a little more broadly, this pattern also gives you locked data that forces you to take the lock before dereferencing the data, avoiding issues where you take the wrong lock, which could end up as memory unsafety eventually.) FWIW there are a few hypervisor projects in the same space as QEMU that are written in Rust: AWS's Firecracker and Chrome's crosvm come to mind.
- saagarjha 7y ago
  I'm actually curious why the project chose to use two fields in the structure to store the buffers instead of a union (not that C's untagged union would have helped much here).
- tsimionescu 7y ago
  > The memory safety stuff is great, but I really think that having a richer type system like this is more fundamentally what prevents bugs I know this is only tangentially related to your point, but all the research I have read about points to the opposite conclusion - richer and stricter type systems don't have a proven effect on bugs, whereas memory safety is guaranteed to eliminate whole classes of bugs. One guess why this would be true, despite your good example of a type system-level fix that would have prevented this bug entirely, is that memory safety is automatic (or at least opt-out), while the richer types are opt-in: nothing in Rust (or Java etc) would have prevented writing the original C, the programmer would have had to think about using the enum or other equivalent in order for the compiler to help. Granted, in this case they would almost certainly have done so, as enums are just so much nicer than flag checking, but for other type-level solutions the same may not happen.
  - geofft 7y ago
    Interesting, I'm very curious if you have pointers to this! Intuitively I feel like I write better code with a better type system, but that argument makes sense to me :-( One thing I'm curious about is if there's a slightly different property than "richer type system" that helps. In Rust but not necessarily in other languages with good type systems, you can't leave a struct member uninitialized, so perhaps the easiest way to write this code is actually to use an enum if you know you'll only ever use one or the other. That might be close enough to the "automatic" property?
- hummerbliss 7y ago
  Am genuinely curious. Why can't C language be extended to do this ?
  - 7y ago

> which is a pointer miscalculation in network backend of QEMU Out of interest does Rust prevent this kind of mistake?

by tus88 7y ago

> which is a pointer miscalculation in network backend of QEMU Out of interest does Rust prevent this kind of mistake?

Possibly, by default yeah, but with `unsafe` blocks you can dereference raw pointers, which can theoretically land you in this same situation. But you'd have to opt into that possibility, and you'd be very aware of that.

jchw 7y ago

Yes: in fact, any memory-safe language can guarantee that a “safe” program would not have this class of bug. In languages without pointer arithmetic and with bounds checking, this type of bug just isn’t possible. Not only does this include Rust but also Java, Go, and a fairly large list of other programming languages. Most of what Rust improves on is related to memory ownership and concurrency - we’ve had ways to prevent this class of problem for a long time, and in fact there are even C variants that can do that too.

geofft 7y ago

Yes. At a surface level, the bug is in doing raw pointer arithmetic: determining the size of some value by subtracting two pointers from each other, incorrectly assuming that both pointers are within the same object. There's a codepath where one is not, and therefore this size computation is incorrect. Later, that size is added to another pointer, allowing for out-of-bounds access, overwriting other variables. Rust doesn't let you subtract two pointers from each other. Even unsafe Rust does not; you'd have to cast the pointers to integers, first, because finding the difference between two unrelated pointers is a fundamentally meaningless operation. (Indeed, it's undefined behavior in C, and Rust compiles through LLVM and would inherit the same optimization passes that wish to consider things UB, so it doesn't pass a request through to LLVM that's going to be undefined.) And safe Rust doesn't let you index to an arbitrary spot in an array / buffer without a bounds check, so even if you got a nonsense offset, it would crash instead of overwriting unrelated values. At a slightly higher level, it seems like the underlying issue here (if I'm reading the article right) is that struct mbuf has two ways of representing the data: the array member m_dat and the pointer m_ext. Which one you're supposed to use is represented by a flag. The code correctly kept track of which one to use in all cases except one. Entirely apart from the memory safety stuff, Rust gives you tagged enums (enums with data, aka "sum types" in functional programming) with the property that you can only access data inside a particular enum variant if the variable you're looking at is actually of that variant. So, for instance, you could have something roughly like: enum MData { Internal(buffer: [u8; 32]), External(ptr: &[u8]), } and syntactically there's no way to get a ptr out of an Internal or a buffer out of an External, so you couldn't have the logic confusion that led up to the memory unsafety. Even if you could do raw pointer arithmetic in Rust, you'd still get it right: let delta = match mbuf.data { Internal(buffer) => q - &buffer, External(ptr) => q - ptr, } so it's impossible to forget to check the flag. (In this case they do check the flag but it sounds like they're not checking the right flag or something? Or the flag is set too early? I don't totally follow the description, but if it's something like that, using a Rust enum would guarantee that the "flag" accurately matches whatever you're looking at.) The memory safety stuff is great, but I really think that having a richer type system like this is more fundamentally what prevents bugs, compared to C where all you have is numbers, pointers, structures, and structures-where-things-overlap. (Another good use of this is nullable pointers that force you to do null checks before dereferencing them, and a little more broadly, this pattern also gives you locked data that forces you to take the lock before dereferencing the data, avoiding issues where you take the wrong lock, which could end up as memory unsafety eventually.) FWIW there are a few hypervisor projects in the same space as QEMU that are written in Rust: AWS's Firecracker and Chrome's crosvm come to mind.

saagarjha 7y ago
I'm actually curious why the project chose to use two fields in the structure to store the buffers instead of a union (not that C's untagged union would have helped much here).
tsimionescu 7y ago
> The memory safety stuff is great, but I really think that having a richer type system like this is more fundamentally what prevents bugs I know this is only tangentially related to your point, but all the research I have read about points to the opposite conclusion - richer and stricter type systems don't have a proven effect on bugs, whereas memory safety is guaranteed to eliminate whole classes of bugs. One guess why this would be true, despite your good example of a type system-level fix that would have prevented this bug entirely, is that memory safety is automatic (or at least opt-out), while the richer types are opt-in: nothing in Rust (or Java etc) would have prevented writing the original C, the programmer would have had to think about using the enum or other equivalent in order for the compiler to help. Granted, in this case they would almost certainly have done so, as enums are just so much nicer than flag checking, but for other type-level solutions the same may not happen.
- geofft 7y ago
  Interesting, I'm very curious if you have pointers to this! Intuitively I feel like I write better code with a better type system, but that argument makes sense to me :-( One thing I'm curious about is if there's a slightly different property than "richer type system" that helps. In Rust but not necessarily in other languages with good type systems, you can't leave a struct member uninitialized, so perhaps the easiest way to write this code is actually to use an enum if you know you'll only ever use one or the other. That might be close enough to the "automatic" property?
hummerbliss 7y ago
Am genuinely curious. Why can't C language be extended to do this ?
- 7y ago