Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5548416

Browse files
committed
Fix oversights in array manipulation.
The nested-arrays code path in ExecEvalArrayExpr() used palloc to allocate the result array, whereas every other array-creating function has used palloc0 since 18c0b4e. This mostly works, but unused bits past the end of the nulls bitmap may end up undefined. That causes valgrind complaints with -DWRITE_READ_PARSE_PLAN_TREES, and could cause planner misbehavior as cited in 18c0b4e. There seems no very good reason why we should strive to avoid palloc0 in just this one case, so fix it the easy way with s/palloc/palloc0/. While looking at that I noted that we also failed to check for overflow of "nbytes" and "nitems" while summing the sizes of the sub-arrays, potentially allowing a crash due to undersized output allocation. For "nbytes", follow the policy used by other array-munging code of checking for overflow after each addition. (As elsewhere, the last addition of the array's overhead space doesn't need an extra check, since palloc itself will catch a value between 1Gb and 2Gb.) For "nitems", there's no very good reason to sum the inputs at all, since we can perfectly well use ArrayGetNItems' result instead of ignoring it. Per discussion of this bug, also remove redundant zeroing of the nulls bitmap in array_set_element and array_set_slice. Patch by Alexander Lakhin and myself, per bug #17858 from Alexander Lakhin; thanks also to Richard Guo. These bugs are a dozen years old, so back-patch to all supported branches. Discussion: https://postgr.es/m/17858-8fd287fd3663d051@postgresql.org
1 parent d435f15 commit 5548416

File tree

2 files changed

+11
-8
lines changed

2 files changed

+11
-8
lines changed

src/backend/executor/execExprInterp.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -2659,7 +2659,7 @@ ExecEvalArrayExpr(ExprState *state, ExprEvalStep *op)
26592659
{
26602660
/* Must be nested array expressions */
26612661
int nbytes = 0;
2662-
int nitems = 0;
2662+
int nitems;
26632663
int outer_nelems = 0;
26642664
int elem_ndims = 0;
26652665
int *elem_dims = NULL;
@@ -2754,9 +2754,14 @@ ExecEvalArrayExpr(ExprState *state, ExprEvalStep *op)
27542754
subbitmaps[outer_nelems] = ARR_NULLBITMAP(array);
27552755
subbytes[outer_nelems] = ARR_SIZE(array) - ARR_DATA_OFFSET(array);
27562756
nbytes += subbytes[outer_nelems];
2757+
/* check for overflow of total request */
2758+
if (!AllocSizeIsValid(nbytes))
2759+
ereport(ERROR,
2760+
(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
2761+
errmsg("array size exceeds the maximum allowed (%d)",
2762+
(int) MaxAllocSize)));
27572763
subnitems[outer_nelems] = ArrayGetNItems(this_ndims,
27582764
ARR_DIMS(array));
2759-
nitems += subnitems[outer_nelems];
27602765
havenulls |= ARR_HASNULL(array);
27612766
outer_nelems++;
27622767
}
@@ -2790,7 +2795,7 @@ ExecEvalArrayExpr(ExprState *state, ExprEvalStep *op)
27902795
}
27912796

27922797
/* check for subscript overflow */
2793-
(void) ArrayGetNItems(ndims, dims);
2798+
nitems = ArrayGetNItems(ndims, dims);
27942799
ArrayCheckBounds(ndims, dims, lbs);
27952800

27962801
if (havenulls)
@@ -2804,7 +2809,7 @@ ExecEvalArrayExpr(ExprState *state, ExprEvalStep *op)
28042809
nbytes += ARR_OVERHEAD_NONULLS(ndims);
28052810
}
28062811

2807-
result = (ArrayType *) palloc(nbytes);
2812+
result = (ArrayType *) palloc0(nbytes);
28082813
SET_VARSIZE(result, nbytes);
28092814
result->ndim = ndims;
28102815
result->dataoffset = dataoffset;

src/backend/utils/adt/arrayfuncs.c

+2-4
Original file line numberDiff line numberDiff line change
@@ -2483,8 +2483,7 @@ array_set_element(Datum arraydatum,
24832483
{
24842484
bits8 *newnullbitmap = ARR_NULLBITMAP(newarray);
24852485

2486-
/* Zero the bitmap to take care of marking inserted positions null */
2487-
MemSet(newnullbitmap, 0, (newnitems + 7) / 8);
2486+
/* palloc0 above already marked any inserted positions as nulls */
24882487
/* Fix the inserted value */
24892488
if (addedafter)
24902489
array_set_isnull(newnullbitmap, newnitems - 1, isNull);
@@ -3100,8 +3099,7 @@ array_set_slice(Datum arraydatum,
31003099
bits8 *newnullbitmap = ARR_NULLBITMAP(newarray);
31013100
bits8 *oldnullbitmap = ARR_NULLBITMAP(array);
31023101

3103-
/* Zero the bitmap to handle marking inserted positions null */
3104-
MemSet(newnullbitmap, 0, (nitems + 7) / 8);
3102+
/* palloc0 above already marked any inserted positions as nulls */
31053103
array_bitmap_copy(newnullbitmap, addedbefore,
31063104
oldnullbitmap, 0,
31073105
itemsbefore);

0 commit comments

Comments
 (0)