On Fri, May 24, 2024 at 12:16 PM Michael Paquier <mich...@paquier.xyz> wrote:
> On Fri, May 24, 2024 at 11:58:51AM +0530, Ashutosh Bapat wrote: > > If we are looking for avoiding a segfault and get a message which helps > > debugging, using get_attname and get_attnum might be better options. > > get_attname throws an error. get_attnum doesn't throw an error and > returns > > InvalidAttnum which won't return any valid identity sequence, and thus > > return a NIL sequence list which is handled in that function already. > Using > > these two functions will avoid the clutter as well as segfault. If that's > > acceptable, I will provide a patch. > > Yeah, you could do that with these two routines as well. The result > would be the same in terms of runtime validity checks. > PFA patch using those two routines. -- Best Wishes, Ashutosh Bapat
From 2390604e8b77a3e67693991cfcac596afc7b8572 Mon Sep 17 00:00:00 2001 From: Ashutosh Bapat <ashutosh.ba...@enterprisedb.com> Date: Fri, 24 May 2024 16:50:00 +0530 Subject: [PATCH] Fix potential NULL pointer dereference in getIdentitySequence() The function invokes SearchSysCacheAttNum() and SearchSysCacheAttName(). Both of them may return NULL if the attribute number or attribute name does not exist. The function does not check for returned NULL value since it never passes non-existing attribute values. Though the risk of segmentation fault because of dereferencing NULL value does not exist today, it may arise in future. Fix it by using wrappers get_attnum() and get_attname() respectively. The second function raises an error when non-existing attribute name is passed to it. The first returns InvalidAttNum which will make getOwnedSequences_internal() return NIL resulting in an error. Author: Ashutosh Bapat Reported by: Ranier Vilela Discussion: https://www.postgresql.org/message-id/CAEudQAqh_RZqoFcYKso5d9VhF-Vd64_ZodfQ_2zSusszkEmyRg%40mail.gmail.com --- src/backend/catalog/pg_depend.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c index 5366f7820c..6422c27591 100644 --- a/src/backend/catalog/pg_depend.c +++ b/src/backend/catalog/pg_depend.c @@ -945,7 +945,7 @@ getOwnedSequences(Oid relid) Oid getIdentitySequence(Relation rel, AttrNumber attnum, bool missing_ok) { - Oid relid; + Oid relid = RelationGetRelid(rel); List *seqlist; /* @@ -954,22 +954,13 @@ getIdentitySequence(Relation rel, AttrNumber attnum, bool missing_ok) */ if (RelationGetForm(rel)->relispartition) { - List *ancestors = - get_partition_ancestors(RelationGetRelid(rel)); - HeapTuple ctup = SearchSysCacheAttNum(RelationGetRelid(rel), attnum); - const char *attname = NameStr(((Form_pg_attribute) GETSTRUCT(ctup))->attname); - HeapTuple ptup; + List *ancestors = get_partition_ancestors(relid); + const char *attname = get_attname(relid, attnum, false); relid = llast_oid(ancestors); - ptup = SearchSysCacheAttName(relid, attname); - attnum = ((Form_pg_attribute) GETSTRUCT(ptup))->attnum; - - ReleaseSysCache(ctup); - ReleaseSysCache(ptup); + attnum = get_attnum(relid, attname); list_free(ancestors); } - else - relid = RelationGetRelid(rel); seqlist = getOwnedSequences_internal(relid, attnum, DEPENDENCY_INTERNAL); if (list_length(seqlist) > 1) -- 2.34.1
