Recursive ReceiveSharedInvalidMessages not safe

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Recursive ReceiveSharedInvalidMessages not safe
Date: 2014-05-05 10:05:31
Message-ID: 20140505100531.GT12715@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

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.

It looks to me like SHAREDINVALRELCACHE_ID messages are the only ones
actually affected by this in reality because all the other ones either
operate on stack memory or don't recurse. What could happen there is
that a different relcache entry is invalidated than the relcache
invalidation callback is called for.
This actually could explain the relfilenodemap error we've seen a couple
weeks back.

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.

Comments?

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2014-05-05 10:10:07 Re: Cluster name in ps output
Previous Message Thomas Munro 2014-05-05 10:00:34 Cluster name in ps output