pgsql: Fix potential NULL pointer dereference in getIdentitySequence()

Started by Michael Paquierabout 2 years ago5 messagescomitters
Jump to latest
#1Michael Paquier
michael@paquier.xyz

Fix potential NULL pointer dereference in getIdentitySequence()

The function invokes SearchSysCacheAttNum() and SearchSysCacheAttName().
They may respectively return 0 for the attribute number or NULL for
the attribute name if the attribute does not exist, without any kind of
error handling. The common practice is to check that the data retrieved
from the syscache is valid. There is no risk of NULL pointer
dereferences currently, but let's stick to the practice of making sure
that this data is always valid, to catch future inconsistency mistakes.
The code is switched to use get_attnum() and get_attname(), and adds
some error handling.

Oversight in 509199587df7.

Reported-by: Ranier Vilela
Author: Ashutosh Bapat
Discussion: /messages/by-id/CAEudQAqh_RZqoFcYKso5d9VhF-Vd64_ZodfQ_2zSusszkEmyRg@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/8285b484a47d829a29fbe0ebe65cdc9f9dfb179d

Modified Files
--------------
src/backend/catalog/pg_depend.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Michael Paquier (#1)
Re: pgsql: Fix potential NULL pointer dereference in getIdentitySequence()

On 2024-05-26 Su 07:58, Michael Paquier wrote:

Fix potential NULL pointer dereference in getIdentitySequence()

The function invokes SearchSysCacheAttNum() and SearchSysCacheAttName().
They may respectively return 0 for the attribute number or NULL for
the attribute name if the attribute does not exist, without any kind of
error handling. The common practice is to check that the data retrieved
from the syscache is valid. There is no risk of NULL pointer
dereferences currently, but let's stick to the practice of making sure
that this data is always valid, to catch future inconsistency mistakes.
The code is switched to use get_attnum() and get_attname(), and adds
some error handling.

Oversight in 509199587df7.

Reported-by: Ranier Vilela
Author: Ashutosh Bapat
Discussion: /messages/by-id/CAEudQAqh_RZqoFcYKso5d9VhF-Vd64_ZodfQ_2zSusszkEmyRg@mail.gmail.com

This appears to have upset a number of buildfarm members

cheers

andrew

--

Andrew Dunstan
EDB: https://www.enterprisedb.com

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#2)
Re: pgsql: Fix potential NULL pointer dereference in getIdentitySequence()

Andrew Dunstan <andrew@dunslane.net> writes:

On 2024-05-26 Su 07:58, Michael Paquier wrote:

Fix potential NULL pointer dereference in getIdentitySequence()

This appears to have upset a number of buildfarm members

It's hard to see how that patch would have broken "configure",
and besides that the failures started earlier.

What it looks like from here is that Andres messed up the
installed-packages set on a lot of his animals.

regards, tom lane

#4Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#3)
Re: pgsql: Fix potential NULL pointer dereference in getIdentitySequence()

On 2024-05-26 Su 17:51, Tom Lane wrote:

Andrew Dunstan<andrew@dunslane.net> writes:

On 2024-05-26 Su 07:58, Michael Paquier wrote:

Fix potential NULL pointer dereference in getIdentitySequence()

This appears to have upset a number of buildfarm members

It's hard to see how that patch would have broken "configure",
and besides that the failures started earlier.

What it looks like from here is that Andres messed up the
installed-packages set on a lot of his animals.

Oops, sorry for the noise. I just looked at a few failures and this was
what I saw.

cheers

andrew

--
Andrew Dunstan
EDB:https://www.enterprisedb.com

#5Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#4)
Re: pgsql: Fix potential NULL pointer dereference in getIdentitySequence()

On Sun, May 26, 2024 at 06:43:50PM -0400, Andrew Dunstan wrote:

Oops, sorry for the noise. I just looked at a few failures and this was what
I saw.

Thanks for the heads-up. I saw the failures before your message,
panicked at first, and then noticed that it was unrelated.
--
Michael