From: Jinoh Kang Subject: Re: [PATCH v3 07/13] loader: Don't clobber existing memory mappings when reserving addresses. Message-Id: <5df5d7c7-a354-2f31-21d7-fdf26467fc86@gmail.com> Date: Wed, 26 Jan 2022 12:15:35 +0900 In-Reply-To: <8bbcb555-b498-45d4-2150-692dea843ce9@gmail.com> References: <87y233yfx3.fsf@wine> <8bbcb555-b498-45d4-2150-692dea843ce9@gmail.com> On 1/26/22 11:52, Jinoh Kang wrote: > On 1/26/22 00:48, Alexandre Julliard wrote: >> Jinoh Kang writes: >> >>> Today, the preloader makes no attempt to avoid unmapping existing >>> memory mappings except the initial stack. This results in irrevocably >>> unmapping some useful preallocated memory areas, such as vDSO. >>> >>> Fix this by reading /proc/self/maps for existing VMAs, and splitting >>> mmap() calls to avoid erasing existing memory mappings. >> >> That defeats the purpose of using the preloader. > > The intention was to *incrementally* scrape memory areas for the reserved ranges, > relocating any critical areas (vDSO, stack, ...) along the way. To clarify, the purpose of the preloader is to reserve some predefined (fixed) virtual memory ranges so that it's safe for Wine to use. It does so in the following manner: 1. It reserves the memory ranges so that future mmap() operations will not touch them. 2. It removes all existing references (from e.g. auxv) to the memory ranges. Note that if the range is already mapped, we can skip (1) and only perform (2). We can forcibly do (1) too, but there are the following drawbacks: - There's no point in re-reserving already occupied memory range. The OS will not reuse them for other allocations anyway, especially since we also do (2). - It might actually be harmful if the memory range coincides with memory that is being actively used (e.g. preloader code/data, stack, etc.). Since Wine does not distinguish between normally mapped pages (e.g. preloader code/data stack) and explicitly reserved pages (PROT_NONE), we can leave the existing memory mappings as-is. After all, (2) is what is actually important. > > It's also why this change is useless without the subsequent patches, > which calls map_reserve_preload_ranges again to actually fill out all > the gaps previously occupied by vDSO/stack. > >> The whole point is to make sure the specified ranges are available. > > It is. That's why I leave the preload_info ranges for Wine even though > I don't actually munmap() those pages. > >> Note that since you don't update the ranges info, the mappings will >> get erased by Wine later anyway. > > That was exactly what I intended: after jumping to ld.so, we no longer > need the code/data from preloader (except stack etc.), so we let Wine > unmap them. > > I'll make this clear in the next revision. > > (Note: the patch does update the ranges info when needed, particularly > when it fails to remap() vDSO/stack.) > -- Sincerely, Jinoh Kang