From 961f0cd933e4d517bff96858f83a7782cb734fe3 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Mon, 18 Feb 2019 12:35:52 +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. Although failure to allocate a tiny span object is fairly unlikely, it was probably made more likely by the bug that was fixed in commit 7215efdc, and the fact that in one place a FreePageManagerGet() "can't happen" case fixed by that commit was reported as regular allocation failure, instead of a FATAL error. Change to match the other similar use of FreePageManagerGet(), for consistency. This possibly explains a one-off null-pointer dereference crash report. Author: Thomas Munro Reported-by: Jakub Glapa Discussion: https://postgr.es/m/CAJk1zg3ZXhDsFg7tQGJ3ZD6N9dp%2BQ1_DU2N3%3Ds3Ywb-u6Lhc5A%40mail.gmail.com Discussion: https://postgr.es/m/CAEepm%3D1-Lo%2B98n7s1jXftEO2BhxFbpKSbPEhNiFkOooxe%2BZBWg%40mail.gmail.com --- src/backend/utils/mmgr/dsa.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/backend/utils/mmgr/dsa.c b/src/backend/utils/mmgr/dsa.c index cb868cfea2..52470e9373 100644 --- a/src/backend/utils/mmgr/dsa.c +++ b/src/backend/utils/mmgr/dsa.c @@ -693,7 +693,18 @@ 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); @@ -1669,13 +1680,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. */ -- 2.20.1