Implement versioning system for DLLs #3239

tkreuzer · 2020-09-26T16:46:21Z

Purpose

This PR implement a system that allows to make DLL exports version specific at runtime, based on the applications compatibility setting. It still allows ReactOS DLLs to statically import functions from higher versions, even if they are hidden to the application.

JIRA issue: CORE-11288

Proposed changes

Add a feature to spec2def to generate a RosCompat descriptor (contained in a special section) pointing to a mask-array, that specifies for each named export on which Windows versions it is present.
Add a feature to the loader in NTDLL that checks for a RosCompat descriptor, and if present, uses it to patch the export table, splitting it into a public and private part.
Modify the loader to allow ReactOS DLLs (those that contain a RosCompat descriptor, even if it is empty) to import from private/hidden exports.

Suggested to review by commit.

Problem

Currently there is the following problem: When an app maps a dll using MapViewOfFile it will see all exports unpatched, if it uses GetModuleHandle() it will get the patched version, so there will be inconsistencies between the exports. This was highlited by kernel32_winetest:loader, which is currently hacked to not crash on that situation. This should obviously be fixed.

Possible solutions:

Patch RtlImageDirectoryEntryToData to detect an unpatched dll and apply the patching there, too. This would solve this scenario, but not the case, where an app looks up the entry manually.
Patch NtMapViewOfSection in ntdll to apply the patching. This would solve all scenarios, except loading the file with raw file access. But it would also create an Nt* function in ntdll, which doesn't match the expected asm stub, breaking applications that enumerate system calls.
Move the patching to kernel mode. This would solve the problem with the systemcall stub, but not the problem with direct file reading.
Abandon the idea of patching the exports and instead develop a completely new system based on kernel mode file system redirection. That would theoretically solve all the issues, but is clearly somewhat tricky and would take a while to develop.
Temporarily use one of the first 3 options, until option 4 is ready.
Feedback/opinions welcome.

TODO

Run on testbot
Fix inconsistency problem

Now spec2def uses the version annotations in spec files to generate a table, which will be used to dynamically patch the export table based on the current process' appcompat settings, when a dll is initialized, allowing to export the appropriate functions for each version. This feature is still disabled in config.cmake with the variable DISABLE_EXPORT_VERSIONING.

The loader checks for a ros-compat-descriptor, and if one is found, parses it's entries. Each entry corresponds to an export and provides a bitmask that specifies on what Windows version the export exists. If the bit for the process' appcompat version (default is still Windows 2003) is set, the export will be kept, otherwise the export will be moved into a second (private) export table. If a module has a ros-compat-descritor, it is still allowed to import these hidden exports.

so that winhttp can use nt6+ apis

This is required, because with dynamically patched export tables we will export all vista functions from the main dll, and you can link to them at compile time, but they will not be resolved at runtime, unless the importer has a roscompat-descriptor. Linking the vista version first will resolve the imports from the vista dll.

Create kernel32_vista_static library and link both kernel32_vista and kernel32 to it. Export some vista functions from kernel32.

Extravert-ir · 2020-09-26T20:54:52Z

dll/apisets/api-ms-win-crt-private-l1-1-0.spec

 @ stub __intrinsic_setjmp
 @ stub __intrinsic_setjmpex
 @ stub __processing_throw
 @ stub __report_gsfailure
 @ stub __std_exception_copy
 @ stub __std_exception_destroy
+@ stub -arch=x86_65 __std_terminate


Suggested change

@ stub -arch=x86_65 __std_terminate

@ stub -arch=x86_64 __std_terminate

The test is mapping ntdll using MapViewOfFile, bypassing the loader, then enumerates it's exports, and then passes the names to GetProcAddress, which is hiding some of the original exports. At this point we do not support this scenario.

wjk · 2020-09-27T18:37:12Z

My 2¢ on the problem listed in the OP: How often do applications legitimately (i.e., not testing against or depending on undocumented or semi-documented behavior) load a DLL using MapViewOfFile()? If it isn't that common a use-case, I would suggest option 1 until option 4 can be implemented (possibly on top of the SxS redirection system).

Also, what do you mean by “if an app looks up the entry manually”? If that means parsing the PE file format by hand in-memory to find, relocate, and call export addresses, this looks to me like an even rarer use-case (except maybe for heavily obfuscated software e.g. malware). The feasibility of this approach depends on how likely it is that normal, correctly written applications engage in these two behaviors, and you would have more knowledge of this than I. Hope this helps!

andrew-boyarshin · 2020-09-28T04:59:06Z

I propose taking the current approach for now (neither of the 1-3), dropping the hack that makes the loader tests pass ok, open an issue to implement kernel-mode FS redirection approach (4) instead, add the issue number to the loader test to be printed before marking test as failed. Make it an explicitly acknowledged regression so that there will be an incentive to fix it.

jschueller · 2020-10-06T08:58:38Z

is this good to go ? (except for the x86_65 typo :)

zefklop · 2020-10-16T07:40:40Z

sdk/tools/spec2def/spec2def.c

+}
+
+void
+Output_RosCompatDescriptor(FILE *file, EXPORT *pexports, unsigned int cExports)


I'd suggest generating the regular export directory, not use the .def file and call the C file <module>_exports.c"

This way, everyone gets a generated C file and we are not bitten when we try to change this and that, because some modules have a stubs file and some don't

zefklop

Why don't you simply merge xxx and xxx_vista modules ?

zefklop · 2020-10-16T07:44:54Z

sdk/tools/spec2def/spec2def.c

+    /// HACK: should make a copy
+    qsort(pexports, cExports, sizeof(EXPORT), CompareExports);
+
+    fprintf(file, "ULONG __roscompat_export_masks__[] =\n{\n");


Shouldn't this belong to the ".expvers" section ?

zefklop · 2020-10-16T07:55:37Z

dll/ntdll/ldr/appcompat.c

+        Peb->OSMinorVersion = AppCompatVersion & 0xFF;
+    }
+
+    DPRINT("roscompat: Patching eat of %wZ for 0x%x\n", &LdrEntry->BaseDllName, AppCompatVersion);


EAT (Export Address Table)

zefklop · 2020-10-16T08:58:44Z

dll/ntdll/include/ntdllp.h

+NTAPI
+LdrpApplyRosCompatMagic(PLDR_DATA_TABLE_ENTRY LdrEntry);
+
+extern char __ImageBase;


What is this used for ?

zefklop · 2020-10-16T14:21:03Z

dll/ntdll/ldr/appcompat.c

+#if 0
+static
+NTSTATUS
+GetExportNameVersionTable(
+    _In_ PVOID ImageBase,
+    _Out_ PULONG *Table,
+    _Out_ PULONG NumberOfEntries)
+{
+    PROSCOMPAT_DESCRIPTOR RosCompatDescriptor;
+
+    RosCompatDescriptor = FindRosCompatDescriptor(ImageBase);
+    if (RosCompatDescriptor == NULL)
+    {
+        return STATUS_NOT_FOUND;
+    }
+
+    *NumberOfEntries = RosCompatDescriptor->NumberOfExportNames;
+    *Table = RosCompatDescriptor->ExportNameMasks;
+
+    return STATUS_SUCCESS;
+}
+#endif


If you don't use it, can you simply remove it ?

zefklop · 2020-10-16T14:26:12Z

dll/ntdll/ldr/appcompat.c

+    for (i = 0; j < RosCompatDescriptor->NumberOfExportNames; i++, j++)
+    {
+        NameTable[j] = OrgNameTable[i];
+        OrdinalTable[j] = OrgOrdinalTable[i];
+    }


Suggested change

for (i = 0; j < RosCompatDescriptor->NumberOfExportNames; i++, j++)

{

NameTable[j] = OrgNameTable[i];

OrdinalTable[j] = OrgOrdinalTable[i];

}

RtlCopyMemory(&NameTable[j], OrgNameTable, (RosCompatDescriptor->NumberOfExportNames - j) * sizeof(OrgNameTable[0]))

RtlCopyMemory(&OrdinalTable[j], OrgOrdinalTable, (RosCompatDescriptor->NumberOfExportNames - j) * sizeof(OrgOrdinalTable[0]))

zefklop · 2020-10-16T14:27:47Z

dll/ntdll/ldr/appcompat.c

+    for (i = 0, j = 0, k = 0; i < ExportDirectory->NumberOfNames; i++)
+    {
+        if (RosCompatDescriptor->ExportNameMasks[i] & VersionMask)
+        {
+            /* Put public functions into the export table */
+            NameTable[j] = OrgNameTable[i];
+            OrdinalTable[j] = OrgOrdinalTable[i];
+            j++;
+        }
+        else
+        {
+            /* Move private functions to the start of the temp buffer */
+            OrgNameTable[k] = OrgNameTable[i];
+            OrgOrdinalTable[k] = OrgOrdinalTable[i];
+            k++;
+        }
+    }


How are we sure that we don't overflow beyond the name table ?

zefklop · 2020-10-16T14:29:55Z

dll/ntdll/ldr/appcompat.c

+        return Status;
+    }
+
+    /* Unprotect the export directory */


Protect again

zefklop · 2020-10-16T14:31:40Z

dll/ntdll/ldr/ldrpe.c

+    if (ImportLdrEntry->PatchInformation != NULL)
+    {
+        SnapFlags |= SNAP_PRIVATE;
+    }


Why do you need this ?

zefklop

Wouldn't it make sense to have spec2def export every function, regardless of the target version ?

zefklop · 2020-10-20T15:19:28Z

sdk/lib/crt/crt.cmake

@@ -368,6 +368,7 @@ if(ARCH STREQUAL "i386")
    list(APPEND CRT_ASM_SOURCE
        except/i386/chkesp.s
        except/i386/prolog.s
+        except/i386/ehandler-asm.s


amd64 ehandler-asm.s missing

zefklop · 2020-10-20T15:53:41Z

sdk/lib/crt/msvcrtex.cmake

@@ -67,7 +69,10 @@ if(ARCH STREQUAL "i386")
        math/i386/cisqrt.c)
 elseif(ARCH STREQUAL "amd64")
    list(APPEND MSVCRTEX_ASM_SOURCE
-        except/amd64/chkstk_ms.s)
+        except/amd64/chkstk_ms.s
+        except/amd64/ehandler-asm.s)


msvcrtex is not linked into msvcrt.dll

jschueller · 2021-11-16T14:37:02Z

hello, any news ?

IniKiwi · 2023-01-18T13:14:45Z

any news ?

tkreuzer and others added 10 commits September 26, 2020 18:31

[ROSCOMPAT_APITEST] Add initial test for export rewriting

f9f1e8b

[COMCTL32] Include roscompat descriptor

afb05f2

[WINHTTP] Include roscompat descriptor,

0540a52

so that winhttp can use nt6+ apis

[NTDLL] Don't derive appcompat version from manifest

973aedb

[KERNEL32] Export more Vista APIs

19f331f

Create kernel32_vista_static library and link both kernel32_vista and kernel32 to it. Export some vista functions from kernel32.

[NTDLL] Link rtl_vista and export some vista functions

5008362

[CMAKE] Update base addresses

7f796e4

tkreuzer added the NT6+ For PRs that aim at implementing NT6+ functionality. label Sep 26, 2020

tkreuzer requested a review from a team September 26, 2020 16:46

tkreuzer requested review from HeisSpiter, learn-more and ThFabba as code owners September 26, 2020 16:46

tkreuzer self-assigned this Sep 26, 2020

tkreuzer added this to New PRs in ReactOS PRs via automation Sep 26, 2020

tkreuzer changed the title ~~Appcompat dll export magic~~ Implement versioning system for DLLs Sep 26, 2020

tkreuzer mentioned this pull request Sep 26, 2020

Implement versioning system for DLLs #1872

Closed

4 tasks

Extravert-ir reviewed Sep 26, 2020

View reviewed changes

tkreuzer added 5 commits September 27, 2020 10:43

[CPPRT][CRT] Move c++ exception handlers to crt

07f9f79

[APISETS] Improve/fix up some exports

1cac444

[MSVCRT] Export _vcwprintf

82a7f1a

[KERNEL32_APITEST] Hack the loader test

ffa1c04

The test is mapping ntdll using MapViewOfFile, bypassing the loader, then enumerates it's exports, and then passes the names to GetProcAddress, which is hiding some of the original exports. At this point we do not support this scenario.

[CMAKE] Finally enable export patching 🎉

42ffa67

tkreuzer force-pushed the appcompat_dll_export_magic branch from 955bc17 to 42ffa67 Compare September 27, 2020 08:45

wjk mentioned this pull request Sep 27, 2020

Second take on DLL export versioning system #3139

Closed

2 tasks

jschueller mentioned this pull request Oct 6, 2020

[KERNEL32] Implement GetFinalPathNameByHandleW #1425

Closed

zefklop reviewed Oct 16, 2020

View reviewed changes

zefklop reviewed Oct 20, 2020

View reviewed changes

binarymaster removed this from New PRs in ReactOS PRs Feb 29, 2024

binarymaster added the enhancement For PRs with an enhancement/new feature. label Feb 29, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement versioning system for DLLs #3239

Implement versioning system for DLLs #3239

tkreuzer commented Sep 26, 2020 •

edited

Extravert-ir Sep 26, 2020

wjk commented Sep 27, 2020

andrew-boyarshin commented Sep 28, 2020

jschueller commented Oct 6, 2020

zefklop Oct 16, 2020 •

edited

zefklop left a comment

zefklop Oct 16, 2020

zefklop Oct 16, 2020

learn-more Nov 16, 2021

zefklop Oct 16, 2020

zefklop Oct 16, 2020

zefklop Oct 16, 2020

zefklop Oct 16, 2020

zefklop Oct 16, 2020

zefklop Oct 16, 2020

zefklop left a comment

zefklop Oct 20, 2020

zefklop Oct 20, 2020

jschueller commented Nov 16, 2021

IniKiwi commented Jan 18, 2023

	@ stub -arch=x86_65 __std_terminate
	@ stub -arch=x86_64 __std_terminate

Implement versioning system for DLLs #3239

Are you sure you want to change the base?

Implement versioning system for DLLs #3239

Conversation

tkreuzer commented Sep 26, 2020 • edited

Purpose

Proposed changes

Problem

TODO

Choose a reason for hiding this comment

wjk commented Sep 27, 2020

andrew-boyarshin commented Sep 28, 2020

jschueller commented Oct 6, 2020

zefklop Oct 16, 2020 • edited

Choose a reason for hiding this comment

zefklop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zefklop left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jschueller commented Nov 16, 2021

IniKiwi commented Jan 18, 2023

tkreuzer commented Sep 26, 2020 •

edited

zefklop Oct 16, 2020 •

edited