Potential unsoundness in PyBytesWriter?

[cdhowie]

I want to ask here before making a bug report about this, but I think byteswriter::PyBytesWriter is unsound, specifically the std::io::Write::write_vectored implementation. These lines in particular:

pyo3/src/byteswriter.rs

Lines 157 to 161 in e8566a9

	// SAFETY: We ensure enough capacity below.
	let mut pos = unsafe { self.as_mut_ptr().add(self.len()) };
	// SAFETY: We write the new uninitialized bytes below.
	unsafe { self.set_len(self.len() + len)? }

self.as_mut_ptr() calls PyBytesWriter_GetData(), and self.set_len() calls PyBytesWriter_Resize().

The docs for PyBytesWriter_GetData state:

The pointer is valid until PyBytesWriter_Finish() or PyBytesWriter_Discard() is called on writer.

However, I'm kind of at a loss to explain how PyBytesWriter_Resize() could be guaranteed to never invalidate the pointer returned by PyBytesWriter_GetData(). If it needs to reallocate to grow the buffer, how would this not invalidate the previously-returned pointer?

If resizing can invalidate the pointer, then the Rust code is unsound as it contains a potential use-after-free. This could be easily fixed by swapping the order of the two lines of code above (which would require grabbing self.len() before resizing so pos can be correctly computed).

This smells like a doc bug on the Python side as well (they should note that PyBytesWriter_Resize() can also invalidate the pointer assuming that PyBytesWriter_Resize() can realloc).

Or is there some reason that this is definitely sound that I'm not seeing?

[alex]

I think your analysis is right -- the reason the test doesn't catch this is that for values <256 there's a small buffer allocated inline in the PyBytesWriter, and the test case's data fits within that:

pyo3/src/byteswriter.rs

Line 244 in e8566a9

fn test_io_write_vectored() {

If you extended that to write 1Kb of data it'd fail.

[cdhowie]

Thanks for the confirmation. I'm happy to file a bug report against this repo to track the issue, though a bug should probably be filed against the Python docs as well and I'm not sure where to do that.

[alex]

https://github.com/python/cpython/issues

[cdhowie]

Thanks, I'll look at filing an issue there.

Created #5906.