Skip to content

Update safety documentation for CString::from_ptr and str::from_boxed_utf8_unchecked #137714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 27, 2025

Conversation

DiuDiu777
Copy link
Contributor

@DiuDiu777 DiuDiu777 commented Feb 27, 2025

PR Description​

This PR addresses missing safety documentation for two APIs:

1. alloc::ffi::CStr::from_raw

  • Alias: The pointer ​must not be aliased​ (accessed via other pointers) during the reconstructed CString's lifetime.
  • Owning: Calling this function twice on the same pointer and creating two objects with overlapping lifetimes, introduces two alive owners of the same memory. This may result in a double-free.
  • Dangling: The prior documentation required the pointer to originate from CString::into_raw, but this constraint is incomplete. A validly sourced pointer can also cause undefined behavior (UB) if it becomes dangling. A simple Poc for this situation:
use std::ffi::CString;
use std::os::raw::c_char;

fn create_dangling() -> *mut c_char {
    let local_ptr: *mut c_char = {
        let valid_data = CString::new("valid").unwrap();
        valid_data.into_raw() 
    }; 

    unsafe { 
        let _x = CString::from_raw(local_ptr); 
    } 
    local_ptr
}

fn main() {
    let dangling = create_dangling();
    unsafe {let _y = CString::from_raw(dangling);} // Cause UB!
}

2. alloc::str::from_boxed_utf8_unchecked

  • ValidStr: Bytes must contain a ​valid UTF-8 sequence.

@rustbot
Copy link
Collaborator

rustbot commented Feb 27, 2025

r? @thomcc

rustbot has assigned @thomcc.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 27, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

/// obtained by calling [`CString::into_raw`]. Other usage (e.g., trying to take
/// ownership of a string that was allocated by foreign code) is likely to lead
/// to undefined behavior or allocator corruption.
/// obtained by calling [`CString::into_raw`] and this pointer must not be accessed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not the pointer, but the memory it points too

///
/// * The provided bytes must contain a valid UTF-8 sequence.
///
/// * The `Box<[u8]>` must have been allocated via the global allocator.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a safety requirement. Box<[u8]> is really Box<[u8], Global>, so passing a box with a custom allocator to this function is prevented by type check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you are right. I will fix these.

@DiuDiu777 DiuDiu777 requested a review from Sky9x March 3, 2025 12:04
Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nit and please squash, then lgtm

r? tgross35

@rustbot rustbot assigned tgross35 and unassigned thomcc Apr 27, 2025
@tgross35 tgross35 changed the title Doc fix Update safety documentation for CString::from_ptr and str::from_boxed_utf8_unchecked Apr 27, 2025
@rustbot
Copy link
Collaborator

rustbot commented Apr 27, 2025

Requested reviewer is already assigned to this pull request.

Please choose another assignee.

@tgross35
Copy link
Contributor

You should drop me as a coauthor from the commit message, suggesting a comma in review doesn't mean I had anything to do with authoring this patch :)

@DiuDiu777
Copy link
Contributor Author

DiuDiu777 commented Apr 27, 2025

Done :) @tgross35

@tgross35
Copy link
Contributor

Thanks!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Apr 27, 2025

📌 Commit bfdd947 has been approved by tgross35

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 27, 2025
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 27, 2025
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#137439 (Stabilise `std::ffi::c_str`)
 - rust-lang#137714 (Update safety documentation for `CString::from_ptr` and `str::from_boxed_utf8_unchecked`)
 - rust-lang#139031 (Use char::is_whitespace directly in str::trim*)
 - rust-lang#139090 (fix docs for `Peekable::next_if{_eq}`)
 - rust-lang#140297 (Update example to use CStr::to_string_lossy)
 - rust-lang#140330 (Clarified bootstrap optimization "true" argument)
 - rust-lang#140339 (session: Cleanup `CanonicalizedPath::new`)
 - rust-lang#140346 (rustc_span: Some hygiene cleanups)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit bd3af53 into rust-lang:master Apr 27, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 27, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 27, 2025
Rollup merge of rust-lang#137714 - DiuDiu777:doc-fix, r=tgross35

Update safety documentation for `CString::from_ptr` and `str::from_boxed_utf8_unchecked`

## PR Description​
This PR addresses missing safety documentation for two APIs:

​**1. alloc::ffi::CStr::from_raw**​

- ​`Alias`: The pointer ​must not be aliased​ (accessed via other pointers) during the reconstructed CString's lifetime.
- `Owning`: Calling this function twice on the same pointer and creating two objects with overlapping lifetimes, introduces two alive owners of the same memory. This may result in a double-free.
- `Dangling`: The prior documentation required the pointer to originate from CString::into_raw, but this constraint is incomplete. A validly sourced pointer can also cause undefined behavior (UB) if it becomes dangling. A simple Poc for this situation:
```
use std::ffi::CString;
use std::os::raw::c_char;

fn create_dangling() -> *mut c_char {
    let local_ptr: *mut c_char = {
        let valid_data = CString::new("valid").unwrap();
        valid_data.into_raw()
    };

    unsafe {
        let _x = CString::from_raw(local_ptr);
    }
    local_ptr
}

fn main() {
    let dangling = create_dangling();
    unsafe {let _y = CString::from_raw(dangling);} // Cause UB!
}
```

​**2. alloc::str::from_boxed_utf8_unchecked**​

- `ValidStr`: Bytes must contain a ​valid UTF-8 sequence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants