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

RuntimeEventSource.Initialize being called before any user code prevents processes from setting safe and performant process defaults. #97410

Open
jlaanstra opened this issue Jan 23, 2024 · 7 comments

Comments

@jlaanstra
Copy link

jlaanstra commented Jan 23, 2024

Description

RuntimeEventSource.Initialize ends up calling CoWaitForMultipleHandles which ends up calling CoInitializeSecurity. RuntimeEventSource is called before any user code preventing it from configuring COM with better process default via IGlobalOptions or setting different COM security.

Reproduction Steps

Add the following module initializer to a Windows app:

internal static class Initializer
{
    [ModuleInitializer]
    internal static void Initialize()
    {
        Marshal.ThrowExceptionForHR(PInvoke.CoCreateInstance<IGlobalOptions>(new Guid("0000034B-0000-0000-C000-000000000046"), null, CLSCTX.CLSCTX_INPROC_SERVER | CLSCTX.CLSCTX_NO_CODE_DOWNLOAD, out var globalOptions));
        globalOptions->Set(GLOBALOPT_PROPERTIES.COMGLB_EXCEPTION_HANDLING, (nuint)GLOBALOPT_EH_VALUES.COMGLB_EXCEPTION_DONOT_HANDLE_ANY);
        globalOptions->Set(GLOBALOPT_PROPERTIES.COMGLB_RO_SETTINGS, (nuint)GLOBALOPT_RO_FLAGS.COMGLB_FAST_RUNDOWN);
    }
}

Expected behavior

We are able to successfully set COM IGlobalOptions in a module initializer.

Actual behavior

Setting global options fails with RPC_E_TOOLATE as COM security is already initialized.

Regression?

Unclear, but the scenario has worked in production for multiple releases with minimal issues.

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 23, 2024
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jan 23, 2024
@AaronRobinsonMSFT AaronRobinsonMSFT added area-System.Diagnostics-coreclr and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 23, 2024
@AaronRobinsonMSFT
Copy link
Member

AaronRobinsonMSFT commented Jan 23, 2024

This is also related to Interop and is another manifestation of #73527.

/cc @dotnet/interop-contrib @elinor-fung @dotnet/dotnet-diag

@AaronRobinsonMSFT
Copy link
Member

I think we should try and figure out a solution here that doesn't break such core features like basic Profiling. The gist here is that profiling/tracing your application could now render you in a broken state.

@elinor-fung
Copy link
Member

The gist here is that profiling/tracing your application could now render you in a broken state.

It's not even actually profiling/tracing, right? It seems like simply having EventSource support enabled (so default behaviour) will make it so that the application can't configure global COM options?

@hoyosjs hoyosjs added this to the 9.0.0 milestone Jan 23, 2024
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Jan 23, 2024
@delmyers
Copy link
Contributor

Just throwing this out there. An alternative solution to this problem would be to have these options configurable in the app config, so that the CLR can guarantee that they run before app startup and ensure that they conform to the contract that Windows enforces. It would be a less brittle solution.

@delmyers
Copy link
Contributor

For reference, the issue was root-caused by investigation of this feedback ticket for VS:

https://developercommunity.visualstudio.com/t/Unable-to-set-COM-global-options-or-call/10544708

It is also related to .NET runtime issue #73527, and impacts the Windows APP SDK: microsoft/WindowsAppSDK#1727

@AaronRobinsonMSFT
Copy link
Member

I finally found time to look into this. Startup hooks will never work due to the feature guaranteeing certain apartment states - see https://github.com/dotnet/runtime/blob/main/docs/design/features/host-startup-hook.md#threading-behavior.

Looks like a new mechanism would be needed for configuring COM security.

@AaronRobinsonMSFT
Copy link
Member

Speaking with some other people in this area, I realized my assumption here was wrong. This isn't about CoInitializeEx and CoIntializeSecurity, but rather just marshalling an interface. I've tried to create simple repro with little luck.

The expectation is that when I enable the RuntimeEventSource I will get a non-zero HRESULT from CoInitializeSecurity(). Could someone reconfirm this still repros in their scenario?

unsafe partial class Program
{
    static void Main(string[] args)
    {
        const int RPC_C_AUTHN_LEVEL_DEFAULT = 0;
        const int RPC_C_IMP_LEVEL_ANONYMOUS = 1;
        int hr = CoInitializeSecurity(null, -1, null, null, RPC_C_AUTHN_LEVEL_DEFAULT, RPC_C_IMP_LEVEL_ANONYMOUS, null, 0, null);
        Console.WriteLine($"{nameof(CoInitializeSecurity)}() {hr:x}");

        Console.WriteLine("In main... Press any key");
        Console.ReadKey();
    }

    [DllImport("Ole32")]
    private static extern int CoInitializeSecurity(
      void* pSecDesc,
      int cAuthSvc,
      void* asAuthSvc,
      void* pReserved1,
      uint dwAuthnLevel,
      uint dwImpLevel,
      void* pAuthList,
      uint dwCapabilities,
      void* pReserved3);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants