Skip to content

Commit 654cc58

Browse files
Icxoludavidhewitt
andauthored
fix possible type confusion in PyClassGuardMut::as_super (#6104)
* fix possible type confusion in `PyClassGuardMut::as_super` * Update src/pyclass/guard.rs --------- Co-authored-by: David Hewitt <mail@davidhewitt.dev>
1 parent 9cc7862 commit 654cc58

4 files changed

Lines changed: 65 additions & 3 deletions

File tree

newsfragments/6104.added.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
added `PyClassGuardMapSuper`

newsfragments/6104.fixed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
fixed soundness issue caused by exposing a mutable reference from `PyClassGuardMut::as_super` by moving into `PyClassGuardMutSuper`

src/pyclass.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ pub(crate) use self::create_type_object::{create_type_object, PyClassTypeObject}
1111
pub use self::gc::{PyTraverseError, PyVisit};
1212
pub use self::guard::{
1313
PyClassGuard, PyClassGuardError, PyClassGuardMap, PyClassGuardMut, PyClassGuardMutError,
14+
PyClassGuardMutSuper,
1415
};
1516

1617
/// Types that can be used as Python classes.

src/pyclass/guard.rs

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -701,9 +701,15 @@ where
701701
/// super-superclass (and so on).
702702
///
703703
/// See [`PyClassGuard::as_super`] for more.
704-
pub fn as_super(&mut self) -> &mut PyClassGuardMut<'a, T::BaseType> {
705-
// SAFETY: `PyClassGuardMut<T>` and `PyClassGuardMut<U>` have the same layout
706-
unsafe { NonNull::from(self).cast().as_mut() }
704+
///
705+
/// # Note
706+
///
707+
/// The mutable reference to the base type is not exposed directly, but through [`PyClassGuardMutSuper`].
708+
pub fn as_super(&mut self) -> PyClassGuardMutSuper<'_, 'a, T::BaseType> {
709+
PyClassGuardMutSuper {
710+
// SAFETY: `PyClassGuardMut<T>` and `PyClassGuardMut<U>` have the same layout
711+
guard: unsafe { NonNull::from(self).cast().as_mut() },
712+
}
707713
}
708714

709715
/// Gets a `PyClassGuardMut<T::BaseType>`.
@@ -842,6 +848,59 @@ impl From<PyClassGuardMutError<'_, '_>> for PyErr {
842848
}
843849
}
844850

851+
/// Wraps a borrowed mutable reference to the base class `T::BaseType` of a PyClass `T`
852+
///
853+
/// See [`PyClassGuardMut::as_super`]
854+
#[repr(transparent)]
855+
pub struct PyClassGuardMutSuper<'a, 'g, T: PyClass<Frozen = False>> {
856+
// NOTE: Exposing this directly from `PyClassGuardMut::as_super` would allow swapping this
857+
// `guard` with another guard of the same basetype but different subtype. That is unsound as the
858+
// original `PyClassGuardMut` allows access to the `T` stored inside. By wrapping the guard in a
859+
// new type which does not expose a mutable reference to the guard directly we ensure that this
860+
// swap is impossible.
861+
guard: &'a mut PyClassGuardMut<'g, T>,
862+
}
863+
864+
impl<'a, 'g, T> PyClassGuardMutSuper<'a, 'g, T>
865+
where
866+
T: PyClass<Frozen = False>,
867+
T::BaseType: PyClass<Frozen = False>,
868+
{
869+
/// Borrows a mutable reference to `PyClassGuardMut<T::BaseType>`.
870+
///
871+
/// See [`PyClassGuardMut::as_super`] for more.
872+
pub fn as_super(&mut self) -> PyClassGuardMutSuper<'a, 'g, T::BaseType> {
873+
PyClassGuardMutSuper {
874+
// SAFETY: `PyClassGuardMut<T>` and `PyClassGuardMut<U>` have the same layout
875+
guard: unsafe { NonNull::from(&mut *self.guard).cast().as_mut() },
876+
}
877+
}
878+
}
879+
880+
impl<T> Deref for PyClassGuardMutSuper<'_, '_, T>
881+
where
882+
T: PyClass<Frozen = False>,
883+
{
884+
type Target = T;
885+
886+
fn deref(&self) -> &T {
887+
// SAFETY: `PyClassObject<T>` contains a valid `T`, by construction no
888+
// alias is enforced
889+
unsafe { &*self.guard.as_class_object().get_ptr().cast_const() }
890+
}
891+
}
892+
893+
impl<T> DerefMut for PyClassGuardMutSuper<'_, '_, T>
894+
where
895+
T: PyClass<Frozen = False>,
896+
{
897+
fn deref_mut(&mut self) -> &mut Self::Target {
898+
// SAFETY: `PyClassObject<T>` contains a valid `T`, by construction no
899+
// alias is enforced
900+
unsafe { &mut *self.guard.as_class_object().get_ptr() }
901+
}
902+
}
903+
845904
/// Wraps a borrowed mutable reference `U` to a value stored inside of a pyclass `T`
846905
///
847906
/// See [`PyClassGuardMut::map`]

0 commit comments

Comments
 (0)