Re: Possible null pointer dereference in afterTriggerAddEvent() - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: Possible null pointer dereference in afterTriggerAddEvent()
Date
Msg-id 202407251707.tkkrcs7c4eso@alvherre.pgsql
Whole thread Raw
In response to Possible null pointer dereference in afterTriggerAddEvent()  (Alexander Kuznetsov <kuznetsovam@altlinux.org>)
Responses Re: Possible null pointer dereference in afterTriggerAddEvent()
List pgsql-hackers
On 2024-Jul-25, Alexander Kuznetsov wrote:

Hello Alexander,

> In src/backend/commands/trigger.c:4031, there is an
> afterTriggerAddEvent() function. The variable chunk is assigned the
> value of events->tail at line 4050. Subsequently, chunk is compared to
> NULL at lines 4051 and 4079, indicating that events->tail could
> potentially be NULL.
> 
> However, at line 4102, we dereference events->tail by accessing
> events->tail->next without first checking if it is NULL.

Thanks for reporting this.  I think the unwritten assumption is that
->tail and ->head are NULL or not simultaneously, so testing for one of
them is equivalent to testing both.  Failing to comply with this would
be data structure corruption.

> To address this issue, I propose at least adding an assertion to
> ensure that events->tail != NULL before the dereference. The suggested
> patch is included in the attachment.

Hmm, this doesn't actually change any behavior AFAICS.  If events->tail
is NULL and we reach that code, then the dereference to get ->next is
going to crash, whether the assertion is there or not.

Maybe for sanity (and perhaps for Svace compliance) we could do it the
other way around, i.e. by testing events->tail for nullness instead of
events->head, then add the assertion:

        if (events->tail == NULL)
        {
            Assert(events->head == NULL);
            events->head = chunk;
        }
        else
            events->tail->next = chunk;

This way, it's not wholly redundant.

That said, I'm not sure we actually *need* to change this.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"El Maquinismo fue proscrito so pena de cosquilleo hasta la muerte"
(Ijon Tichy en Viajes, Stanislaw Lem)



pgsql-hackers by date:

Previous
From: Alexander Lakhin
Date:
Subject: Re: Sporadic connection-setup-related test failures on Cygwin in v15-
Next
From: Jacob Champion
Date:
Subject: Re: Serverside SNI support in libpq