Re: Recursive ReceiveSharedInvalidMessages not safe

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Recursive ReceiveSharedInvalidMessages not safe
Date: 2014-05-05 18:15:58
Message-ID: 11846.1399313758@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> While investigating an issue pointed out by valgrind around undefined
> bytes in inval.c SHAREDINVALSMGR_ID processing I noticed that there's a
> bug in ReceiveSharedInvalidMessages(). It tries to be safe against
> recursion but it's not:
> When it recurses into ReceiveSharedInvalidMessages() from it's main loop
> from inside the inval callback while nextmsg = nummsgs it'll overwrite
> the 'messages' array with new contents. But at this point the old
> content of one entry in the messages array is still passed to
> the LocalExecuteInvalidationMessage() that caused the recursion.

Hm, yeah, so if the called inval function continues to use the message
contents after doing something that could result in a recursive call,
it might be looking at trashed data.

> It looks to me like this is broken since at least fad153ec. I think the
> fix is just to make the current 'SharedInvalidationMessage *msg' not be
> pointers but a local copiy of the to-be-processed entry.

Yeah, that should do it. I think I'd been trying to avoid copying
messages more times than necessary, but evidently I optimized away
one copy step too many :-(

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2014-05-05 18:17:18 Re: TABLESPACE and directory for Foreign tables?
Previous Message Andres Freund 2014-05-05 18:13:38 Re: avoiding tuple copying in btree index builds