Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion crates/vm/src/stdlib/ctypes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@

pub(crate) mod array;
pub(crate) mod base;
pub(crate) mod field;
pub(crate) mod function;
pub(crate) mod library;
pub(crate) mod pointer;
pub(crate) mod structure;
pub(crate) mod thunk;
pub(crate) mod union;

use crate::builtins::PyModule;
Expand All @@ -17,14 +19,18 @@ pub fn extend_module_nodes(vm: &VirtualMachine, module: &Py<PyModule>) {
let ctx = &vm.ctx;
PyCSimpleType::make_class(ctx);
array::PyCArrayType::make_class(ctx);
field::PyCFieldType::make_class(ctx);
pointer::PyCPointerType::make_class(ctx);
extend_module!(vm, module, {
"_CData" => PyCData::make_class(ctx),
"_SimpleCData" => PyCSimple::make_class(ctx),
"Array" => array::PyCArray::make_class(ctx),
"CField" => field::PyCField::make_class(ctx),
"CFuncPtr" => function::PyCFuncPtr::make_class(ctx),
"_Pointer" => pointer::PyCPointer::make_class(ctx),
"_pointer_type_cache" => ctx.new_dict(),
"Structure" => structure::PyCStructure::make_class(ctx),
"CThunkObject" => thunk::PyCThunk::make_class(ctx),
"Union" => union::PyCUnion::make_class(ctx),
})
}
Expand Down Expand Up @@ -207,7 +213,9 @@ pub(crate) mod _ctypes {
// TODO: load_flags
let cache = library::libcache();
let mut cache_write = cache.write();
let (id, _) = cache_write.get_or_insert_lib(&name, vm).unwrap();
let (id, _) = cache_write
.get_or_insert_lib(&name, vm)
.map_err(|e| vm.new_os_error(e.to_string()))?;
Ok(id)
}

Expand Down
1 change: 1 addition & 0 deletions crates/vm/src/stdlib/ctypes/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ impl PyCArray {
}

impl PyCArray {
#[allow(unused)]
pub fn to_arg(&self, _vm: &VirtualMachine) -> PyResult<libffi::middle::Arg> {
let value = self.value.read();
let py_bytes = value.downcast_ref::<PyBytes>().unwrap();
Expand Down
27 changes: 14 additions & 13 deletions crates/vm/src/stdlib/ctypes/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,12 @@ impl Constructor for PyCSimple {
_ => vm.ctx.none(), // "z" | "Z" | "P"
}
};
Ok(PyCSimple {
PyCSimple {
_type_,
value: AtomicCell::new(value),
}
.to_pyobject(vm))
.into_ref_with_type(vm, cls)
.map(Into::into)
}
}

Expand Down Expand Up @@ -276,25 +277,25 @@ impl PyCSimple {
let value = unsafe { (*self.value.as_ptr()).clone() };
if let Ok(i) = value.try_int(vm) {
let i = i.as_bigint();
if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
return i.to_u8().map(|r: u8| libffi::middle::Arg::new(&r));
return if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
i.to_u8().map(|r: u8| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i8().as_raw_ptr()) {
return i.to_i8().map(|r: i8| libffi::middle::Arg::new(&r));
i.to_i8().map(|r: i8| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u16().as_raw_ptr()) {
return i.to_u16().map(|r: u16| libffi::middle::Arg::new(&r));
i.to_u16().map(|r: u16| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i16().as_raw_ptr()) {
return i.to_i16().map(|r: i16| libffi::middle::Arg::new(&r));
i.to_i16().map(|r: i16| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u32().as_raw_ptr()) {
return i.to_u32().map(|r: u32| libffi::middle::Arg::new(&r));
i.to_u32().map(|r: u32| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i32().as_raw_ptr()) {
return i.to_i32().map(|r: i32| libffi::middle::Arg::new(&r));
i.to_i32().map(|r: i32| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u64().as_raw_ptr()) {
return i.to_u64().map(|r: u64| libffi::middle::Arg::new(&r));
i.to_u64().map(|r: u64| libffi::middle::Arg::new(&r))
} else if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::i64().as_raw_ptr()) {
return i.to_i64().map(|r: i64| libffi::middle::Arg::new(&r));
i.to_i64().map(|r: i64| libffi::middle::Arg::new(&r))
} else {
return None;
}
None
};
Comment on lines +280 to +298
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Use-after-free: Arg::new references stack-local values that are immediately dropped.

libffi::middle::Arg::new(&r) captures a reference to r, but r is a local variable created within the closure passed to map. Once the closure returns, r is dropped, leaving a dangling reference inside the Arg. This is undefined behavior when the Arg is later used in an FFI call.

You need to store the converted values in a longer-lived location. Consider storing the value in a field or using a boxed/heap-allocated approach:

     pub fn to_arg(
         &self,
         ty: libffi::middle::Type,
         vm: &VirtualMachine,
-    ) -> Option<libffi::middle::Arg> {
+    ) -> Option<(Box<dyn std::any::Any>, libffi::middle::Arg)> {
         let value = unsafe { (*self.value.as_ptr()).clone() };
         if let Ok(i) = value.try_int(vm) {
             let i = i.as_bigint();
-            return if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
-                i.to_u8().map(|r: u8| libffi::middle::Arg::new(&r))
+            if std::ptr::eq(ty.as_raw_ptr(), libffi::middle::Type::u8().as_raw_ptr()) {
+                return i.to_u8().map(|r| {
+                    let boxed: Box<u8> = Box::new(r);
+                    let arg = libffi::middle::Arg::new(boxed.as_ref());
+                    (boxed as Box<dyn std::any::Any>, arg)
+                });
+            }
             // ... similar pattern for other types

Alternatively, refactor to return an owned representation that the caller can hold until the FFI call completes.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
crates/vm/src/stdlib/ctypes/base.rs around lines 280-298: the current code calls
libffi::middle::Arg::new(&r) with r as a stack-local created inside the map
closure, producing a dangling reference after the closure returns; replace this
by producing owned, longer-lived storage for the converted value (for example
allocate the converted primitive on the heap with Box::new(...) or push it into
a Vec owned by the returned structure) and create the Arg from a pointer to that
owned storage, or refactor the function to return an owned wrapper (e.g., a
struct holding the boxed values plus the libffi::Arg pointers) so the converted
values outlive the Arg until the FFI call completes.

}
if let Ok(_f) = value.try_float(vm) {
todo!();
Expand Down
125 changes: 125 additions & 0 deletions crates/vm/src/stdlib/ctypes/field.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
use crate::builtins::PyType;
use crate::builtins::PyTypeRef;
use crate::stdlib::ctypes::PyCData;
use crate::types::Constructor;
use crate::types::Representable;
use crate::{Py, PyResult, VirtualMachine};

#[pyclass(name = "PyCFieldType", base = PyType, module = "_ctypes")]
#[derive(PyPayload, Debug)]
pub struct PyCFieldType {
#[allow(dead_code)]
pub(super) inner: PyCField,
}

#[pyclass]
impl PyCFieldType {}

#[pyclass(
name = "CField",
base = PyCData,
metaclass = "PyCFieldType",
module = "_ctypes"
)]
#[derive(Debug, PyPayload)]
pub struct PyCField {
byte_offset: usize,
byte_size: usize,
#[allow(unused)]
index: usize,
proto: PyTypeRef,
anonymous: bool,
bitfield_size: bool,
bit_offset: u8,
name: String,
}

impl Representable for PyCField {
fn repr_str(zelf: &Py<Self>, _vm: &VirtualMachine) -> PyResult<String> {
let tp_name = zelf.proto.name().to_string();
if zelf.bitfield_size {
Ok(format!(
"<{} type={}, ofs={byte_offset}, bit_size={bitfield_size}, bit_offset={bit_offset}",
zelf.name,
tp_name,
byte_offset = zelf.byte_offset,
bitfield_size = zelf.bitfield_size,
bit_offset = zelf.bit_offset
))
} else {
Ok(format!(
"<{} type={tp_name}, ofs={}, size={}",
zelf.name, zelf.byte_offset, zelf.byte_size
))
}
}
}

#[derive(Debug, FromArgs)]
pub struct PyCFieldConstructorArgs {
// PyObject *name, PyObject *proto,
// Py_ssize_t byte_size, Py_ssize_t byte_offset,
// Py_ssize_t index, int _internal_use,
// PyObject *bit_size_obj, PyObject *bit_offset_obj
}

impl Constructor for PyCField {
type Args = PyCFieldConstructorArgs;

fn py_new(_cls: PyTypeRef, _args: Self::Args, vm: &VirtualMachine) -> PyResult {
Err(vm.new_type_error("Cannot instantiate a PyCField".to_string()))
}
}

#[pyclass(flags(BASETYPE, IMMUTABLETYPE), with(Constructor, Representable))]
impl PyCField {
#[pygetset]
fn size(&self) -> usize {
self.byte_size
}

#[pygetset]
fn bit_size(&self) -> bool {
self.bitfield_size
}

#[pygetset]
fn is_bitfield(&self) -> bool {
self.bitfield_size
}

#[pygetset]
fn is_anonymous(&self) -> bool {
self.anonymous
}

#[pygetset]
fn name(&self) -> String {
self.name.clone()
}

#[pygetset(name = "type")]
fn type_(&self) -> PyTypeRef {
self.proto.clone()
}

#[pygetset]
fn offset(&self) -> usize {
self.byte_offset
}

#[pygetset]
fn byte_offset(&self) -> usize {
self.byte_offset
}

#[pygetset]
fn byte_size(&self) -> usize {
self.byte_size
}

#[pygetset]
fn bit_offset(&self) -> u8 {
self.bit_offset
}
}
Loading
Loading