Thread: making bgworkers without shmem access actually not have shmem access

I've complained about this problem a few times before: there's nothing
to prevent a background worker which doesn't request shared memory
access from calling InitProcess() and then accessing shared memory
anyway.  The attached patch is a first crack at fixing it.
Unfortunately, there's still a window between when we fork() and when
we detach shared memory, but that's also true for processes like
syslogger, whose death is nevertheless does not cause a
crash-and-restart.  Also unfortunately, in the EXEC_BACKEND case, we
actually have to attach shared memory first, to get our background
worker entry details. This is undesirable, but I'm not sure there's
any good way to fix it at all, and certainly not in time for 9.4.
Hopefully, the window when shared memory is attached with this patch
is short enough to make things relatively safe.

At a minimum, it's got to be better than the status quo, where shared
memory is accessible throughout the entire lifetime of
non-shmem-access background workers.

Attached is also a new background worker called say_hello which I used
for testing this patch.  That's obviously not for commit.

Comments?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> I've complained about this problem a few times before: there's nothing
> to prevent a background worker which doesn't request shared memory
> access from calling InitProcess() and then accessing shared memory
> anyway.  The attached patch is a first crack at fixing it.

> Comments?

Looks reasonable to me.
        regards, tom lane

On Wed, May 7, 2014 at 2:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I've complained about this problem a few times before: there's nothing
>> to prevent a background worker which doesn't request shared memory
>> access from calling InitProcess() and then accessing shared memory
>> anyway.  The attached patch is a first crack at fixing it.
>
>> Comments?
>
> Looks reasonable to me.

Thanks for the fast review.  Committed, after fixing one further bug I spotted.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 07/05/14 20:37, Robert Haas wrote:
> At a minimum, it's got to be better than the status quo, where shared
> memory is accessible throughout the entire lifetime of
> non-shmem-access background workers.
>

Seems reasonable to me, it might need to be revisited to at least try to 
figure out if we can make EXEC_BACKEND safer, but it's definitely better 
than the current state.


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