Skip to content

Commit 46b3c01

Browse files
timcasselltimcassell
authored
Fix linux crash from disassembler (#2413)
* Update dac vulnerable version. * Skip 0 and -1 addresses. * Check for minimum address. * Read file instead of bash. * Use SystemPageSize * Add OS validation. * Some cleanup.
1 parent 8227bbf commit 46b3c01

File tree

4 files changed

+67
-63
lines changed

4 files changed

+67
-63
lines changed

‎src/BenchmarkDotNet/Disassemblers/Arm64Disassembler.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
247247
}
248248
}
249249
}
250-
TryTranslateAddressToName(address, isPrestubMD, state, isIndirect, depth, currentMethod);
250+
TryTranslateAddressToName(address, isPrestubMD, state, depth, currentMethod);
251251
}
252252

253253
accumulator.Feed(instruction);

‎src/BenchmarkDotNet/Disassemblers/ClrMdV2Disassembler.cs

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,32 @@ namespace BenchmarkDotNet.Disassemblers
1414
// This Disassembler uses ClrMd v2x. Please keep it in sync with ClrMdV1Disassembler (if possible).
1515
internal abstract class ClrMdV2Disassembler
1616
{
17-
// Translating an address to a method can cause AV and a process crash (https://github.com/dotnet/BenchmarkDotNet/issues/2070).
18-
// It was fixed in https://github.com/dotnet/runtime/pull/79846,
19-
// and most likely will be backported to 7.0.2 very soon (https://github.com/dotnet/runtime/pull/79862).
20-
protected static readonly bool IsVulnerableToAvInDac = !RuntimeInformation.IsWindows() && Environment.Version < new Version(7, 0, 2);
17+
private static readonly ulong MinValidAddress = GetMinValidAddress();
18+
19+
private static ulong GetMinValidAddress()
20+
{
21+
// https://github.com/dotnet/BenchmarkDotNet/pull/2413#issuecomment-1688100117
22+
if (RuntimeInformation.IsWindows())
23+
return ushort.MaxValue + 1;
24+
if (RuntimeInformation.IsLinux())
25+
return (ulong) Environment.SystemPageSize;
26+
if (RuntimeInformation.IsMacOS())
27+
return RuntimeInformation.GetCurrentPlatform() switch
28+
{
29+
Environments.Platform.X86 or Environments.Platform.X64 => 4096,
30+
Environments.Platform.Arm64 => 0x100000000,
31+
_ => throw new NotSupportedException($"{RuntimeInformation.GetCurrentPlatform()} is not supported")
32+
};
33+
throw new NotSupportedException($"{System.Runtime.InteropServices.RuntimeInformation.OSDescription} is not supported");
34+
}
35+
36+
private static bool IsValidAddress(ulong address)
37+
// -1 (ulong.MaxValue) address is invalid, and will crash the runtime in older runtimes. https://github.com/dotnet/runtime/pull/90794
38+
// 0 is NULL and therefore never valid.
39+
// Addresses less than the minimum virtual address are also invalid.
40+
=> address != ulong.MaxValue
41+
&& address != 0
42+
&& address >= MinValidAddress;
2143

2244
internal DisassemblyResult AttachAndDisassemble(Settings settings)
2345
{
@@ -245,13 +267,13 @@ protected static bool TryReadNativeCodeAddresses(ClrRuntime runtime, ClrMethod m
245267
return false;
246268
}
247269

248-
protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD, State state, bool isIndirectCallOrJump, int depth, ClrMethod currentMethod)
270+
protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD, State state, int depth, ClrMethod currentMethod)
249271
{
250-
var runtime = state.Runtime;
251-
252-
if (state.AddressToNameMapping.ContainsKey(address))
272+
if (!IsValidAddress(address) || state.AddressToNameMapping.ContainsKey(address))
253273
return;
254274

275+
var runtime = state.Runtime;
276+
255277
var jitHelperFunctionName = runtime.GetJitHelperFunctionName(address);
256278
if (!string.IsNullOrEmpty(jitHelperFunctionName))
257279
{
@@ -260,9 +282,9 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,
260282
}
261283

262284
var method = runtime.GetMethodByInstructionPointer(address);
263-
if (method is null && (address & ((uint)runtime.DataTarget.DataReader.PointerSize - 1)) == 0)
285+
if (method is null && (address & ((uint) runtime.DataTarget.DataReader.PointerSize - 1)) == 0)
264286
{
265-
if (runtime.DataTarget.DataReader.ReadPointer(address, out ulong newAddress) && newAddress > ushort.MaxValue)
287+
if (runtime.DataTarget.DataReader.ReadPointer(address, out ulong newAddress) && IsValidAddress(newAddress))
266288
{
267289
method = runtime.GetMethodByInstructionPointer(newAddress);
268290

@@ -276,31 +298,24 @@ protected void TryTranslateAddressToName(ulong address, bool isAddressPrecodeMD,
276298

277299
if (method is null)
278300
{
279-
if (isAddressPrecodeMD || !IsVulnerableToAvInDac)
301+
var methodDescriptor = runtime.GetMethodByHandle(address);
302+
if (methodDescriptor is not null)
280303
{
281-
var methodDescriptor = runtime.GetMethodByHandle(address);
282-
if (!(methodDescriptor is null))
304+
if (isAddressPrecodeMD)
283305
{
284-
if (isAddressPrecodeMD)
285-
{
286-
state.AddressToNameMapping.Add(address, $"Precode of {methodDescriptor.Signature}");
287-
}
288-
else
289-
{
290-
state.AddressToNameMapping.Add(address, $"MD_{methodDescriptor.Signature}");
291-
}
292-
return;
306+
state.AddressToNameMapping.Add(address, $"Precode of {methodDescriptor.Signature}");
293307
}
308+
else
309+
{
310+
state.AddressToNameMapping.Add(address, $"MD_{methodDescriptor.Signature}");
311+
}
312+
return;
294313
}
295314

296-
if (!IsVulnerableToAvInDac)
315+
var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);
316+
if (!string.IsNullOrEmpty(methodTableName))
297317
{
298-
var methodTableName = runtime.DacLibrary.SOSDacInterface.GetMethodTableName(address);
299-
if (!string.IsNullOrEmpty(methodTableName))
300-
{
301-
state.AddressToNameMapping.Add(address, $"MT_{methodTableName}");
302-
return;
303-
}
318+
state.AddressToNameMapping.Add(address, $"MT_{methodTableName}");
304319
}
305320
return;
306321
}

‎src/BenchmarkDotNet/Disassemblers/DisassemblyDiagnoser.cs

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -109,24 +109,31 @@ public IEnumerable<ValidationError> Validate(ValidationParameters validationPara
109109
yield return new ValidationError(true, "Currently NativeAOT has no DisassemblyDiagnoser support", benchmark);
110110
}
111111

112-
if (RuntimeInformation.IsLinux() && ShouldUseClrMdDisassembler(benchmark))
112+
if (ShouldUseClrMdDisassembler(benchmark))
113113
{
114-
var runtime = benchmark.Job.ResolveValue(EnvironmentMode.RuntimeCharacteristic, EnvironmentResolver.Instance);
115-
116-
if (runtime.RuntimeMoniker < RuntimeMoniker.NetCoreApp30)
117-
{
118-
yield return new ValidationError(true, $"{nameof(DisassemblyDiagnoser)} supports only .NET Core 3.0+", benchmark);
119-
}
120-
121-
if (ptrace_scope.Value == "2")
114+
if (RuntimeInformation.IsLinux())
122115
{
123-
yield return new ValidationError(false, $"ptrace_scope is set to 2, {nameof(DisassemblyDiagnoser)} is going to work only if you run as sudo");
124-
}
125-
else if (ptrace_scope.Value == "3")
126-
{
127-
yield return new ValidationError(true, $"ptrace_scope is set to 3, {nameof(DisassemblyDiagnoser)} is not going to work");
116+
var runtime = benchmark.Job.ResolveValue(EnvironmentMode.RuntimeCharacteristic, EnvironmentResolver.Instance);
117+
118+
if (runtime.RuntimeMoniker < RuntimeMoniker.NetCoreApp30)
119+
{
120+
yield return new ValidationError(true, $"{nameof(DisassemblyDiagnoser)} supports only .NET Core 3.0+", benchmark);
121+
}
122+
123+
if (ptrace_scope.Value == "2")
124+
{
125+
yield return new ValidationError(false, $"ptrace_scope is set to 2, {nameof(DisassemblyDiagnoser)} is going to work only if you run as sudo");
126+
}
127+
else if (ptrace_scope.Value == "3")
128+
{
129+
yield return new ValidationError(true, $"ptrace_scope is set to 3, {nameof(DisassemblyDiagnoser)} is not going to work");
130+
}
128131
}
129132
}
133+
else if (!ShouldUseMonoDisassembler(benchmark))
134+
{
135+
yield return new ValidationError(true, $"Only Windows and Linux are supported in DisassemblyDiagnoser without Mono. Current OS is {System.Runtime.InteropServices.RuntimeInformation.OSDescription}");
136+
}
130137
}
131138
}
132139

‎src/BenchmarkDotNet/Disassemblers/IntelDisassembler.cs

Lines changed: 1 addition & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -109,14 +109,7 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
109109
}
110110
}
111111
}
112-
113-
if (address > ushort.MaxValue)
114-
{
115-
if (!IsVulnerableToAvInDac || IsCallOrJump(instruction))
116-
{
117-
TryTranslateAddressToName(address, isPrestubMD, state, isIndirect, depth, currentMethod);
118-
}
119-
}
112+
TryTranslateAddressToName(address, isPrestubMD, state, depth, currentMethod);
120113
}
121114

122115
yield return new IntelAsm
@@ -130,17 +123,6 @@ protected override IEnumerable<Asm> Decode(byte[] code, ulong startAddress, Stat
130123
}
131124
}
132125

133-
private static bool IsCallOrJump(Instruction instruction)
134-
=> instruction.FlowControl switch
135-
{
136-
FlowControl.Call => true,
137-
FlowControl.IndirectCall => true,
138-
FlowControl.ConditionalBranch => true,
139-
FlowControl.IndirectBranch => true,
140-
FlowControl.UnconditionalBranch => true,
141-
_ => false
142-
};
143-
144126
private static bool TryGetReferencedAddress(Instruction instruction, uint pointerSize, out ulong referencedAddress)
145127
{
146128
for (int i = 0; i < instruction.OpCount; i++)

0 commit comments

Comments
 (0)