Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RTL] Sync actctx.c to wine 9.18 #7517

Draft
wants to merge 26 commits into
base: master
Choose a base branch
from

Conversation

tkreuzer
Copy link
Contributor

@tkreuzer tkreuzer commented Nov 20, 2024

Purpose

Sync actctx.c to wine 9.18.

Tests

🟥 KVM x86: https://reactos.org/testman/compare.php?ids=98835,98839,98874,98889

  • comctl32:tooltips: +64 failures

  • kernel32:actctx: +6 failures

  • ole32:dragdrop: +31 failures

  • KVM x64:

@tkreuzer tkreuzer self-assigned this Nov 20, 2024
@binarymaster binarymaster added the 3rd party sync Updating 3rd party components, such as Wine and others label Nov 20, 2024
tkreuzer and others added 22 commits November 22, 2024 15:08
Signed-off-by: Nikolay Sivov <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id d3ce5fff85131cea5170b6d3646d21b268efb66e by Nikolay Sivov <[email protected]>
…bal variable.

Signed-off-by: Michael Stefaniuc <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id cdf1b61a47a2fee8befb2ccbcce4e891f60a24f8 by Michael Stefaniuc <[email protected]>
Signed-off-by: Michael Stefaniuc <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id 3b4239351a6e6028b8b63246042a68be26bfb7be by Michael Stefaniuc <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id 58eceff167ad13530f4841b2b606472d34bb392c by Alexandre Julliard <[email protected]>
…ity element.

Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id 2d519f5aa451513a6e12dec7855c08cd1b8d12fb by Alexandre Julliard <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id 8f3383f741732f418493f60dc780bd36de7a02bd by Alexandre Julliard <[email protected]>
…aded from file.

Signed-off-by: Dmitry Timoshkov <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id 33bc90e6873f9663550964df1e2f49515d9c5580 by Dmitry Timoshkov <[email protected]>
Signed-off-by: Rémi Bernon <[email protected]>
Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id c944f64166fd8e5f0b7a233d76b3e11292ea3870 by Rémi Bernon <[email protected]>
…paces.

Signed-off-by: Alexandre Julliard <[email protected]>

wine commit id 1326687feb2b66d8013034387cfb47ea1bb99e7e by Alexandre Julliard <[email protected]>
wine commit id 0adec25f92219b5626b8ee3c1c13396d36e56ed3 by Alexandre Julliard <[email protected]>
…ETE.

Today, RtlCreateActivationContext (CreateActCtxW) opens the source
manifest file via NtOpenFile without the FILE_SHARE_DELETE sharing mode.

This causes CreateActCtxW to fail if the source manifest file was
created with the FILE_DELETE_ON_CLOSE flag.  FILE_DELETE_ON_CLOSE is
often used for temporary files that should be automatically deleted
after use, even if the creator process crashes.

Fix this by specifying FILE_SHARE_DELETE for sharing mode when opening
the source manifest or module file.  This allows the source manifest or
module file to be marked as deleted while it is open.

Note that concurrent deletion is not an issue for the following reasons:

- The ability to read from an open file handle is unaffected by deletion
  of the corresponding file's name.

- RtlCreateActivationContext does not open the source manifest or module
  file by the given filename (lpSource) more than once.

wine commit id bc854efd7c20802a28499779dc35e3394ff9ac5e by Jinoh Kang <[email protected]>
…a helper function.

This refactoring makes it easier to change the algorithm of obtaining
the current activation context stack.

Note that RtlAddRefActivationContext(NULL) is a no-op, and
check_actctx(NULL) returns NULL without doing anything.

wine commit id 4dadf7a0deffdacc82e056f93818b4bc31a17402 by Jinoh Kang <[email protected]>
… a local variable.

This refactoring makes it easier to change the algorithm of obtaining
the current activation context stack.

wine commit id 68e354f3e457600351f2b847bad2ebf57a840134 by Jinoh Kang <[email protected]>
…ncing ActivationContextStack directly.

This allows changing the location of activation context stack if it
should be put somewhere else (e.g. inside the fiber structure).

wine commit id 41a193c41813eb393662f319f24722ad9b8c59ff by Jinoh Kang <[email protected]>
wine commit id 7c4eb574f036053b9687b874874c5c1700d78d53 by Jinoh Kang <[email protected]>
…ion.

Instead of multiplying it by 2.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=54808
Co-authored-by: Nikolay Sivov <[email protected]>

wine commit id 7af7ff872b3b1f3ab82a2b666cae2bd3d9401006 by Alex Henrie <[email protected]>
…ctivationContext.

This prevents passing NULL resource name to get_manifest_in_module().

wine commit id d992e2d0aa8b59b6213c0b29ac45147a7ae10d2b by Jinoh Kang <[email protected]>
…ng up dependency assembly.

This allows any manifest resource IDs (e.g.,
ISOLATIONAWARE_MANIFEST_RESOURCE_ID) to be recognized when looking up
the assembly manifest of a dependency.

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=18889

wine commit id d0d472bb3e8680e286e404a73fceb29cebe85b73 by Jinoh Kang <[email protected]>
… to allocate memory.

Many built-in callers of ActivateActCtx() just assume that it will
always succeed.  If it ever fails, then DeactivateActCtx() will notice
that the cookie is invalid and raise an exception anyway.

wine commit id 86893ce299223d6dbb2a58cd06d2b8ca8dec26c8 by Jinoh Kang <[email protected]>
Signed-off-by: Nikolay Sivov <[email protected]>

wine commit id a81535d8c995d7c29a084f669842d667ea549c25 by Nikolay Sivov <[email protected]>
Signed-off-by: Eric Pouech <[email protected]>

wine commit id 6558611fa2d24447297cb62d168b924c33839c43 by Eric Pouech <[email protected]>
winesync added 4 commits November 22, 2024 15:10
…nContext().

Signed-off-by: Dmitry Timoshkov <[email protected]>

wine commit id 909204005b2510db1314dd48b2ab3e49ebd49d9a by Dmitry Timoshkov <[email protected]>
wine commit id 6a153ac33e25d7cbd5633aa4b9ab95eddf94eeab by Alexandre Julliard <[email protected]>
… ARM64EC.

wine commit id 6e6450fd96f2ab3ab1535a2c0a9b0efd8e84802d by Alexandre Julliard <[email protected]>
#elif defined __x86_64__
static const WCHAR current_archW[] = {'a','m','d','6','4',0};
static const WCHAR current_archW[] = L"amd64";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In another 10 years they might even start using c++ for COM

FILE_SHARE_READ | FILE_SHARE_DELETE, FILE_SYNCHRONOUS_IO_ALERT );
}

static NTSTATUS find_first_manifest_resource_in_module( HANDLE hModule, const WCHAR **resname )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems fishy, iirc windows does not use just any manifest, but the numbers actually have meaning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CREATEPROCESS_MANIFEST_RESOURCE_ID is 1 so it should be found first. In practice I don't think anyone has more than one manifest? Does ROS support ISOLATION_AWARE_ENABLED at all?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CREATEPROCESS_MANIFEST_RESOURCE_ID is 1 so it should be found first. In practice I don't think anyone has more than one manifest? Does ROS support ISOLATION_AWARE_ENABLED at all?

ReactOS does partially support this, but there is 1, 2 and 123 that I know of,
just selecting the 'first' one might actually use a manifest when it shouldn't be used.

else if (xml_attr_cmp(&attr, hashW))
else if (xml_attr_cmp(&attr, L"loadFrom"))
{
if (!(dll->load_from = xmlstrdupW(&attr.value))) set_error( xmlbuf );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

neat, we were lacking this undocumented thing indeed, now only to use that while loading dlls!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this help with parts of CORE-19707? It just sets the ACTIVATION_CONTEXT_DATA_DLL_REDIRECTION_PATH_EXPAND flag but where does it expand?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the loadFrom thing, that tells sxs to ignore the default load paths.
I don't think it is related to the ACTIVATION_CONTEXT_DATA_DLL_REDIRECTION_PATH_EXPAND flag, but I am not certain.

This thread suggests that they did indeed apply this as mitigation against dll proxying attacks:
https://groups.google.com/g/innosetup/c/IgoRx-i2LeU

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flag gets set if the loadfrom path contains a %.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right,
but considering we completely ignored loadFrom until now I doubt we would do anything with one of it's flags.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party sync Updating 3rd party components, such as Wine and others
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants