pg.ddx.io pgsql-committers@postgresql.org mailing list archivehelp / color / mirror / Atom feed
pgsql: Detect pfree or repalloc of a previously-freed memory chunk. 4+ messages / 1 participants [nested] [flat]
* pgsql: Detect pfree or repalloc of a previously-freed memory chunk. @ 2026-03-30 16:02 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2026-03-30 16:02 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Detect pfree or repalloc of a previously-freed memory chunk. Before the major rewrite in commit c6e0fe1f2, AllocSetFree() would typically crash when asked to free an already-free chunk. That was an ugly but serviceable way of detecting coding errors that led to double pfrees. But since that rewrite, double pfrees went through just fine, because the "hdrmask" of a freed chunk isn't changed at all when putting it on the freelist. We'd end with a corrupt freelist that circularly links back to the doubly-freed chunk, which would usually result in trouble later, far removed from the actual bug. This situation is no good at all for debugging purposes. Fortunately, we can fix it at low cost in MEMORY_CONTEXT_CHECKING builds by making AllocSetFree() check for chunk->requested_size == InvalidAllocSize, relying on the pre-existing code that sets it that way just below. I investigated the alternative of changing a freed chunk's methodid field, which would allow detection in non-MEMORY_CONTEXT_CHECKING builds too. But that adds measurable overhead. Seeing that we didn't notice this oversight for more than three years, it's hard to argue that detecting this type of bug is worth any extra overhead in production builds. Likewise fix AllocSetRealloc() to detect repalloc() on a freed chunk, and apply similar changes in generation.c and slab.c. (generation.c would hit an Assert failure anyway, but it seems best to make it act like aset.c.) bump.c doesn't need changes since it doesn't support pfree in the first place. Ideally alignedalloc.c would receive similar changes, but in debugging builds it's impossible to reach AlignedAllocFree() or AlignedAllocRealloc() on a pfreed chunk, because the underlying context's pfree would have wiped the chunk header of the aligned chunk. But that means we should get an error of some sort, so let's be content with that. Per investigation of why the test case for bug #19438 didn't appear to fail in v16 and up, even though the underlying bug was still present. (This doesn't fix the underlying double-free bug, just cause it to get detected.) Bug: #19438 Reported-by: Dmitriy Kuzmin <kuzmin.db4@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org Backpatch-through: 16 Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/095555daf1246ac28595fcf5b26bbb4686b0cdb6 Modified Files -------------- src/backend/utils/mmgr/aset.c | 24 ++++++++++++++++++++++++ src/backend/utils/mmgr/generation.c | 10 ++++++++++ src/backend/utils/mmgr/slab.c | 8 ++++++++ 3 files changed, 42 insertions(+) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Detect pfree or repalloc of a previously-freed memory chunk. @ 2026-03-30 16:02 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2026-03-30 16:02 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Detect pfree or repalloc of a previously-freed memory chunk. Before the major rewrite in commit c6e0fe1f2, AllocSetFree() would typically crash when asked to free an already-free chunk. That was an ugly but serviceable way of detecting coding errors that led to double pfrees. But since that rewrite, double pfrees went through just fine, because the "hdrmask" of a freed chunk isn't changed at all when putting it on the freelist. We'd end with a corrupt freelist that circularly links back to the doubly-freed chunk, which would usually result in trouble later, far removed from the actual bug. This situation is no good at all for debugging purposes. Fortunately, we can fix it at low cost in MEMORY_CONTEXT_CHECKING builds by making AllocSetFree() check for chunk->requested_size == InvalidAllocSize, relying on the pre-existing code that sets it that way just below. I investigated the alternative of changing a freed chunk's methodid field, which would allow detection in non-MEMORY_CONTEXT_CHECKING builds too. But that adds measurable overhead. Seeing that we didn't notice this oversight for more than three years, it's hard to argue that detecting this type of bug is worth any extra overhead in production builds. Likewise fix AllocSetRealloc() to detect repalloc() on a freed chunk, and apply similar changes in generation.c and slab.c. (generation.c would hit an Assert failure anyway, but it seems best to make it act like aset.c.) bump.c doesn't need changes since it doesn't support pfree in the first place. Ideally alignedalloc.c would receive similar changes, but in debugging builds it's impossible to reach AlignedAllocFree() or AlignedAllocRealloc() on a pfreed chunk, because the underlying context's pfree would have wiped the chunk header of the aligned chunk. But that means we should get an error of some sort, so let's be content with that. Per investigation of why the test case for bug #19438 didn't appear to fail in v16 and up, even though the underlying bug was still present. (This doesn't fix the underlying double-free bug, just cause it to get detected.) Bug: #19438 Reported-by: Dmitriy Kuzmin <kuzmin.db4@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org Backpatch-through: 16 Branch ------ REL_16_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/a26ac902f662e81f9b8b8ba1e7b784d4ab82b05f Modified Files -------------- src/backend/utils/mmgr/aset.c | 24 ++++++++++++++++++++++++ src/backend/utils/mmgr/generation.c | 10 ++++++++++ src/backend/utils/mmgr/slab.c | 8 ++++++++ 3 files changed, 42 insertions(+) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Detect pfree or repalloc of a previously-freed memory chunk. @ 2026-03-30 16:02 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2026-03-30 16:02 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Detect pfree or repalloc of a previously-freed memory chunk. Before the major rewrite in commit c6e0fe1f2, AllocSetFree() would typically crash when asked to free an already-free chunk. That was an ugly but serviceable way of detecting coding errors that led to double pfrees. But since that rewrite, double pfrees went through just fine, because the "hdrmask" of a freed chunk isn't changed at all when putting it on the freelist. We'd end with a corrupt freelist that circularly links back to the doubly-freed chunk, which would usually result in trouble later, far removed from the actual bug. This situation is no good at all for debugging purposes. Fortunately, we can fix it at low cost in MEMORY_CONTEXT_CHECKING builds by making AllocSetFree() check for chunk->requested_size == InvalidAllocSize, relying on the pre-existing code that sets it that way just below. I investigated the alternative of changing a freed chunk's methodid field, which would allow detection in non-MEMORY_CONTEXT_CHECKING builds too. But that adds measurable overhead. Seeing that we didn't notice this oversight for more than three years, it's hard to argue that detecting this type of bug is worth any extra overhead in production builds. Likewise fix AllocSetRealloc() to detect repalloc() on a freed chunk, and apply similar changes in generation.c and slab.c. (generation.c would hit an Assert failure anyway, but it seems best to make it act like aset.c.) bump.c doesn't need changes since it doesn't support pfree in the first place. Ideally alignedalloc.c would receive similar changes, but in debugging builds it's impossible to reach AlignedAllocFree() or AlignedAllocRealloc() on a pfreed chunk, because the underlying context's pfree would have wiped the chunk header of the aligned chunk. But that means we should get an error of some sort, so let's be content with that. Per investigation of why the test case for bug #19438 didn't appear to fail in v16 and up, even though the underlying bug was still present. (This doesn't fix the underlying double-free bug, just cause it to get detected.) Bug: #19438 Reported-by: Dmitriy Kuzmin <kuzmin.db4@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org Backpatch-through: 16 Branch ------ REL_17_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/0c8b4e9cfc453c484feabeaf95813a788d3fb16c Modified Files -------------- src/backend/utils/mmgr/aset.c | 24 ++++++++++++++++++++++++ src/backend/utils/mmgr/generation.c | 10 ++++++++++ src/backend/utils/mmgr/slab.c | 8 ++++++++ 3 files changed, 42 insertions(+) ^ permalink raw reply [nested|flat] 4+ messages in thread
* pgsql: Detect pfree or repalloc of a previously-freed memory chunk. @ 2026-03-30 16:02 Tom Lane <tgl@sss.pgh.pa.us> 0 siblings, 0 replies; 4+ messages in thread From: Tom Lane @ 2026-03-30 16:02 UTC (permalink / raw) To: pgsql-committers@lists.postgresql.org Detect pfree or repalloc of a previously-freed memory chunk. Before the major rewrite in commit c6e0fe1f2, AllocSetFree() would typically crash when asked to free an already-free chunk. That was an ugly but serviceable way of detecting coding errors that led to double pfrees. But since that rewrite, double pfrees went through just fine, because the "hdrmask" of a freed chunk isn't changed at all when putting it on the freelist. We'd end with a corrupt freelist that circularly links back to the doubly-freed chunk, which would usually result in trouble later, far removed from the actual bug. This situation is no good at all for debugging purposes. Fortunately, we can fix it at low cost in MEMORY_CONTEXT_CHECKING builds by making AllocSetFree() check for chunk->requested_size == InvalidAllocSize, relying on the pre-existing code that sets it that way just below. I investigated the alternative of changing a freed chunk's methodid field, which would allow detection in non-MEMORY_CONTEXT_CHECKING builds too. But that adds measurable overhead. Seeing that we didn't notice this oversight for more than three years, it's hard to argue that detecting this type of bug is worth any extra overhead in production builds. Likewise fix AllocSetRealloc() to detect repalloc() on a freed chunk, and apply similar changes in generation.c and slab.c. (generation.c would hit an Assert failure anyway, but it seems best to make it act like aset.c.) bump.c doesn't need changes since it doesn't support pfree in the first place. Ideally alignedalloc.c would receive similar changes, but in debugging builds it's impossible to reach AlignedAllocFree() or AlignedAllocRealloc() on a pfreed chunk, because the underlying context's pfree would have wiped the chunk header of the aligned chunk. But that means we should get an error of some sort, so let's be content with that. Per investigation of why the test case for bug #19438 didn't appear to fail in v16 and up, even though the underlying bug was still present. (This doesn't fix the underlying double-free bug, just cause it to get detected.) Bug: #19438 Reported-by: Dmitriy Kuzmin <kuzmin.db4@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Reviewed-by: David Rowley <dgrowleyml@gmail.com> Discussion: https://postgr.es/m/19438-9d37b179c56d43aa@postgresql.org Backpatch-through: 16 Branch ------ REL_18_STABLE Details ------- https://git.postgresql.org/pg/commitdiff/3f3eefc288925c61068a05d6f0f5fcbf865a1c70 Modified Files -------------- src/backend/utils/mmgr/aset.c | 24 ++++++++++++++++++++++++ src/backend/utils/mmgr/generation.c | 10 ++++++++++ src/backend/utils/mmgr/slab.c | 8 ++++++++ 3 files changed, 42 insertions(+) ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-03-30 16:02 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-03-30 16:02 pgsql: Detect pfree or repalloc of a previously-freed memory chunk. Tom Lane <tgl@sss.pgh.pa.us> 2026-03-30 16:02 pgsql: Detect pfree or repalloc of a previously-freed memory chunk. Tom Lane <tgl@sss.pgh.pa.us> 2026-03-30 16:02 pgsql: Detect pfree or repalloc of a previously-freed memory chunk. Tom Lane <tgl@sss.pgh.pa.us> 2026-03-30 16:02 pgsql: Detect pfree or repalloc of a previously-freed memory chunk. Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions for how to clone and mirror all data and code used for this inbox