runtime: Exception caught in FormClosing / FormClosed lead to crash in coreclr.dll

@kirsan31 @weltkante would one of you be able to verify the fix for #2240 locally? I verified that the sample app provided no longer crashes, but it would be nice to make sure that it solves the original issue for you before going through the servicing process.

I created a release on my fork of coreclr with binaries with the fix: https://github.com/davmason/coreclr/releases/tag/0.0.1

To test it out you would have to:

• Close everything using dotnet core
• Make a backup of everything in C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App\3.1.7 to somewhere safe so you can restore it after (I usually copy the whole folder to 3.1.7-clean in the same directory, so when I’m done I can delete the 3.1.7 folder and rename 3.1.7-clean and know everything is back to normal)
• Copy the files in that release to C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App\3.1.7
• Restart VS and run your scenario that was crashing
• Once you’ve validated it, you can delete C:\Program Files (x86)\dotnet\shared\Microsoft.NETCore.App\3.1.7 and restore from your clean backup and everything will be back to normal

Please let me know if you run in to any issues

Great work getting a repro, I’ve reduced it further. Any caught exception in FormClosing handler together with a nontrivial workload will trigger this. Additional requirements from your previous post for completeness:

• .NET Core 3.1 (does not trigger on .NET 5 preview 7)
• targeting x86
• TieredCompilation disabled
• VS debugger attached (otherwise the stack instrumentation is probably off)

Sounds like a runtime issue to me.

What do you think - how is dangerous to run such an app in production?

Depends on the nature of the stack corruption, considering that its only noticeable when the debugger/runtime are specifically looking for stack corruptions this might not be dangerous. Generally stack corruptions can be security issues so this should definitely be looked at to make sure. (After all we don’t know if this bug can also be triggered in an ASP.NET Core server application.)

static class Program
{
    [STAThread]
    static void Main()
    {
        Application.EnableVisualStyles();
        Application.SetCompatibleTextRenderingDefault(false);

        bool first = true;

        var form = new Form { Text = "Try closing me", Width = 400 };
        form.FormClosing += (sender, e) =>
        {
            if (first)
            {
                first = false;
                form.Text = "Wait for crash (or close to exit)";
                e.Cancel = true;

                // caught exception triggers the bug
                try { throw new InvalidOperationException(); }
                catch { }

                // together with a nontrivial workload
                Application.Idle += delegate
                {
                    object data;
                    for (int i = 0; i < 100; i++)
                        data = new byte[10000];
                };
            }
        };

        Application.Run(form);

        MessageBox.Show("Closed successfully (no crash).");
    }
}

I’ve tested with the original and the reduced repro scenario, thats both fixed, but lets wait for @kirsan31 in case he wants to test in his actual environment. As far as I’m concerned the fix looks good, great work.

This is caused by #2240. I was able to debug to the point where I saw that the frame chain was corrupted, and then by searching through issues that were fixed between 3.1 and 5.0 found this suspiciously similar issue.

I don’t know exactly why this only repros under the debugger, but I validated that applying the fix for #2240 makes the provided app no longer crash so I am pretty confident this is the same issue.

@janvorli is there anything preventing us from porting the fix back to 3.1?

Even with all modules loaded I can’t trap the exception

It is interesting to note, the first time I try to close the child form I get this:

Exception thrown: 'System.FormatException' in System.Private.CoreLib.dll
Exception thrown: 'System.Reflection.TargetInvocationException' in System.Private.CoreLib.dll
Exception thrown: 'System.FormatException' in System.Windows.Forms.dll

The second time I try to close the form I get this:

Exception thrown: 'System.FormatException' in Unknown Module.
Exception thrown: 'System.Reflection.TargetInvocationException' in Unknown Module.
Exception thrown: 'System.FormatException' in Unknown Module.

After which point the app crashes (with some delay as mentioned above):

The thread 0x8ddc has exited with code 0 (0x0).
The program '[35408] bindingSource.exe: Program Trace' has exited with code 0 (0x0).
The program '[35408] bindingSource.exe' has exited with code -1073740791 (0xc0000409).

@danmosemsft this looks like this bug is upstream from Windows Forms.

Fixed in dotnet/coreclr#28090

Thank you both for validating the fix. I opened a PR to port it back to 3.1 in https://github.com/dotnet/coreclr/pull/28090.

@davmason Tested with our original project and provided repro - all works good. Thank you!

Since the problem does appear to reproduce on 5.0

I don’t think it is, only 3.1 appears to be affected.

no, the diagnostic instrumentation detects a stack overrun, there is never any AV reported, not even with debugger attached

Stack cookie instrumentation code detected a stack-based buffer overrun.

So something writes the stack in some place that its not supposed to write in, typical stack corruption. Not all kinds of stack corruptions lead to AV.

Its unclear what the consequences are, i.e. if this can have any negative effect or just happens to always overwrite unused stack memory.

IMHO the first step is to figure out what this “stack instrumentation” is - some CLR runtime feature enabled when the debugger is attached? something VS instruments by itself? Can it be enabled with windbg attached instead of VS?

Once its clear what diagnostic feature is detecting the overrun it may be easier to enable it in context of a TTD time travel session and look who does the bad write.

Since this only repros with Debugger attached moving to Diagnostics for now

erm, maybe I’m misunderstanding, but just because a debugger notifies you of a bug doesn’t mean its causing the bug, or the bug isn’t present when nobody is looking.

This shouldn’t be about fixing the diagnostic/debugger experience, this should be about investigating whether this is a security-relevant stack corruption present in a .NET Core 3 LTS release.

Must have:

• Nlog loaded: private static Logger _logger = LogManager.GetCurrentClassLogger();
• private static Font _BoldDefFont; and in constructor: _BoldDefFont = new Font(DefaultFont, FontStyle.Bold);
• <TieredCompilation>false</TieredCompilation> in csproj.
• .net core + x86 + VS debugger attached.
• Wrong input in BindingSource + attempt to close form.
• Some action after that - really some sort of action (in code or by user), in repro you can move mouse over ittems in File menu for some time.

Repro proj: bindingSource.zip

So, for now candidates:

• Nlog
• WinForms
• CoreCLR
• .NET Runtime
• VS
• Mix of them?

What do you think - how is dangerous to run such an app in production?

Got it! It’s related to Nlog (simple logger instance - no logging at all) and this line of code in constructor of main form:

public Form2()
{
    InitializeComponent();
    _BoldDefFont = new Font(DefaultFont, FontStyle.Bold);
}

and one more condition: <TieredCompilation>false</TieredCompilation> 😲 I will provide a repro a bit later…

with debugger attached only.

I assume the debugger just adds instrumentation which reports the issue, but it probably also exists without debugger, just nobody notices it.

Is it possible to reproduce with WinDbg instead of VS? (Available as app in the store, no need to install any sdk.) Then it might be possible to record a TTD trace which was immensely helpful when looking at memory corruptions.