Conversation
There was a problem hiding this comment.
I like the idea to insist on this point 💯
We could maybe even mention that in the rare case where ptr::read is needed, then:
-
either use a pre-emptive
ManuallyDropof the original value; -
or
ptr::write-overwrite the original value before it is used. This last case should be handled with great caution, though, given how minimal exception safety / unwind safety may be broken otherwise (e.g., a panic before thewritewill still lead to a double drop; thus the write should happen in a "finally" like block, such as one from::scopeguard(or usingcatch_unwind)); code snippets of https://docs.rs/replace_with being very educational in that regard.
I still think the suggested documentation should be a bit more harsh as to how bad misusing this function is, given how many things can go wrong, and how wrong that can be (UAF / double free can cause remote code execution), hence my request for changes 😉
- (Note that I am just a random user, I am not part of the maintainers of the repo).
Co-authored-by: Daniel Henry-Mantilla <daniel.henry.mantilla@gmail.com>
|
Okay, so finally I should have took last reviews into account (with a lot of commits, sorry about that...). |
| fn main(){ | ||
| let obj: MyStruct = MyStruct(100); | ||
| let ptr: *const MyStruct = &test as *const MyStruct; | ||
| println!("{:?} @ {:p}", unsafe { std::ptr::read(ptr) }, ptr); | ||
| } |
There was a problem hiding this comment.
Donner le résultat obtenu après exécution du programme
| ## Déplacement de valeurs | ||
|
|
||
| Rust propose trois différents modes de déplacement de valeur: | ||
|
|
||
| - Soit par *déplacement*, qui est le comportement par défaut. | ||
| - Ou par *déplacement* plus un *drop* si `mem::needs_drop::<T>()` renvoie `true`. | ||
| - Ou par *copie*, si son type implémente le trait `Copy` | ||
|
|
||
| Cependant, des problèmes peuvent être constater lors de l'utilisation de la fonction `std::ptr::read`. | ||
| Selon la [documentation](https://doc.rust-lang.org/std/ptr/fn.read.html), cette fonction: | ||
| > Lis la valeur pointée par src sans la déplacer. Ce qui laisse la mémoire pointée intact. | ||
|
|
||
| Cette fonction est donc responsable d'effectuer une copie de la valeur pointée, indépendamment du mode de déplacement du type en question. | ||
| Ce comportement peut être dangereux car il peut mener à des *double-free* et/ou des *double-drop*. | ||
|
|
||
| Pour illustrer ce comportement, considérons le code suivant : | ||
|
|
||
| ```rust | ||
| # use std::ops::Drop; | ||
| # | ||
| #[derive(Debug)] | ||
| struct MyStruct(u8); | ||
|
|
||
| impl Drop for MyStruct { | ||
| fn drop(&mut self) { | ||
| # println!("---Dropping an object---\nBefore zeroing: {} @ {:p}", self.0, &self.0 as *const u8); | ||
| self.0 = 0; | ||
| # println!("After zeroing: {} @ {:p}", self.0, &self.0 as *const u8); | ||
| } | ||
| } | ||
|
|
||
| fn main(){ | ||
| let obj: MyStruct = MyStruct(100); | ||
| let ptr: *const MyStruct = &test as *const MyStruct; | ||
| println!("{:?} @ {:p}", unsafe { std::ptr::read(ptr) }, ptr); | ||
| } | ||
| ``` | ||
|
|
||
| On peut observer qu'un deuxième objet a implicitement été créé lors de l'appel à `std::ptr::read`, i.e. une copie d'un objet *non copiable* est effectuée. | ||
|
|
||
| Cela peut poser différents problèmes : | ||
|
|
||
| - Dans certains _rares_ cas (pas de _drop glue_, ni de guarantie de non-aliasing, ni d'invariants de sûreté reliés au type; i.e. quand le type aurait dû être `Copy`), cela peut être sans danger. | ||
|
|
||
| - Dans la plupart des cas, il pourrait y avoir des invariants de sûreté ou de validité (tel que l'aliasing de pointeur) ce qui rend une instance `unsafe` à utilisé, voire même UB, au moment où l'autre instance est utilisée. | ||
|
|
||
| - Exemple: | ||
|
|
||
| ```rust,no_run | ||
| # use ::core::mem; | ||
| # | ||
| let box_1: Box<i32> = Box::new(42); | ||
| let at_box_1: *const Box<i32> = &box_1; | ||
| let box_2: Box<i32> = unsafe { at_box_1.read() }; | ||
| mem::forget(box_1); // `Box` non-aliasing guarantees invalidates the pointer in `box_2` | ||
| drop(box_2); // UB: "usage" of invalidated pointer. | ||
| ``` | ||
| Cf. [Stacked Borrows](https://plv.mpi-sws.org/rustbelt/stacked-borrows/) pour plus d'informations. | ||
|
|
||
| - Si le type doit être *drop* (i.e. `mem::needs_drop::<T>()` renvoie vraie), alors quand une des deux instances est *drop*, toute utilisation de l'autre peu causer un *use-after-free*, l'exemple le plus notable étant que la seconde instance sera également *drop*, ce qui causera un *double-free*. | ||
|
|
||
| **Ceci est un Comportement Non Défini (*Undefined Behavior*), et est responsable de vulnérabilités majeures**. | ||
|
|
||
| Le code suivant illustre ce problème : | ||
|
|
||
| ```rust | ||
| # use std::boxed::Box; | ||
| # use std::ops::Drop; | ||
| # | ||
| #[derive(Debug)] | ||
| struct MyStructBoxed(Box<u8>); | ||
|
|
||
| impl Drop for MyStructBoxed { | ||
| fn drop(&mut self) { | ||
| # println!("---Dropping an object---\nBefore zeroing: {} @ {:p}", self.0, self.0); | ||
| let value: &mut u8 = self.0.as_mut(); | ||
| *value = 0; | ||
| # println!("After zeroing: {} @ {:p}", self.0, self.0); | ||
| } | ||
| } | ||
|
|
||
| fn main(){ | ||
| let test: MyStructBoxed = MyStructBoxed(Box::new(100)); | ||
| let ptr: *const MyStructBoxed = &test as *const MyStructBoxed; | ||
| println!("{:?} @ {:p}", unsafe { std::ptr::read(ptr) }, unsafe { &*ptr }.0 ); | ||
| } | ||
| ``` |
There was a problem hiding this comment.
Je pense que cette partie n'est pas nécessaire à la justification de la règle. En effet, un pointeur raw n'est, dans la pratique, pas utilisable sans unsafe.
hg-anssi
left a comment
hg-anssi
left a comment
There was a problem hiding this comment.
Thank you for your contribution. Even if the rule may be OK, the justification could be simplified and clarified.
Co-authored-by: hg-anssi <211649438+hg-anssi@users.noreply.github.com>
Co-authored-by: hg-anssi <211649438+hg-anssi@users.noreply.github.com>
|
Thanks for the review and the comments, I'll try to have a look at simplifying the rule. |
5 participants
During an internship studying Rust compilation phase, I encounter this problem with
std::ptr::readfunction.As I read this guide, I thought I could contribute to it.
So this is a presentation of my contribution to this guide (english and french).
I extended the chapter 4 of the book, and added a rule about the usage of the
std::ptr::readfunction.As explained, a usage with a type implementing the
Droptrait could lead to double-drop and possibly a double-free if the value is stored on the heap and freed during drop execution.This kind of contribution should have followed an issue, but I have been told that it would be easier for maintainers to directly go for a pull request. So here it is! 😄