Double-inheriting from two buffer classes can cause corruption

[JelleZijlstra]

If you have a class that double-inherits from two buffer classes implemented in C, of which the first only implements bf_getbuffer and the second also implements bf_releasebuffer, then if you create a memoryview from the child class, on release the second class's bf_releasebuffer will be called, which may break that class's invariants.

Demonstration:

diff --git a/Modules/_testcapi/buffer.c b/Modules/_testcapi/buffer.c
index aff9a477ef..3cfa19a57b 100644
--- a/Modules/_testcapi/buffer.c
+++ b/Modules/_testcapi/buffer.c
@@ -89,14 +89,50 @@ static PyTypeObject testBufType = {
     .tp_members = testbuf_members
 };
 
+
+static int
+getbufferonly_getbuf(testBufObject *self, Py_buffer *view, int flags)
+{
+    PyObject *bytes = PyBytes_FromString("test");
+    if (bytes == NULL) {
+        return -1;
+    }
+    // Save a reference
+    if (PyObject_SetAttrString((PyObject *)Py_TYPE(self), "bytes", bytes) < 0) {
+        return -1;
+    }
+    int buf = PyObject_GetBuffer(bytes, view, flags);
+    view->obj = Py_NewRef(self);
+    return buf;
+}
+
+static PyBufferProcs getbufferonly_as_buffer = {
+    .bf_getbuffer = (getbufferproc) getbufferonly_getbuf,
+};
+
+static PyTypeObject getBufferOnlyType = {
+    PyVarObject_HEAD_INIT(NULL, 0)
+    .tp_name = "getBufferOnlyType",
+    .tp_basicsize = sizeof(PyObject),
+    .tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE,
+    .tp_new = PyType_GenericNew,
+    .tp_as_buffer = &getbufferonly_as_buffer,
+};
+
 int
 _PyTestCapi_Init_Buffer(PyObject *m) {
     if (PyType_Ready(&testBufType) < 0) {
         return -1;
     }
+    if (PyType_Ready(&getBufferOnlyType) < 0) {
+        return -1;
+    }
     if (PyModule_AddObjectRef(m, "testBuf", (PyObject *)&testBufType)) {
         return -1;
     }
+    if (PyModule_AddObjectRef(m, "getBufferOnly", (PyObject *)&getBufferOnlyType)) {
+        return -1;
+    }
 
     return 0;
 }

% ./python.exe 
Python 3.12.0a7+ (heads/pep688fix-dirty:5536853ed0, May  7 2023, 07:53:16) [Clang 14.0.0 (clang-1400.0.29.202)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import _testcapi
>>> class X(_testcapi.getBufferOnly, bytearray): pass
... 
>>> with memoryview(X()): pass
... 
Assertion failed: (obj->ob_exports >= 0), function bytearray_releasebuffer, file bytearrayobject.c, line 64.
zsh: abort      ./python.exe

This is unlikely to happen in practice because in practice any useful C buffer class will have to store some data in its C struct, so it won't be possible to double-inherit from it and another buffer, but still we should ideally protect against it.

I found out about this case while playing with PEP-688 edge cases in #104227, but the reproduction case does not rely on PEP 688 changes, and it should reproduce also on earlier Python versions. (The exact assertion failure won't, because I added that assertion in #104227; previously, you'd just set the bytearray's ob_exports field to -1 and havoc would ensue.)

[iritkatriel]

Does #104227 resolve this issue or is there more to do?

[JelleZijlstra]

#104227 did not address this issue. The test class I mentioned above no longer exists but the issue is likely still possible in theory.