Object.lower() can result in use-after-free errors

[bendk]

When an object is lowered from a foreign language we return the raw pointer from the foreign code then clone the Arc using the raw pointer in the Rust code.

How do we know that the pointer is still valid in that Rust code? For normal Rust calls, we can be sure since the a reference to the foreign object is still on the stack. However, for callbacks there are cases when this isn't true:

• Thread A returns the raw pointer, then gets interrupted (after the Python function returns, but before the Rust code runs).
• Thread B destroys the Rust object
• Thread A continues and calls from_raw on the freed raw pointer.

I also think there's a related case when Rust returns a callback interface or trait interface.

[bendk]

I think the general system we should use is that object handles are passed as borrows and returned as owned values. In this case, we should be cloning the handle before returning it.

This seems pretty intuitive to me. In Rust you can pass a reference in without any special handling, but returning a reference requires lifetime checks. You can't check the lifetime of an object on the other side of the FFI, so the rule has to be any handle that's returned is an owned value.

[bendk]

I think the above comment represents the ideal solution, but implementing that at this point would be quite complex. We could split lower() into lower_arg() and lower_return() in the foreign bindings, like the Rust code does. But then what about container objects that get lowered into Rust buffers? I think we would need to split write into write_arg and write_return which just feels like too much work to me.

I'm going to go with the simpler solution: always clone the reference in lower(). It's an extra FFI call in some circumstances, but I think we can live with that.

[arg0d]

I'm trying to understand your proposed solution.

What would be the ownership of handles in case of tail-returning foreign handle? If foreign bindings transfer ownership of foreign handle to Rust, then its up to Rust to free the foreign handle. If foreign bindings borrow foreign handle to Rust, then Rust doesn't have to free the handle, and foreign bindings will free the handle after function call. However, borrowing means the handle will go out of scope as soon as Rust function exits, so the handle can't be retained for any longer for callback-like use cases. How does cloning the handle in lower() solve the issue here?

123 = lower(PyButton())

123 = press(123) {
    // Rust
    foreign.free_handle(123)
    return 123
}

button = lift(123) // error: invalid 123

To me it sounds like what is actually needed here is to dedicate one handle bit to signal that its a tail-handle, and should be freed when lifting by foreign bindings themselves. This would create an option for Rust to conditionally either free the handle in the more usual case, or omit freeing the handle and instead set the tail bit for the handle. I'm not entirely sure how practical it would be detect this type of tail-return.