Skip to content

Commit

Permalink
ffi: fix PyStatus._type
Browse files Browse the repository at this point in the history
The field wasn't defined previously. And the enum wasn't defined as
`[repr(C)]`.

This missing field could result in memory corruption if a Rust-allocated
`PyStatus` was passed to a Python API, which could perform an
out-of-bounds write. In my code, the out-of-bounds write corrupted a
variable on the stack, leading to a segfault due to illegal memory
access. However, this crash only occurred on Rust 1.54! So I initially
mis-attribted it as a compiler bug / regression. It appears that a
low-level Rust change in 1.54.0 changed the LLVM IR in such a way to
cause LLVM optimization passes to produce sufficiently different
assembly code, tickling the crash. See
rust-lang/rust#87947 if you want to see
the wild goose chase I went on in Rust / LLVM land to potentially
pin this on a compiler bug.

Lessen learned: Rust crashes are almost certainly due to use of
`unsafe`.
  • Loading branch information
indygreg committed Aug 14, 2021
1 parent d69912b commit b5b211f
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.

- Restrict FFI definitions `PyGILState_Check` and `Py_tracefunc` to the unlimited API. [#1787](https://github.com/PyO3/pyo3/pull/1787)
- Raise `AttributeError` to avoid panic when calling `del` on a `[setter]` defined class property. [#1775](https://github.com/PyO3/pyo3/issues/1775)
- Add missing `_type` field to `PyStatus` struct definition.

## [0.14.2] - 2021-08-09

Expand Down
2 changes: 2 additions & 0 deletions src/ffi/cpython/initconfig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::ffi::Py_ssize_t;
use libc::wchar_t;
use std::os::raw::{c_char, c_int, c_ulong};

#[repr(C)]
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum _PyStatus_TYPE {
_PyStatus_TYPE_OK = 0,
Expand All @@ -14,6 +15,7 @@ pub enum _PyStatus_TYPE {
#[repr(C)]
#[derive(Copy, Clone)]
pub struct PyStatus {
pub _type: _PyStatus_TYPE,
pub func: *const c_char,
pub err_msg: *const c_char,
pub exitcode: c_int,
Expand Down

0 comments on commit b5b211f

Please sign in to comment.