x86 Segmentation fault on depythonize #78

Activity

1. MarshalX commented on Dec 2, 2024

   

   MarshalX

   on Dec 2, 2024

   Last edited by MarshalX

   More actions

   You are right about depth. The deserialization is recursive, and there is no control of the depth at all. You get a segmentation fault because of stack overflow. The limit depends on the machine configuration. That's why it could pass in the environment with an ARM arch.

   Let's take a look at map deserialization:
   

   pythonize/src/de.rs

   Lines 286 to 291 in 4491cdb

   	fn deserialize_map<V>(self, visitor: V) -> Result<V::Value>
   	where
   	V: de::Visitor<'de>,
   	{
   	visitor.visit_map(self.dict_access()?)
   	}

   As we can see there is no depth counting or calling Py_EnterRecursiveCall + Py_LeaveRecursiveCall.

   Here is the example of how serde json fixed the same issue: https://github.com/serde-rs/json/pull/163/files (tl;dr count remaining_depth; hard limit to 128; if reach return error; do -1 before visit_map and +1 after visit_map)

   Speaking of the Py_EnterRecursiveCall + Py_LeaveRecursiveCall I used it in the exact same bug report in my lib. This is the best and smoothest option for the devs, but, from my testing, it gives 30% perf degradation. You can check it here: MarshalX/python-libipld#51

   @davidhewitt I will be happy to read your thoughts!
2. MarshalX commented on Mar 3, 2025

   

   MarshalX

   on Mar 3, 2025

   More actions

   upd. I decline to use Py_EnterRecursiveCall + Py_LeaveRecursiveCall because it is slow. I choose to count depth by myself and check it against Py_GetRecursionLimit to raise PyRecursionError