Closed dmik closed 4 years ago
In theory, there is an option to implement shm_open
API on OS/2 (see https://www.man7.org/linux/man-pages/man7/shm_overview.7.html) but it involves mmap and general LIBC functions like fstat and is a substantially larger task than what Chromium needs. Definitely not an option to go in this regard.
The first version of the API is ready but Chromium requires two more things: memory handle duplication (to manage ownership) and converting read-write memory objects to read-only memory objects (to have read-only views of the same memory). The former is not hard but requires some changes in the data structures. The latter is harsher. The only way to do so is use DosAliasMem
again but this again will brings its low memory limit: aliases seem to be created only in the first 512 M (of which only around 200 M is practically available for a process). This tiny space is, again, prone to OOM conditions (the same issue we want to avoid with the new API in the first place). Given that the new API allows to reuse the same memory object in many mappings it will waste address space much less but I fear that it won't be enough.
The only way we can do here is to ignore this read-only constrain and always supply the application with the same original read-write memory region. This will make address space usage as tight as possible. But I don't know how much impact it will have on Chromium security considerations — my guess is that this is why they want to have read-only mappings in the first place, but this needs checking though.
The other thing to double-check is if DosAliasMem
is actually constrained to the first 512M.
I've tested DosAliasMem
— my assumption is right: it can only get memory in the private area below 512M and in my case it's actually smaller than 200M — it's only about 100M for a new process. And it gets exhausted pretty fast by Chromium. And when this memory ends, it starts returning ERROR_NOT_ENOUGH_MEMORY (8). So it's surely not a way to go for its needs.
As for Chromium, looks like I was right too. Judging by comments in this bug https://bugs.chromium.org/p/chromium/issues/detail?id=789959 and by the actual code that uses read-only handles, Chromium seems to use this whole make read-only handles from read-write handles concept as a security measure to protect from various kinds of attacks to the privileged browser process. But as this bug suggests, this protection is only needed on the process boundary, not within the process itself. I.e. Chromium first allocates a read-write memory object in the "main" process and then, when it wants to share it with some other, unprivileged process, it creates a read-only duplicate of that handle and passes it on to that unprivileged process to ensure that the process won't be able to write to that memory. This also explains why I don't see any calls to this "make read-only duplicate" machinery in my logs — I'm working in single process mode for now, so handles are not passed between processes and this machinery is not involved.
The above means that we don't need DosAliasMem
at all — as each process maintains its own permission mask for shared memory. This, however, is not very useless in terms of security. Any process that has access to shared memory, may write to it. I.e. even if the parent process calls DosAllocSharedMem
with only PAG_READ and passes memory to the child process, a call to e.g. DosGetSharedMemory
with PAG_READ and PAG_WRITE in child succeeds and the memory is indeed writable. More over, even if the parent process gives any other process its shared memory with DosGiveSharedMem
and specifies only read-only access during this transfer, the receiving process may still call DosSetMem
to escalate it to PAG_WRITE and writes then silently succeed. I.e. the OS/2 kernel does not enforce this kind of permission reduction. I doubt we can do anything about that.
OTOH, in Chromium this kind of protection is mostly for user scripts and such (i.e. some sort of a controlled environment if I get it right) and scripts obviously can't call DosGetSharedMem
or DosSetMem
to make a read-only shared region writable in their process as long as the shared memory API receiving the handle from the allocating process makes sure the actual memory is mapped read-only within the receiving process. This is what our API should do then. Scripts attempting to write into such memory will crash their processes with access violation.
Also, it's best to not use OBJ_GETTABLE
when allocating memory in Chromium at all as this way any process may get write access to the shared memory object in question (and do bad things there like malicious code injection and so on). Our shared API should support reducing access only to specific processes (using OBJ_GIVEABLE
and DosGiveSharedMem
).
I've added the new API to Chromium and it works much better now, no OOM conditions! There is one problem with read-only views for read-write handles though. I was wrong above, Chromium does create read-only mappings for read-write memory in the single process mode too. With the current implementation where mapping such a read-only handle will switch all other mappings to read-only (because they share the same virtual address range) and Chromium does not like this. It crashes as following when trying to write to such a mapping done via a read-write handle but after some read-only handle also mapped the very same region for reading:
Filename: D:\CODING\QT5\QT5-DEV-BUILD\QTBASE\LIB\QT5WEBC.DLL (10/04/2020 18:18:07 249,551,353)
Address: 005B:BA329618 (0001:038C9618)
Cause: Attempted to write to B5C2FA7C
(read/exec memory allocated by LIBCX0)
______________________________________________________________________
Call Stack
______________________________________________________________________
EBP Address Module Obj:Offset Nearest Public Symbol
-------- --------- -------- ------------- -----------------------
Trap -> BA329618 QT5WEBC 0001:038C9618 __ZN11visitedlink17VisitedLinkMaster14AddFingerprintEyb + B8 0001:038C9560 (visitedlink_master.o)
Lost Stack chain - invalid EBP F7F37FFE
This happens when going through browser history.
If I disable changing the permissions of the existing memory object from RW to RO when it is mapped through an read-only handle, this crash goes away.
I will try a compromise here: set the memory object to RO only if there are no RW mappings in the current process, and switch it back to RW as soon as any RW mapping is requested (and switch back to RO when the last one is gone). But I will commit the current state first because I want this change to be separately recorded.
With the above compromise regarding R/O and R/W it looks good now. Chromium seems to be happy too. All current goals reached, closing.
Chromium requires a new shared memory API in order to implement required functionality. See https://github.com/bitwiseworks/qtwebengine-chromium-os2/issues/14#issuecomment-684001251 and also https://github.com/bitwiseworks/qtwebengine-os2/issues/6#issuecomment-695041508.
This API should be (obviosly) system-wide. It should operate memory "handles" rather than direct memory addresses (as
DosAllocSharedMem
does). Handles to memory objects are to be "mapped" to the address space of the target process on request in order to be available in this process. And although it's not actual mapping (the way how shared memory is implemented in OS/2 assumes that it is technically mapped into all existing and future processes at allocation time and the mapped address is the same in all processes), this operation will track memory object usage in each process in order to free the memory object when it is unmapped by all processes that mapped it and therefore is no longer in use.Note that the current "hack" in Chromium abuses
DosAliasMem
to work around inaccessible reference counting of the standard OS/2 API but this turned out to be insufficient:DosAliasMem
wastes address space like hell and, given that aliases are always created in low private memory (which is just several hundred megabytes), it's exhausted pretty fast in case of Chromium which is very greedy memory wise especially in 32-bit mode. The new implementation will therefore not useDosAliasMem
. The original address of the shared memory object will be used, which, together with reference counting, minimize both address space and physical memory page usage (in the current hack, we never free the shared memory object because it might get in use in the given process again and we won't ever have access to it if we free it).This needs to be done in LIBCx as it already maintains system-wide shared data for IPC communication and resource management.