Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Petr Jelinek <petr(dot)jelinek(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "Tsunakawa, Takayuki" <tsunakawa(dot)takay(at)jp(dot)fujitsu(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)
Date: 2017-06-05 03:45:35
Message-ID: 14475.1496634335@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> writes:
> On Mon, Jun 5, 2017 at 4:00 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I took a quick look at this, and it seems rather beside the point.

> What I understood from the randomization shm allocation behavior due
> to ASLR is that when we try to allocate memory (say using
> VirtualAllocEx as in our case) at a pre-defined address, it will
> instead allocate it at a different address which can lead to a
> collision.

No. The problem we're concerned about is that by the time we try
pgwin32_ReserveSharedMemoryRegion's call
VirtualAllocEx(hChild, UsedShmemSegAddr, UsedShmemSegSize,
MEM_RESERVE, PAGE_READWRITE)
in the child process, something might have already taken that address
space in the child process, which would cause the call to fail not just
allocate some randomly different space. That can't happen unless the base
executable, or ntdll.dll, or possibly something injected by an antivirus
product, has taken up address space different from what it took up in the
postmaster process. What we are assuming here is that any such variation
is randomized, and so will probably be different in the next child process
launched by the postmaster, allowing a new process launch to possibly fix
the problem. But once that address space is allocated in a given process,
no amount of merely retrying a new allocation request in that process is
going to make it go away. You'd have to do something to free the existing
allocation, and we have no way to do that short of starting over.

> I think the same problem can happen during reattach as well.
> Basically, MapViewOfFileEx can fail to load image at predefined
> address (UsedShmemSegAddr).

Once we've successfully done the VirtualAllocEx call, that should hold
until the VirtualFree call in PGSharedMemoryReAttach, allowing the
immediately-following MapViewOfFileEx call to succeed. Were that not the
case, we'd have had problems even without ASLR. We did have problems
exactly like that before the pgwin32_ReserveSharedMemoryRegion code was
added. So my feeling about this is that retrying the process creation as
in my prototype patch ought to be sufficient; if you think it isn't, the
burden of proof is on you.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jing Wang 2017-06-05 04:20:04 Re: Support to COMMENT ON DATABASE CURRENT_DATABASE
Previous Message Amit Kapila 2017-06-05 03:20:11 Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)