Fix unsafe coding in ResourceOwnerReleaseAll()

Started by Tom Lane1 day ago1 messageshackers
Jump to latest
#1Tom Lane
tgl@sss.pgh.pa.us

I wondered why the example shown in [1]/messages/by-id/19527-6e7686960c6dce78@postgresql.org led to a double-free crash
rather than a clean failure. On investigation, the problem is that
ResourceOwnerReleaseAll() calls the resource kind's ReleaseResource
method before deleting the resource from the owner, rather than
afterwards as the code in ResourceOwnerReleaseAllOfKind() does.
So if we throw an error inside ReleaseResource, the subsequent abort
cleanup comes back and does resource owner cleanup again, and we
try to delete the item again. Kaboom.

The attached patch converts the bug example into a clean failure,
even with the recent bug fix in pgcrypto undone:

regression=# SELECT encrypt_iv(
repeat('A', 1073741308)::bytea,
decode('00112233445566778899aabbccddeeff', 'hex'),
decode('000102030405060708090a0b0c0d0e0f', 'hex'),
'aes'
);
ERROR: invalid memory alloc request size 1073741824
WARNING: AbortTransaction while in ABORT state
ERROR: ResourceOwnerForget called for pgcrypto OpenSSL cipher handle after release started
regression=#

Of course, this might lead to leaking the resource we wished to free.
But that's better than crashing, or at least that's the value judgment
we made long ago in the original ResourceOwner code.

regards, tom lane

[1]: /messages/by-id/19527-6e7686960c6dce78@postgresql.org

Attachments:

make-ResourceOwnerReleaseAll-safer.patchtext/x-diff; charset=us-ascii; name=make-ResourceOwnerReleaseAll-safer.patchDownload+15-5