From d3762a679ff7287350eb38478695e1e0b80742fa Mon Sep 17 00:00:00 2001 From: Gregory Szorc Date: Fri, 13 Aug 2021 20:51:26 -0700 Subject: [PATCH 1/2] ffi: fix PyStatus._type 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 https://github.com/rust-lang/rust/issues/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`. --- CHANGELOG.md | 1 + src/ffi/cpython/initconfig.rs | 2 ++ 2 files changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b1219cd09e..1acb18d97a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Restrict FFI definitions `PyGILState_Check` and `Py_tracefunc` to the unlimited API. [#1787](https://github.com/PyO3/pyo3/pull/1787) +- Add missing `_type` field to `PyStatus` struct definition. ## [0.14.2] - 2021-08-09 diff --git a/src/ffi/cpython/initconfig.rs b/src/ffi/cpython/initconfig.rs index ee6fc75b53d..95d28b02bf3 100644 --- a/src/ffi/cpython/initconfig.rs +++ b/src/ffi/cpython/initconfig.rs @@ -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, @@ -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, From 150f4adbba71adf673d71be18e8f3b4222a2c5ad Mon Sep 17 00:00:00 2001 From: David Hewitt <1939362+davidhewitt@users.noreply.github.com> Date: Sat, 14 Aug 2021 07:40:52 +0100 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1acb18d97a3..d16717dd603 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,7 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. ### Fixed - Restrict FFI definitions `PyGILState_Check` and `Py_tracefunc` to the unlimited API. [#1787](https://github.com/PyO3/pyo3/pull/1787) -- Add missing `_type` field to `PyStatus` struct definition. +- Add missing `_type` field to `PyStatus` struct definition. [#1791](https://github.com/PyO3/pyo3/pull/1791) ## [0.14.2] - 2021-08-09