From 29ddb548f683e15429cd5464029c01d83c40bb40 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 25 Feb 2019 10:54:12 +1300 Subject: [PATCH] Fix inconsistent out-of-memory error reporting in dsa.c. Commit 16be2fd1 introduced the flag DSA_ALLOC_NO_OOM to control whether the DSA allocator would raise an error or return InvalidDsaPointer on failure to allocate. One edge case was not handled correctly: if we fail to allocate an internal "span" object for a large allocation, we would always return InvalidDsaPointer regardless of the flag; a caller not expecting that could then dereference a null pointer. This is a plausible explanation for a one-off report of a segfault. Remove a redundant pair of braces so that all three stanzas that handle DSA_ALLOC_NO_OOM match in style, for visual consistency. While fixing inconsistencies, if FreePageManagerGet() can't supply the pages that our book-keeping says it should be able to supply, then we should always report a FATAL error. Previously we treated that as a regular allocation failure in one code path, but as a FATAL condition in another. Back-patch to 10, where dsa.c landed. Author: Thomas Munro Reported-by: Jakub Glapa Discussion: https://postgr.es/m/CAEepm=2oPqXxyWQ-1o60tpOLrwkw=VpgNXqqF1VN2EyO9zKGQw@mail.gmail.com --- src/backend/utils/mmgr/dsa.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c index cb868cfea2..62e6b652af 100644 --- a/src/backend/utils/mmgr/dsa.c +++ b/src/backend/utils/mmgr/dsa.c @@ -693,7 +693,16 @@ dsa_allocate_extended(dsa_area *area, size_t size, int flags) /* Obtain a span object. */ span_pointer = alloc_object(area, DSA_SCLASS_BLOCK_OF_SPANS); if (!DsaPointerIsValid(span_pointer)) + { + /* Raise error unless asked not to. */ + if ((flags & DSA_ALLOC_NO_OOM) == 0) + ereport(ERROR, + (errcode(ERRCODE_OUT_OF_MEMORY), + errmsg("out of memory"), + errdetail("Failed on DSA request of size %zu.", + size))); return InvalidDsaPointer; + } LWLockAcquire(DSA_AREA_LOCK(area), LW_EXCLUSIVE); @@ -790,12 +799,10 @@ dsa_allocate_extended(dsa_area *area, size_t size, int flags) { /* Raise error unless asked not to. */ if ((flags & DSA_ALLOC_NO_OOM) == 0) - { ereport(ERROR, (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"), errdetail("Failed on DSA request of size %zu.", size))); - } return InvalidDsaPointer; } @@ -1669,13 +1676,14 @@ ensure_active_superblock(dsa_area *area, dsa_area_pool *pool, return false; } } + /* + * This shouldn't happen: get_best_segment() or make_new_segment() + * promised that we can successfully allocate npages. + */ if (!FreePageManagerGet(segment_map->fpm, npages, &first_page)) - { - LWLockRelease(DSA_AREA_LOCK(area)); - if (size_class != DSA_SCLASS_BLOCK_OF_SPANS) - dsa_free(area, span_pointer); - return false; - } + elog(FATAL, + "dsa_allocate could not find %zu free pages for superblock", + npages); LWLockRelease(DSA_AREA_LOCK(area)); /* Compute the start of the superblock. */