Hi all,
As there have been complaints that it was hard to follow all the small
patches I have sent to fix the issues related to Coverity, here they
are gathered with patches for each one of them:
1) Missing return value checks in jsonfuncs.c, fixed by 0001 (send
here previously =>
cab7npqqcj3hu9p7a6vuhomepjkoyqrjxnt1g2f7qy_cq0q8...@mail.gmail.com)
JsonbIteratorNext is missing a set of (void) in front of its calls.
2) Potential pointer dereference in plperl.c, fixed by 0002 (sent
previously here =>
CAB7nPqRBCWAXTLw0yBR=bk94cryxu8twvxgyyoxautw08ok...@mail.gmail.com).
This is related to a change done by transforms. In short,
plperl_call_perl_func@plperl.c will have a pointer dereference if
desc->arg_arraytype[i] is InvalidOid. And AFAIK,
fcinfo->flinfo->fn_oid can be InvalidOid in this code path.
3) visibilitymap_truncate and FreeSpaceMapTruncateRel are doing a
NULL-pointer check on rel->rd_smgr but it has been dereferenced in all
the code paths leading to those checks. See 0003. For code readability
mainly.
4) Return result of timestamp2tm is not checked 2 times in
GetCurrentDateTime and GetCurrentTimeUsec, while all the other 40
calls of this function do sanity checks. Returning
ERRCODE_DATETIME_VALUE_OUT_OF_RANGE in case of an error would be good
for consistency. See 0004. (issue reported previously here
CAB7nPqRSk=J8eUdd55fL-w+k=8sDTHLVBt-cgG9jWN=vo2o...@mail.gmail.com)
Each issue is independent, please feel free to comment.
Regards,
--
Michael
From e3f9729de90e37d173de7fce076dc0f0d4362a20 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 7 Jul 2015 16:01:35 +0900
Subject: [PATCH 1/4] Fix missing return value checks for JsonbIteratorNext
Spotted by Coverity.
---
src/backend/utils/adt/jsonfuncs.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index 13d5b7a..b4258d8 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3362,8 +3362,7 @@ jsonb_delete(PG_FUNCTION_ARGS)
{
/* skip corresponding value as well */
if (r == WJB_KEY)
- JsonbIteratorNext(&it, &v, true);
-
+ (void) JsonbIteratorNext(&it, &v, true);
continue;
}
@@ -3436,7 +3435,7 @@ jsonb_delete_idx(PG_FUNCTION_ARGS)
if (i++ == idx)
{
if (r == WJB_KEY)
- JsonbIteratorNext(&it, &v, true); /* skip value */
+ (void) JsonbIteratorNext(&it, &v, true); /* skip value */
continue;
}
}
--
2.4.5
From 88a8baa7702c54512af24e72749a12f905bfadff Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 7 Jul 2015 16:03:56 +0900
Subject: [PATCH 2/4] Fix pointer dereference in plperl caused by transforms
Spotted by Coverity.
---
src/pl/plperl/plperl.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/pl/plperl/plperl.c b/src/pl/plperl/plperl.c
index 78baaac..d78cff1 100644
--- a/src/pl/plperl/plperl.c
+++ b/src/pl/plperl/plperl.c
@@ -2100,8 +2100,11 @@ plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
PUSHMARK(SP);
EXTEND(sp, desc->nargs);
- if (fcinfo->flinfo->fn_oid)
+ if (OidIsValid(fcinfo->flinfo->fn_oid))
+ {
get_func_signature(fcinfo->flinfo->fn_oid, &argtypes, &nargs);
+ Assert(nargs == desc->nargs);
+ }
for (i = 0; i < desc->nargs; i++)
{
@@ -2120,7 +2123,8 @@ plperl_call_perl_func(plperl_proc_desc *desc, FunctionCallInfo fcinfo)
if (OidIsValid(desc->arg_arraytype[i]))
sv = plperl_ref_from_pg_array(fcinfo->arg[i], desc->arg_arraytype[i]);
- else if ((funcid = get_transform_fromsql(argtypes[i], current_call_data->prodesc->lang_oid, current_call_data->prodesc->trftypes)))
+ else if (OidIsValid(fcinfo->flinfo->fn_oid) &&
+ (funcid = get_transform_fromsql(argtypes[i], current_call_data->prodesc->lang_oid, current_call_data->prodesc->trftypes)))
sv = (SV *) DatumGetPointer(OidFunctionCall1(funcid, fcinfo->arg[i]));
else
{
--
2.4.5
From b99a83e052ce10f97af0a574f077167fcc992e6c Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 7 Jul 2015 16:08:15 +0900
Subject: [PATCH 3/4] Remove unnecessary NULL-pointer checks
All the code paths leading to those checks dereference those pointers.
Spotted by Coverity.
---
src/backend/access/heap/visibilitymap.c | 3 +--
src/backend/storage/freespace/freespace.c | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/backend/access/heap/visibilitymap.c b/src/backend/access/heap/visibilitymap.c
index 7c38772..acd9fde 100644
--- a/src/backend/access/heap/visibilitymap.c
+++ b/src/backend/access/heap/visibilitymap.c
@@ -510,8 +510,7 @@ visibilitymap_truncate(Relation rel, BlockNumber nheapblocks)
* invalidate their copy of smgr_vm_nblocks, and this one too at the next
* command boundary. But this ensures it isn't outright wrong until then.
*/
- if (rel->rd_smgr)
- rel->rd_smgr->smgr_vm_nblocks = newnblocks;
+ rel->rd_smgr->smgr_vm_nblocks = newnblocks;
}
/*
diff --git a/src/backend/storage/freespace/freespace.c b/src/backend/storage/freespace/freespace.c
index fddb47c..c948cb6 100644
--- a/src/backend/storage/freespace/freespace.c
+++ b/src/backend/storage/freespace/freespace.c
@@ -308,8 +308,7 @@ FreeSpaceMapTruncateRel(Relation rel, BlockNumber nblocks)
* at the next command boundary. But this ensures it isn't outright wrong
* until then.
*/
- if (rel->rd_smgr)
- rel->rd_smgr->smgr_fsm_nblocks = new_nfsmblocks;
+ rel->rd_smgr->smgr_fsm_nblocks = new_nfsmblocks;
}
/*
--
2.4.5
From b1475317954276eb2daa7aaaa7d3baffcdd39247 Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@otacoo.com>
Date: Tue, 7 Jul 2015 16:11:48 +0900
Subject: [PATCH 4/4] Check return values of timestamp2tm
All the other code paths are doing similar checks, and even if this is
a minor problem it is better to be consistent with the rest to prevent
future problems.
Spotted by Coverity.
---
src/backend/utils/adt/datetime.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/datetime.c b/src/backend/utils/adt/datetime.c
index 2a44b6e..fe0ba80 100644
--- a/src/backend/utils/adt/datetime.c
+++ b/src/backend/utils/adt/datetime.c
@@ -371,8 +371,11 @@ GetCurrentDateTime(struct pg_tm * tm)
int tz;
fsec_t fsec;
- timestamp2tm(GetCurrentTransactionStartTimestamp(), &tz, tm, &fsec,
- NULL, NULL);
+ if (timestamp2tm(GetCurrentTransactionStartTimestamp(), &tz, tm, &fsec,
+ NULL, NULL) != 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("timestamp out of range")));
/* Note: don't pass NULL tzp to timestamp2tm; affects behavior */
}
@@ -387,8 +390,11 @@ GetCurrentTimeUsec(struct pg_tm * tm, fsec_t *fsec, int *tzp)
{
int tz;
- timestamp2tm(GetCurrentTransactionStartTimestamp(), &tz, tm, fsec,
- NULL, NULL);
+ if (timestamp2tm(GetCurrentTransactionStartTimestamp(), &tz, tm, fsec,
+ NULL, NULL) != 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+ errmsg("timestamp out of range")));
/* Note: don't pass NULL tzp to timestamp2tm; affects behavior */
if (tzp != NULL)
*tzp = tz;
--
2.4.5
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
