Re: Possible null pointer dereference in afterTriggerAddEvent()

Alexander Kuznetsov <kuznetsovam@altlinux.org>

From: Alexander Kuznetsov <kuznetsovam@altlinux.org>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: pgsql-hackers@lists.postgresql.org, nickel@altlinux.org, egori@altlinux.org
Date: 2024-07-26T09:16:00Z
Lists: pgsql-hackers

Attachments

25.07.2024 20:07, Alvaro Herrera wrote:
> 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.
Thanks for your response!
I agree with the proposed changes and have updated the patch accordingly. Version 2 is attached.
> That said, I'm not sure we actually *need* to change this.
I understand and partly agree. But it appears that with these changes, the dereference of a null pointer is impossible even in builds where assertions are disabled. Previously, this issue could theoretically occur. Consequently, these changes slightly enhance overall security.

-- 
Best regards,
Alexander Kuznetsov

Commits

  1. Turn 'if' condition around to avoid Svace complaint