[System.Drawing.Common] Work around libgdiplus use after free (#43074)

* [System.Drawing.Common] Work around libgdiplus use after free

On Windows, both of the following are legal

    Metafile mf = ... ; // get a metafile instance
    Graphics g = Graphics.FromImage(mf);
    g.Dispose(); mf.Dispose();

and

    Metafile mf = ... ; // get a metafile instance
    Graphics g = Graphics.FromImage(mf);
    mf.Dispose(); g.Dispose();

On Unix, libgdiplus has a use after free bug for the second form - the metafile
native image is disposed, but the graphics instance still has a pointer to the
memory that it will use during cleanup.  If the memory is reused, the graphics
instance will see garbage values and crash.

The workaround is to add a MetadataHolder class and to transfer responsibility
for disposing of the native image instance to it if the Metafile is disposed
before the Graphics.

Note that the following is not allowed (throws OutOfMemoryException on GDI+ on
Windows), so there's only ever one instance of Graphics associated with a
Metafile at a time.

    Graphics g = Graphics.FromImage(mf);
    Graphics g2 = Graphics.FromImage(mf); // throws

Addresses #37838

* Formatting fixes

Co-authored-by: Santiago Fernandez Madero <[email protected]>

* Address review feedback

* Inilne unhelpful helper

* formatting

Co-authored-by: Santiago Fernandez Madero <[email protected]>

src/libraries/System.Drawing.Common/src/System/Drawing/Graphics.Unix.cs

@@ -50,9 +50,18 @@ public sealed partial class Graphics : MarshalByRefObject, IDisposable, IDeviceC
         private bool disposed;
         private static float defDpiX;
         private static float defDpiY;
+        private Metafile.MetafileHolder? _metafileHolder;
 
         internal Graphics(IntPtr nativeGraphics) => NativeGraphics = nativeGraphics;
 
+        internal Graphics(IntPtr nativeGraphics, Image image) : this(nativeGraphics)
+        {
+            if (image is Metafile mf)
+            {
+                _metafileHolder = mf.AddMetafileHolder();
+            }
+        }
+
         ~Graphics()
         {
             Dispose();
@@ -226,6 +235,14 @@ public void Dispose()
                 status = Gdip.GdipDeleteGraphics(new HandleRef(this, NativeGraphics));
                 NativeGraphics = IntPtr.Zero;
                 Gdip.CheckStatus(status);
+
+                if (_metafileHolder != null)
+                {
+                    var mh = _metafileHolder;
+                    _metafileHolder = null;
+                    mh.GraphicsDisposed();
+                }
+
                 disposed = true;
             }
 
@@ -488,7 +505,7 @@ public static Graphics FromImage(Image image)
 
             int status = Gdip.GdipGetImageGraphicsContext(image.nativeImage, out graphics);
             Gdip.CheckStatus(status);
-            Graphics result = new Graphics(graphics);
+            Graphics result = new Graphics(graphics, image);
 
             Rectangle rect = new Rectangle(0, 0, image.Width, image.Height);
             Gdip.GdipSetVisibleClip_linux(result.NativeGraphics, ref rect);

src/libraries/System.Drawing.Common/src/System/Drawing/Imaging/Metafile.Unix.cs

@@ -35,6 +35,7 @@
 using System.IO;
 using System.Reflection;
 using System.ComponentModel;
+using System.Diagnostics;
 using System.Runtime.InteropServices;
 using Gdip = System.Drawing.SafeNativeMethods.Gdip;
 using System.Runtime.Serialization;
@@ -43,6 +44,93 @@ namespace System.Drawing.Imaging
 {
     public sealed partial class Metafile : Image
     {
+        // Non-null if a graphics instance was created using
+        // Graphics.FromImage(this) The metadata holder is responsible for
+        // freeing the nativeImage if the Metadata instance is disposed before
+        // the Graphics instance.
+        private MetafileHolder? _metafileHolder;
+
+        // A class responsible for disposing of the native Metafile instance
+        // if it needs to outlive the managed Metafile instance.
+        //
+        // The following are both legal with win32 GDI+:
+        //     Metafile mf = ...; // get a metafile instance
+        //     Graphics g = Graphics.FromImage(mf); // get a graphics instance
+        //     g.Dispose();  mf.Dispose(); // dispose of the graphics instance first
+        //   OR
+        //     mf.Dispose(); g.Dispose();  // dispose of the metafile instance first
+        //
+        // ligbgdiplus has a bug where disposing of the metafile instance first will
+        // trigger a use of freed memory when the graphics instance is disposed, which
+        // could lead to crashes when the native memory is reused.
+        //
+        // The metafile holder is designed to take ownership of the native metafile image
+        // when the managed Metafile instance is disposed while a Graphics instance is still
+        // not disposed (ie the second code pattern above) and to keep the native image alive until the graphics
+        // instance is disposed.
+        //
+        // Note that the following throws, so we only ever need to keep track of one Graphics
+        // instance at a time:
+        //      Metafile mf = ...;  // get a metafile instance
+        //      Graphics g = Graphics.FromImage(mf);
+        //      Graphics g2 = Graphics.FromImage(mf);  // throws OutOfMemoryException on GDI+ on Win32
+        internal sealed class MetafileHolder : IDisposable
+        {
+            private bool _disposed;
+            private IntPtr _nativeImage;
+
+
+            internal bool Disposed { get => _disposed; }
+            internal MetafileHolder()
+            {
+                _disposed = false;
+                _nativeImage = IntPtr.Zero;
+            }
+
+            ~MetafileHolder() => Dispose(false);
+
+            public void Dispose()
+            {
+                Dispose(true);
+                GC.SuppressFinalize(this);
+            }
+
+            internal void Dispose(bool disposing)
+            {
+                if (!_disposed)
+                {
+                    IntPtr nativeImage = _nativeImage;
+                    _nativeImage = IntPtr.Zero;
+                    _disposed = true;
+                    if (nativeImage != IntPtr.Zero)
+                    {
+                        int status = Gdip.GdipDisposeImage(nativeImage);
+                        Gdip.CheckStatus(status);
+                    }
+                }
+            }
+
+            internal void MetafileDisposed(IntPtr nativeImage)
+            {
+                _nativeImage = nativeImage;
+            }
+
+            internal void GraphicsDisposed()
+            {
+                Dispose();
+            }
+        }
+
+        internal MetafileHolder? AddMetafileHolder()
+        {
+            // If _metafileHolder is not null and hasn't been disposed yet, there's already a graphics instance associated with
+            // this metafile, the native code will return an error status.
+            if (_metafileHolder != null && !_metafileHolder.Disposed)
+                return null;
+            _metafileHolder = new MetafileHolder();
+            return _metafileHolder;
+        }
+
         // Usually called when cloning images that need to have
         // not only the handle saved, but also the underlying stream
         // (when using MS GDI+ and IStream we must ensure the stream stays alive for all the life of the Image)
@@ -143,6 +231,21 @@ public Metafile(string fileName, IntPtr referenceHdc, Rectangle frameRect, Metaf
             Gdip.CheckStatus(status);
         }
 
+        protected override void Dispose(bool disposing)
+        {
+            if (_metafileHolder != null && !_metafileHolder.Disposed)
+            {
+                // There's a graphics instance created from this Metafile,
+                // transfer responsibility for disposing the nativeImage to the
+                // MetafileHolder
+                _metafileHolder.MetafileDisposed(nativeImage);
+                _metafileHolder = null;
+                nativeImage = IntPtr.Zero;
+            }
+
+            base.Dispose(disposing);
+        }
+
         // methods
 
         public IntPtr GetHenhmetafile()