Alexandre Julliard wrote:
Imports are not necessarily contained in the .idata section. Also base relocations can touch just about every code page, so you'd need to unprotect everything. Probably easier to only set up the right permissions after all imports and relocations have been done.
That was my first thought. Then I looked at the call tree for map_image.
map_image is only called by MapViewOfFileEx.
MapViewOfFileEx is called by MapViewOfFile, DPLAYX_ConstructData, HEAP_CreateSystemHeap, and VXD_Win32s. At this point my eyes began to glaze over. But I forged on with looking MapViewOfFile. GAAAH! That function is referenced in no less than sixteen places!
So already if we wanted to set protections sometime after map_image is called, we would have to do it in at least four, if not nineteen, different places -- provided all those calls aren't unified somewhere farther up, but not too far up, the call tree. This is just asking for bugs when someone decides to call MapViewOfFileEx from somewhere else and forgets to set up protections.
That's why I felt it was maybe a better idea to set up the protections beforehand, and then require whoever is modifying readonly data to unprotect and then reprotect that data. The worst that can happen then is that Wine segfaults in that place, which is a reminder that the data is protected and needs to be temporarily unprotected. And if currently we only have to do that in maybe two places, that's a much better proposition than in nineteen (and counting).
I took a quick look at PE_fixup_imports, and it *should* be reasonbly simple to do the protect-unprotect there. I haven't proven it, though, and I haven't looked at the other places where this would need to be done such as in do_relocations and in...
Also note that it is allowed for an app to write to the resource section, even though it is marked read-only. NT sets up an exception handler to unprotect it when necessary; this is supposed to help in finding bugs. So you probably have to do the same thing.
Yes, that should be done, too. I haven't dug that far into NT yet, so that mod may have to come in a future patch.
Anyway, what are your thoughts on this?
Thanks,
--Rob