Skip to content

Commit a3c354f

Browse files
author
Mike McLaughlin
authored
Fix DAC verify signature issue (#5470)
Running `clrstack` a second time after a verify DAC failure succeeds. Runtime.GetDacFilePath wasn't caching the verifySignature parameter so the DAC wasn't being verified once the dac path was cached. Verify the DAC path passed to the DBI entry points for the `clrstack -i` path in RuntimeWrapper.CreateCorDebugProcess. Display the ISettingsService values in both `runtimes` and `sosstatus`.
1 parent 3a08702 commit a3c354f

File tree

6 files changed

+35
-15
lines changed

6 files changed

+35
-15
lines changed

src/Microsoft.Diagnostics.DebugServices.Implementation/Runtime.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ public class Runtime : IRuntime, IDisposable
2525
private Version _runtimeVersion;
2626
private ClrRuntime _clrRuntime;
2727
private string _dacFilePath;
28+
private bool _verifySignature; // This only applies to the regular DAC, not the CDAC
2829
private string _cdacFilePath;
2930
private string _dbiFilePath;
3031

@@ -113,19 +114,22 @@ public string GetCDacFilePath()
113114

114115
public string GetDacFilePath(out bool verifySignature)
115116
{
116-
verifySignature = false;
117117
if (_settingsService.ForceUseContractReader)
118118
{
119+
// Don't verify signature when using the CDAC and don't change the cached value
120+
// because it only applies to the regular DAC in _dacFilePath.
121+
verifySignature = false;
119122
return GetCDacFilePath();
120123
}
121124
if (_dacFilePath is null)
122125
{
123126
_dacFilePath = GetLibraryPath(DebugLibraryKind.Dac);
124127
if (_dacFilePath is not null)
125128
{
126-
verifySignature = _settingsService.DacSignatureVerificationEnabled;
129+
_verifySignature = _settingsService.DacSignatureVerificationEnabled;
127130
}
128131
}
132+
verifySignature = _verifySignature;
129133
return _dacFilePath;
130134
}
131135

@@ -339,7 +343,8 @@ public override string ToString()
339343
if (_dacFilePath is not null)
340344
{
341345
sb.AppendLine();
342-
sb.Append($" DAC: {_dacFilePath}");
346+
string verify = _verifySignature ? "(verify)" : "(don't verify)";
347+
sb.Append($" DAC: {_dacFilePath} {verify}");
343348
}
344349
if (_cdacFilePath is not null)
345350
{

src/Microsoft.Diagnostics.ExtensionCommands/Host/CommandFormatHelpers.cs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,15 @@ public static class CommandFormatHelpers
1616
public const string DebugHeaderExport = "DotNetRuntimeDebugHeader";
1717
public const string ContractDescriptorExport = "DotNetRuntimeContractDescriptor";
1818

19+
public static void DisplaySettingService(this CommandBase command)
20+
{
21+
ISettingsService settingsService = command.Services.GetService<ISettingsService>() ?? throw new DiagnosticsException("Settings service required");
22+
command.Console.WriteLine("Settings:");
23+
command.Console.WriteLine($"-> Use CDAC contract reader: {settingsService.UseContractReader}");
24+
command.Console.WriteLine($"-> Force use CDAC contract reader: {settingsService.ForceUseContractReader}");
25+
command.Console.WriteLine($"-> DAC signature verification check enabled: {settingsService.DacSignatureVerificationEnabled}");
26+
}
27+
1928
/// <summary>
2029
/// Displays the special diagnostics info header memory block (.NET Core 8 or later on Linux/MacOS)
2130
/// </summary>

src/Microsoft.Diagnostics.ExtensionCommands/Host/RuntimesCommand.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,10 +128,8 @@ public override void Invoke()
128128
}
129129
this.DisplayResources(runtime.RuntimeModule, all: false, indent: " ");
130130
this.DisplayRuntimeExports(runtime.RuntimeModule, error: true, indent: " ");
131-
WriteLine($"Use CDAC contract reader: {SettingsService.UseContractReader}");
132-
WriteLine($"Force use CDAC contract reader: {SettingsService.ForceUseContractReader}");
133-
WriteLine($"DAC signature verification check enabled: {SettingsService.DacSignatureVerificationEnabled}");
134131
}
132+
this.DisplaySettingService();
135133
}
136134
}
137135
}

src/Microsoft.Diagnostics.ExtensionCommands/Host/StatusCommand.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ public override void Invoke()
6363
}
6464
}
6565
this.DisplaySpecialInfo();
66+
this.DisplaySettingService();
6667
Write(SymbolService.ToString());
6768

6869
List<Assembly> extensions = new(ServiceManager.ExtensionsLoaded);

src/SOS/SOS.Extensions/HostServices.cs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,10 @@ private int DispatchCommand(
356356
{
357357
return HResult.E_INVALIDARG;
358358
}
359+
if (_commandService is null)
360+
{
361+
return HResult.E_FAIL;
362+
}
359363
try
360364
{
361365
_commandService.Execute(commandName, commandArguments, string.Equals(commandName, "help", StringComparison.OrdinalIgnoreCase) ? _contextServiceFromDebuggerServices.Services : _servicesWithManagedOnlyFilter);

src/SOS/SOS.Hosting/RuntimeWrapper.cs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,9 @@ private IntPtr CreateClrDataProcess(IntPtr dacHandle)
383383
private IntPtr CreateCorDebugProcess()
384384
{
385385
string dbiFilePath = _runtime.GetDbiFilePath();
386-
string dacFilePath = _runtime.GetDacFilePath(out bool verifySignature);
387-
if (dbiFilePath == null || dacFilePath == null)
386+
if (dbiFilePath == null)
388387
{
389-
Trace.TraceError($"Could not find matching DBI {dbiFilePath ?? ""} or DAC {dacFilePath ?? ""} for this runtime: {_runtime.RuntimeModule.FileName}");
388+
Trace.TraceError($"Could not find matching DBI {dbiFilePath ?? ""} for this runtime: {_runtime.RuntimeModule.FileName}");
390389
return IntPtr.Zero;
391390
}
392391
if (_dbiHandle == IntPtr.Zero)
@@ -415,6 +414,16 @@ private IntPtr CreateCorDebugProcess()
415414
int hresult = 0;
416415
try
417416
{
417+
// This will verify the DAC signature if needed before DBI is passed the DAC path or handle
418+
IntPtr dacHandle = GetDacHandle();
419+
if (dacHandle == IntPtr.Zero)
420+
{
421+
return IntPtr.Zero;
422+
}
423+
424+
// The DAC was verified in the GetDacHandle call above. Ignore the verifySignature parameter here.
425+
string dacFilePath = _runtime.GetDacFilePath(out bool _);
426+
418427
OpenVirtualProcessImpl2Delegate openVirtualProcessImpl2 = SOSHost.GetDelegateFunction<OpenVirtualProcessImpl2Delegate>(_dbiHandle, "OpenVirtualProcessImpl2");
419428
if (openVirtualProcessImpl2 != null)
420429
{
@@ -436,12 +445,6 @@ private IntPtr CreateCorDebugProcess()
436445
return corDebugProcess;
437446
}
438447

439-
IntPtr dacHandle = GetDacHandle();
440-
if (dacHandle == IntPtr.Zero)
441-
{
442-
return IntPtr.Zero;
443-
}
444-
445448
// On Linux/MacOS the DAC module handle needs to be re-created using the DAC PAL instance
446449
// before being passed to DBI's OpenVirtualProcess* implementation. The DBI and DAC share
447450
// the same PAL where dbgshim has it's own.

0 commit comments

Comments
 (0)