gh-142663: Fix use-after-free in memoryview comparison

[superboy-zjc]

When comparing two memoryview objects with different formats, memory_richcompare uses the struct module to unpack elements. A custom struct.Struct.unpack_from implementation could releases and resizes underlying buffer, which invalidates the buffer pointer, during iteration. This leads to a use-after-free when the comparison loop continued accessing the freed memory.

The fix increments the exports count of the memoryview objects before performing the comparison, effectively locking the buffers. This mirrors the protection already provided for non-memoryview objects via PyObject_GetBuffer.

• Issue: Use-after-free in memoryview comparison via re-entrant struct.Struct.unpack_from #142663

[superboy-zjc]

All resolved. Thanks for the review!

[sobolevn]

This line puzzles me a little bit. When you swap classes in the stdlib, you can get unexpected results. Can we replicate this without monkeypatching?

[superboy-zjc]

Hi @sobolevn I doubt the possibility of replicating this without monkeypatching. The issue stems from, during a mixed format buffer comparison, memory_richcompare function calls a re-entrant struct.Struct.unpack_from that is loaded by a PyImport_ImportModuleAttrString("struct", "Struct") call in the struct_get_unpacker. Is there anyway you would recommend to work around it ?

/* Return a new unpacker for the given format. */
static struct unpacker *
struct_get_unpacker(const char *fmt, Py_ssize_t itemsize)
{
    ...

    // struct.Struct Importation
    Struct = PyImport_ImportModuleAttrString("struct", "Struct");
    if (Struct == NULL)
        return NULL;

    x = unpacker_new();

    ...

    x->unpack_from = PyObject_GetAttrString(structobj, "unpack_from");
    if (x->unpack_from == NULL)
        goto error;
    ...

}

[sobolevn]

There are countless potential issues if we replace stdlib types with something else. Let's please first discuss in the issue whether this is even a valid bug report.

[picnixz]

I did consider it as a crash because we also fixed similar occurrences elsewhere. If we just had an exception it would have been ok, but here we have a crash. We do not want this.

[superboy-zjc]

I see there are many tests having monkey patch. Why does this case make difference? Is there any update regarding this pr or should we @ more core dev to take a look?