Skip to content

Commit 8e9cfab

Browse files
committed
Add feature type check in lender closing issue #9
1 parent fe176e2 commit 8e9cfab

File tree

5 files changed

+54
-20
lines changed

5 files changed

+54
-20
lines changed

src/error.rs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ pub enum PointerError {
1616
/// A pointer that was not previously lent to the FFI user.
1717
#[cfg(all(feature = "std", feature = "lender"))]
1818
Invalid,
19+
/// A pointer previously lent but the type is not the same.
20+
#[cfg(all(feature = "std", feature = "lender"))]
21+
InvalidType,
1922
/// Trying to convert to `&str` a C string which content is not valid UTF-8.
2023
Utf8Error(Utf8Error),
2124
/// Trying to alloc memory, see [`alloc::collections::TryReserveError`].
@@ -28,6 +31,11 @@ impl core::fmt::Display for PointerError {
2831
match self {
2932
Self::Null => write!(f, "dereference a null pointer will produce a crash"),
3033
Self::Invalid => write!(f, "dereference a unknown pointer could produce a crash"),
34+
#[cfg(all(feature = "std", feature = "lender"))]
35+
Self::InvalidType => write!(
36+
f,
37+
"dereference a pointer with a different type could produce errors"
38+
),
3139
Self::Utf8Error(error) => write!(
3240
f,
3341
"the provided C string is not a valid UTF-8 string: {error}"
@@ -44,7 +52,7 @@ impl core::fmt::Display for PointerError {
4452
impl std::error::Error for PointerError {
4553
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
4654
match self {
47-
Self::Null | Self::Invalid => None,
55+
Self::Null | Self::Invalid | Self::InvalidType => None,
4856
Self::Utf8Error(error) => Some(error),
4957
Self::TryReserveError(error) => Some(error),
5058
}

src/lender.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
#![cfg(all(feature = "std", feature = "lender"))]
22

33
use lazy_static::lazy_static;
4-
use std::collections::HashSet;
4+
use std::any::TypeId;
5+
use std::collections::HashMap;
56
use std::sync::{RwLock, RwLockWriteGuard};
67

78
use crate::error::PointerError;
89

910
lazy_static! {
10-
static ref LENT_POINTERS: RwLock<HashSet<usize>> = RwLock::new(HashSet::new());
11+
static ref LENT_POINTERS: RwLock<HashMap<usize, TypeId>> = RwLock::new(HashMap::new());
1112
}
1213

1314
/// Check if a pointer was [`lent`](lend).
@@ -17,12 +18,13 @@ lazy_static! {
1718
/// If the [`RwLock`] used is poisoned, but it only happens if a panic happens
1819
/// while holding it. And it's specially reviewed and in a small module to
1920
/// avoid panics while holding it.
20-
pub(super) fn is_lent<T>(pointer: *const T) -> bool {
21+
pub(super) fn lent_type_of<T>(pointer: *const T) -> Option<TypeId> {
2122
let Ok(lent_pointers) = LENT_POINTERS.read() else {
2223
log::error!("RwLock poisoned, it is not possible to check pointers");
2324
unreachable!();
2425
};
25-
return lent_pointers.contains(&(pointer as usize));
26+
27+
lent_pointers.get(&(pointer as usize)).copied()
2628
}
2729

2830
/// Use only when lend memory as a [`raw`](crate::raw) pointer.
@@ -32,15 +34,16 @@ pub(super) fn is_lent<T>(pointer: *const T) -> bool {
3234
/// If the [`RwLock`] used is poisoned, but it only happens if a panic happens
3335
/// while holding it. And it's specially reviewed and in a small module to
3436
/// avoid panics while holding it.
35-
pub(super) fn lend<T>(pointer: *const T) -> Result<(), PointerError> {
37+
pub(super) fn lend<T: 'static>(pointer: *const T) -> Result<(), PointerError> {
3638
let mut lent_pointers = writable_lent_pointers();
3739

3840
if let Err(error) = lent_pointers.try_reserve(1) {
3941
log::error!("Can not alloc memory to lent a pointer: {error}");
4042
return Err(PointerError::from(error));
4143
}
42-
lent_pointers.insert(pointer as usize);
43-
return Ok(());
44+
45+
lent_pointers.insert(pointer as usize, TypeId::of::<T>());
46+
Ok(())
4447
}
4548

4649
/// Use only when [`own_back`](crate::own_back) memory.
@@ -54,7 +57,7 @@ pub(super) fn retrieve<T>(pointer: *const T) {
5457
writable_lent_pointers().remove(&(pointer as usize));
5558
}
5659

57-
fn writable_lent_pointers() -> RwLockWriteGuard<'static, HashSet<usize>> {
60+
fn writable_lent_pointers() -> RwLockWriteGuard<'static, HashMap<usize, TypeId>> {
5861
let Ok(lent_pointers) = LENT_POINTERS.write() else {
5962
log::error!("RwLock poisoned, it is not possible to add or remove pointers");
6063
unreachable!();

src/lib.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ mod validation;
3232
/// If the allocator reports a failure, then an error is returned.
3333
#[cfg(any(feature = "alloc", feature = "std"))]
3434
#[inline]
35-
pub fn raw<T>(data: T) -> Result<*mut T, PointerError> {
35+
pub fn raw<T: 'static>(data: T) -> Result<*mut T, PointerError> {
3636
let pointer = Box::into_raw(Box::new(data));
3737

3838
#[cfg(all(feature = "std", feature = "lender"))]
@@ -53,13 +53,16 @@ pub fn raw<T>(data: T) -> Result<*mut T, PointerError> {
5353
///
5454
/// The pointer must be not null as it is an obvious invalid pointer.
5555
///
56+
/// Also, the type must be the same as the original.
57+
///
5658
/// # Safety
5759
///
5860
/// Invalid pointer or call it twice could cause an undefined behavior or heap error and a crash.
5961
#[cfg(any(feature = "alloc", feature = "std"))]
6062
#[inline]
6163
#[allow(clippy::not_unsafe_ptr_arg_deref)]
62-
pub unsafe fn own_back<T>(pointer: *mut T) -> Result<T, PointerError> {
64+
pub unsafe fn own_back<T: 'static>(pointer: *mut T) -> Result<T, PointerError> {
65+
validation::not_null_pointer(pointer)?;
6366
validation::lent_pointer(pointer)?;
6467
let boxed = { Box::from_raw(pointer) };
6568

@@ -84,7 +87,7 @@ pub unsafe fn object<'a, T>(pointer: *const T) -> Result<&'a T, PointerError> {
8487
Ok(&*pointer)
8588
}
8689

87-
/// Mutable reference to a object but without back to own it.
90+
/// Mutable reference to an object but without back to own it.
8891
///
8992
/// # Errors
9093
///

src/validation.rs

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,17 @@ pub fn not_null_pointer<T>(pointer: *const T) -> Result<(), PointerError> {
1313
}
1414

1515
#[inline]
16-
pub fn lent_pointer<T>(pointer: *const T) -> Result<(), PointerError> {
17-
not_null_pointer(pointer)?;
18-
16+
pub fn lent_pointer<T: 'static>(pointer: *const T) -> Result<(), PointerError> {
1917
#[cfg(all(feature = "std", feature = "lender"))]
20-
if !lender::is_lent(pointer) {
21-
log::error!("Using an invalid pointer as an opaque pointer to Rust's data");
22-
return Err(PointerError::Invalid);
18+
match lender::lent_type_of(pointer) {
19+
Some(type_id) if type_id != std::any::TypeId::of::<T>() => {
20+
log::error!("Using a pointer with a different type as an opaque pointer to Rust's data");
21+
Err(PointerError::InvalidType)
22+
}
23+
None => {
24+
log::error!("Using an invalid pointer as an opaque pointer to Rust's data");
25+
Err(PointerError::Invalid)
26+
}
27+
_ => Ok(()),
2328
}
24-
25-
Ok(())
2629
}

tests/pointer.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use opaque_pointer;
22
use opaque_pointer::error::PointerError;
33

4+
45
#[derive(Debug)]
56
struct TestIt {
67
value: u8,
@@ -18,6 +19,7 @@ impl TestIt {
1819
}
1920
}
2021

22+
2123
#[test]
2224
fn own_back() {
2325
let pointer = opaque_pointer::raw(TestIt::new(2)).unwrap();
@@ -36,6 +38,20 @@ fn own_back_invalid_pointer() {
3638
);
3739
}
3840

41+
#[cfg(all(feature = "std", feature = "lender"))]
42+
#[test]
43+
fn own_back_invalid_type() {
44+
let pointer = opaque_pointer::raw(TestIt::new(2)).unwrap() as *mut u8;
45+
let error = unsafe { opaque_pointer::own_back(pointer).unwrap_err() };
46+
assert_eq!(
47+
error,
48+
PointerError::InvalidType
49+
);
50+
51+
unsafe { opaque_pointer::own_back(pointer as *mut TestIt).unwrap() };
52+
}
53+
54+
3955
#[test]
4056
fn immutable_reference() {
4157
let pointer = opaque_pointer::raw(TestIt::new(2)).unwrap();
@@ -45,6 +61,7 @@ fn immutable_reference() {
4561
unsafe { opaque_pointer::own_back(pointer).unwrap() };
4662
}
4763

64+
4865
#[test]
4966
fn mutable_reference() {
5067
let pointer = opaque_pointer::raw(TestIt::new(2)).unwrap();

0 commit comments

Comments
 (0)