Skip to content

Commit 8655fee

Browse files
[release/10.0] Fix crash in UnsafeComStreamWrapper.Stat with in-memory symbols (#428)
* Fix crash in UnsafeComStreamWrapper.Stat caused by uninitialized STATSTG fields Replace STATSTG with a blittable NativeSTATSTG struct passed by ref to avoid the marshaller processing uninitialized memory (e.g. pwcsName) from native implementations that don't zero-initialize the structure. Always pass STATFLAG_NONAME to prevent name allocation. * Fix field order * Add `StructLayout` --------- Co-authored-by: Gregg Miskelly <greggm@microsoft.com>
1 parent da24ef5 commit 8655fee

4 files changed

Lines changed: 20 additions & 83 deletions

File tree

src/Microsoft.DiaSymReader/Utilities/ComMemoryStream.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -203,12 +203,10 @@ void IUnsafeComStream.SetSize(long libNewSize)
203203
_length = (int)libNewSize;
204204
}
205205

206-
void IUnsafeComStream.Stat(out STATSTG pstatstg, int grfStatFlag)
206+
void IUnsafeComStream.Stat(ref NativeSTATSTG pstatstg, int grfStatFlag)
207207
{
208-
pstatstg = new STATSTG()
209-
{
210-
cbSize = _length
211-
};
208+
pstatstg = default;
209+
pstatstg.cbSize = _length;
212210
}
213211

214212
unsafe void IUnsafeComStream.Write(byte* pv, int cb, int* pcbWritten)

src/Microsoft.DiaSymReader/Utilities/ComStreamWrapper.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -174,12 +174,10 @@ public void SetSize(long libNewSize)
174174
void System.Runtime.InteropServices.ComTypes.IStream.SetSize(long libNewSize)
175175
=> SetSize(libNewSize);
176176

177-
public void Stat(out STATSTG pstatstg, int grfStatFlag)
177+
public void Stat(ref NativeSTATSTG pstatstg, int grfStatFlag)
178178
{
179-
pstatstg = new STATSTG()
180-
{
181-
cbSize = _stream.Length
182-
};
179+
pstatstg = default;
180+
pstatstg.cbSize = _stream.Length;
183181
}
184182

185183
void System.Runtime.InteropServices.ComTypes.IStream.Stat(out System.Runtime.InteropServices.ComTypes.STATSTG pstatstg, int grfStatFlag)

src/Microsoft.DiaSymReader/Utilities/IUnsafeComStream.cs

Lines changed: 9 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,6 @@
1010
using System.Runtime.InteropServices.Marshalling;
1111
#endif
1212

13-
#if NETSTANDARD2_0
14-
using STATSTG = System.Runtime.InteropServices.ComTypes.STATSTG;
15-
#endif
16-
1713
namespace Microsoft.DiaSymReader
1814
{
1915
/// <summary>
@@ -37,15 +33,19 @@ internal unsafe partial interface IUnsafeComStream
3733
void Revert();
3834
void LockRegion(long libOffset, long cb, int dwLockType);
3935
void UnlockRegion(long libOffset, long cb, int dwLockType);
40-
void Stat(out STATSTG pstatstg, int grfStatFlag);
36+
void Stat(ref NativeSTATSTG pstatstg, int grfStatFlag);
4137
void Clone(out IntPtr ppstm);
4238
}
4339

44-
#if NET9_0_OR_GREATER
45-
[NativeMarshalling(typeof(STATSTGMarshaller))]
46-
public struct STATSTG
40+
/// <summary>
41+
/// Native definition of `STATSTG`. Needed because the implementation of <see cref="IUnsafeComStream.Stat" /> in mscordbi
42+
/// (see https://github.com/dotnet/runtime/blob/87523393fdb14746ceb529ab308f11047819fd01/src/coreclr/inc/memorystreams.h#L115) doesn't
43+
/// zero-initialize the structure, so normal marshaling would corrupt the heap trying to process unitialized memory for (e.g. <see cref="STATSTG.pwcsName"/>).
44+
/// </summary>
45+
[StructLayout(LayoutKind.Sequential, Pack = 8)]
46+
internal struct NativeSTATSTG
4747
{
48-
public string pwcsName;
48+
public IntPtr pwcsName;
4949
public int type;
5050
public long cbSize;
5151
public FILETIME mtime;
@@ -57,65 +57,4 @@ public struct STATSTG
5757
public int grfStateBits;
5858
public int reserved;
5959
}
60-
61-
[CustomMarshaller(typeof(STATSTG), MarshalMode.ManagedToUnmanagedOut, typeof(STATSTGMarshaller))]
62-
[CustomMarshaller(typeof(STATSTG), MarshalMode.UnmanagedToManagedOut, typeof(STATSTGMarshaller))]
63-
public static unsafe class STATSTGMarshaller
64-
{
65-
public struct Native
66-
{
67-
public ushort* pwcsName;
68-
public int type;
69-
public long cbSize;
70-
public FILETIME mtime;
71-
public FILETIME ctime;
72-
public FILETIME atime;
73-
public int grfMode;
74-
public Guid clsid;
75-
public int grfLocksSupported;
76-
public int grfStateBits;
77-
public int reserved;
78-
}
79-
80-
public static STATSTG ConvertToManaged(Native n)
81-
{
82-
string name = null;
83-
if (n.pwcsName != null)
84-
{
85-
name = Utf16StringMarshaller.ConvertToManaged(n.pwcsName);
86-
Marshal.FreeCoTaskMem((IntPtr)n.pwcsName);
87-
}
88-
89-
return new()
90-
{
91-
pwcsName = name,
92-
type = n.type,
93-
cbSize = n.cbSize,
94-
mtime = n.mtime,
95-
ctime = n.ctime,
96-
atime = n.atime,
97-
grfMode = n.grfMode,
98-
clsid = n.clsid,
99-
grfLocksSupported = n.grfLocksSupported,
100-
grfStateBits = n.grfStateBits,
101-
reserved = n.reserved
102-
};
103-
}
104-
105-
public static Native ConvertToUnmanaged(STATSTG n) => new ()
106-
{
107-
pwcsName = n.pwcsName is null ? null : Utf16StringMarshaller.ConvertToUnmanaged(n.pwcsName),
108-
type = n.type,
109-
cbSize = n.cbSize,
110-
mtime = n.mtime,
111-
ctime = n.ctime,
112-
atime = n.atime,
113-
grfMode = n.grfMode,
114-
clsid = n.clsid,
115-
grfLocksSupported = n.grfLocksSupported,
116-
grfStateBits = n.grfStateBits,
117-
reserved = n.reserved
118-
};
119-
}
120-
#endif
12160
}

src/Microsoft.DiaSymReader/Utilities/UnsafeComStreamWrapper.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System;
66
using System.IO;
77
using System.Runtime.CompilerServices;
8+
using System.Runtime.InteropServices;
89
using System.Runtime.InteropServices.ComTypes;
910

1011
#if NET9_0_OR_GREATER
@@ -53,7 +54,7 @@ public UnsafeComStreamWrapper(IUnsafeComStream stream)
5354

5455
public void UnlockRegion(long libOffset, long cb, int dwLockType) => _stream.UnlockRegion(libOffset, cb, dwLockType);
5556

56-
public void Stat(out STATSTG pstatstg, int grfStatFlag) => _stream.Stat(out pstatstg, grfStatFlag);
57+
public unsafe void Stat(ref NativeSTATSTG pstatstg, int grfStatFlag) => _stream.Stat(ref pstatstg, grfStatFlag);
5758

5859
public void Clone(out IntPtr ppstm) => _stream.Clone(out ppstm);
5960

@@ -85,11 +86,12 @@ public unsafe void Write(byte[] pv, int cb, IntPtr pcbWritten)
8586

8687
void System.Runtime.InteropServices.ComTypes.IStream.Stat(out System.Runtime.InteropServices.ComTypes.STATSTG pstatstg, int grfStatFlag)
8788
{
88-
_stream.Stat(out var unsafeSTASTG, grfStatFlag);
89+
NativeSTATSTG nativeStat = default;
90+
_stream.Stat(ref nativeStat, grfStatFlag | 1 /* STATFLAG_NONAME */);
8991

9092
pstatstg = new System.Runtime.InteropServices.ComTypes.STATSTG()
9193
{
92-
cbSize = unsafeSTASTG.cbSize,
94+
cbSize = nativeStat.cbSize,
9395
};
9496
}
9597

0 commit comments

Comments
 (0)