[BUG #17888] Incorrect memory access in gist__int_ops for an input array with many elements
Hi,
I was looking at this issue:
/messages/by-id/17888-f72930e6b5ce8c14@postgresql.org
pfree call on contrib/intarray/_int_gist.c:345
```
if (in != (ArrayType *) DatumGetPointer(entry->key))
pfree(in);
```
leads to BogusFree function call and server crash.
This is when we use array size of 1001.
If we increase array size to 3001, we get index size error,
NOTICE: input array is too big (199 maximum allowed, 3001 current), use
gist__intbig_ops opclass instead
ERROR: index row requires 12040 bytes, maximum size is 8191
When array size is 1001 is concerned, we raise elog about input array is
too big, multiple of times. Wouldn't it make more sense to error out
instead of proceeding further and getting bogus pointer errors messages
(as we do when size is 3001)?
Changing elog to ereport makes behavior consistent (between array size
of 1001 vs 3001), so I have
attached a patch for that.
It errors out like this:
ERROR: input array is too big (199 maximum allowed, 1001 current), use
gist__intbig_ops opclass instead
This is same error which was raised as notice earlier.
Let me know if it makes sense.
Also, comments on BogusFree mentions `As a possible
aid in debugging, we report the header word along with the pointer
address`. How can we interpret useful debugging information from this?
`pfree called with invalid pointer 0x7ff1706d0030 (header
0x4fc8000100000000)`
Regards,
Ankit
Attachments:
v1-gist-int-ops-overflow.patchtext/x-patch; charset=UTF-8; name=v1-gist-int-ops-overflow.patchDownload
diff --git a/contrib/intarray/_int_gist.c b/contrib/intarray/_int_gist.c
index 7a48ce624d..79c81127f6 100644
--- a/contrib/intarray/_int_gist.c
+++ b/contrib/intarray/_int_gist.c
@@ -181,8 +181,10 @@ g_int_compress(PG_FUNCTION_ARGS)
PREPAREARR(r);
if (ARRNELEMS(r) >= 2 * num_ranges)
- elog(NOTICE, "input array is too big (%d maximum allowed, %d current), use gist__intbig_ops opclass instead",
- 2 * num_ranges - 1, ARRNELEMS(r));
+ ereport(ERROR,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("input array is too big (%d maximum allowed, %d current), use gist__intbig_ops opclass instead",
+ 2 * num_ranges - 1, ARRNELEMS(r))));
retval = palloc(sizeof(GISTENTRY));
gistentryinit(*retval, PointerGetDatum(r),
On Wed, 12 Apr 2023 at 03:49, Ankit Kumar Pandey <itsankitkp@gmail.com> wrote:
Also, comments on BogusFree mentions `As a possible
aid in debugging, we report the header word along with the pointer
address`. How can we interpret useful debugging information from this?`pfree called with invalid pointer 0x7ff1706d0030 (header
0x4fc8000100000000)`
elog(ERROR)s are not meant to happen. ISTM, what's there is about the
best that can be done with our current infrastructure. If that occurs
on some machine that we can't get access to debug on, then having the
header bits might be useful, at least, certainly much more useful than
just not having them at all.
If you can think of something more useful to put in the elog, then we
could consider changing it to improve it.
Just in case you suggest it, I don't believe it's wise to try and
split it out into the components of MemoryChunk's hdrmask.
MemoryContexts aren't forced into using that. They're only forced into
using the 3 least significant bits for the MemoryContextMethodID.
David