Posted By: W3bbo | Jun 27th @ 6:22 PM
page 1 of 1
Comments: 3 | Views: 542
W3bbo
W3bbo
The Master of Baiters

Here's some code I wrote that sets the right checksum field value in a PE file's header. It's part of a larger program that modifies system files:

(Sfh is a type alias for SafeFileHandle)

// Get handle to a file with read/write access
// CreateFileMapping
// MapViewOfFile
// Check DOS/NT Headers (optional, not needed in this case since if it's gotten this far it's going to be a Win32 PE)
// CheckSumMappedFile
// Close the handles
            
            Sfh    sfh   = null;
            IntPtr hMap  = IntPtr.Zero;
            IntPtr pData = IntPtr.Zero;
            
            try {
                
                sfh = NativeMethods.CreateFile( fileName, FileAccess.ReadWrite, FileShare.None, IntPtr.Zero, FileMode.Open, 0, new Sfh(IntPtr.Zero, true) );
                
                if( sfh.IsInvalid ) throw new AnolisException("Invalid SafeFileHandle: " + NativeMethods.GetLastErrorString() );
                
                hMap = NativeMethods.CreateFileMapping( sfh, IntPtr.Zero, FileMapProtection.PageReadWrite, 0, 0, null );
                
                if( hMap == IntPtr.Zero ) throw new AnolisException("Invalid File Mapping: " + NativeMethods.GetLastErrorString() );
                
                pData = NativeMethods.MapViewOfFile( hMap, FileMapAccess.FileMapWrite, 0, 0, IntPtr.Zero );
                
                if( pData == IntPtr.Zero ) throw new AnolisException("Invalid Map View of File: " + NativeMethods.GetLastErrorString() );
                
                // checking the PE headers for validity isn't required in this scenario
                
                UInt32 oldChecksum = 0;
                UInt32 newChecksum = 0;
                
                UInt32 size = NativeMethods.GetFileSize( sfh, IntPtr.Zero );
                
                // Get the NTHeader
                IntPtr pNTHeader = NativeMethods.CheckSumMappedFile( pData, size, ref oldChecksum, ref newChecksum );
                
                if( pNTHeader == IntPtr.Zero ) throw new AnolisException("CheckSumMappedFile Failed: " + NativeMethods.GetLastErrorString() );
                
                NTHeader ntHeader = (NTHeader)Marshal.PtrToStructure( pNTHeader, typeof(NTHeader) );
                
                // Set the CheckSum, but as this is a new allocation by Marshal it needs to be copied back to the mapped memory
                ntHeader.OptionalHeader.CheckSum = newChecksum;
                
                // this overwrites the old NTHeader @ pNTHeader
                Marshal.StructureToPtr( ntHeader, pNTHeader, false );
                
            } finally {
                
                String errors = String.Empty;
                
                if( pData != IntPtr.Zero )
                    if( !NativeMethods.UnmapViewOfFile( pData ) ) {
                        errors += "UnmapViewOfFile failed: " + NativeMethods.GetLastErrorString() + "\r\n";
                    }
                    
                if( hMap  != IntPtr.Zero )
                    if( !NativeMethods.CloseHandle( hMap ) ) {
                        errors += "CloseHandle(hMap) failed: " + NativeMethods.GetLastErrorString() + "\r\n";
                    }
                    
                if( sfh   != null && !sfh.IsInvalid )
                    if( !NativeMethods.CloseHandle( sfh ) ) {
                        errors += "CloseHandle(sfh) failed: " + NativeMethods.GetLastErrorString() + "\r\n";
                    }
                
                
            }


The problem is, this code doesn't seem to close the handles right. My program's exception log is on Pastebin. The "The handle is invalid" exceptions occur at random intervals.

At the end of everything, my process' handle count is at about ~200, so it's not like it's obviously leaking them. But is there anything in here that isn't done right that could fail?

Sven Groot
Sven Groot
My name has 9 letters. Coincidence? I think not...

You're not supposed to call CloseHandle directly on a SafeHandle. You should call SafeHandle.Close or SafeHandle.Dispose (or just wrap the safe handle in a using block). The way you're doing it, the SafeFileHandle will close the handle again when its finalizer runs, decrementing the handle's reference count once too many, and apparently some FileStream was using that same handle which is now invalidated.

Sven Groot
Sven Groot
My name has 9 letters. Coincidence? I think not...

Some time ago, I wrote a class that encapsulates a file mapping view, you can use it if you like:

static class NativeMethods
{
    [Flags]
    public enum FileAccess : uint
    {
        DeviceAttributes = 0,
        GenericRead = 0x80000000,
        GenericWrite = 0x40000000
    }

    public enum CreationDisposition
    {
        CreateNew = 1,
        CreateAlways = 2,
        OpenExisting = 3,
        OpenAlways = 4,
        TruncateExisting = 5
    }

    [Flags]
    public enum FileShareMode
    {
        Read = 1,
        Write = 2,
        Delete = 4

    }

    [Flags]
    public enum FileFlagsAndAttributes : uint
    {
        ReadOnly = 0x1,
        Hidden = 0x2,
        System = 0x4,
        Archive = 0x20,
        Normal = 0x80,
        FlagWriteThrough = 0x80000000,
        FlagRandomAccess = 0x10000000
    }

    public enum PageProtect
    {
        ReadOnly = 0x2,
        ReadWrite = 0x4,
    }

    [Flags]
    public enum MapAccess
    {
        Write = 0x2,
        Read = 0x4,
        AllAccess = Write | Read
    }

    [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
    public static extern SafeFileHandle CreateFile(
       string lpFileName,
       FileAccess dwDesiredAccess,
       FileShareMode dwShareMode,
       IntPtr lpSecurityAttributes,
       CreationDisposition dwCreationDisposition,
       FileFlagsAndAttributes dwFlagsAndAttributes,
       IntPtr hTemplateFile);

    [DllImport("kernel32.dll", SetLastError = true, CharSet = CharSet.Unicode)]
    public static extern SafeFileMappingHandle CreateFileMapping(SafeFileHandle hFile, IntPtr lpFileMappingAttributes, PageProtect flProtect,
        uint dwMaximumSizeHigh, uint dwMaximumSizeLow, string lpName);

    [DllImport("kernel32.dll", SetLastError = true)]
    public static extern IntPtr MapViewOfFile(SafeFileMappingHandle hFileMappingObject, MapAccess dwDesiredAccess, uint dwFileOffsetHigh,
        uint dwFileOffsetLow, IntPtr dwNumberOfBytesToMap);

    [DllImport("kernel32.dll")]
    [return: MarshalAs(UnmanagedType.Bool)]
    public static extern bool UnmapViewOfFile(IntPtr lpBaseAddress);
    
    [DllImport("kernel32.dll"), ReliabilityContract(Consistency.WillNotCorruptState, Cer.Success)]
    [return: MarshalAs(UnmanagedType.Bool)]
    public static extern bool CloseHandle(IntPtr hObject);
}

[SecurityPermission(SecurityAction.LinkDemand, UnmanagedCode = true)]
sealed class SafeFileMappingHandle : SafeHandleZeroOrMinusOneIsInvalid
{
    public SafeFileMappingHandle()
        : base(true)
    {
    }

    public SafeFileMappingHandle(IntPtr handle, bool ownsHandle)
        : base(ownsHandle)
    {
        SetHandle(handle);
    }

    protected override bool ReleaseHandle()
    {
        return NativeMethods.CloseHandle(handle);
    }
}

/// <summary>
/// Represents a memory-mapped file.
/// </summary>
[SecurityPermission(SecurityAction.LinkDemand, UnmanagedCode=true)]
public sealed class FileMapping : IDisposable
{
    private readonly SafeFileHandle _file;
    private readonly SafeFileMappingHandle _mapping;
    private readonly IntPtr _view;
    private readonly long _size;
    private bool _disposed;

    /// <summary>
    /// Initializes a new instance of the <see cref="FileMapping"/> class.
    /// </summary>
    /// <param name="fileName">The file that should be memory-mapped</param>
    public FileMapping(string fileName)
    {
        if( fileName == null )
            throw new ArgumentNullException("fileName");

        // Get the size.
        // This throws a proper FileNotFoundException if the file doesn't exist so we don't need to check that anymore.
        _size = new FileInfo(fileName).Length;

        try
        {
            _file = NativeMethods.CreateFile(fileName, NativeMethods.FileAccess.GenericRead, NativeMethods.FileShareMode.Read, IntPtr.Zero, NativeMethods.CreationDisposition.OpenExisting, NativeMethods.FileFlagsAndAttributes.Normal | NativeMethods.FileFlagsAndAttributes.FlagRandomAccess, IntPtr.Zero);
            if( _file.IsInvalid )
                throw new Win32Exception();

            _mapping = NativeMethods.CreateFileMapping(_file, IntPtr.Zero, NativeMethods.PageProtect.ReadOnly, 0, 0, null);
            if( _mapping.IsInvalid )
                throw new Win32Exception();

            _view = NativeMethods.MapViewOfFile(_mapping, NativeMethods.MapAccess.Read, 0, 0, IntPtr.Zero);
            if( _view == IntPtr.Zero )
                throw new Win32Exception();
        }
        catch
        {
            if( _view != IntPtr.Zero )
                NativeMethods.UnmapViewOfFile(_view);
            if( _mapping != null )
                _mapping.Dispose();
            if( _file != null )
                _file.Dispose();
            throw;
        }
    }

    /// <summary>
    /// Cleans up resources on finalization.
    /// </summary>
    ~FileMapping()
    {
        Dispose(false);
    }

    /// <summary>
    /// Gets a memory-mapped view to the file's contents.
    /// </summary>
    /// <value>
    /// A pointer to the first byte of the memory-mapped file.
    /// </value>
    public IntPtr View
    {
        get
        {
            CheckDisposed();
            return _view;
        }
    }

    /// <summary>
    /// Gets the size of the memory-mapped file.
    /// </summary>
    public long Size
    {
        get
        {
            CheckDisposed();
            return _size;
        }
    }

    /// <summary>
    /// Cleans up resources used by the <see cref="FileMapping"/> class.
    /// </summary>
    /// <param name="disposing"><see langword="true"/> to clean up both managed and unmanaged resources; <see langword="false" /> to clean up only unmanaged resources.</param>
    private void Dispose(bool disposing)
    {
        if( !_disposed )
        {
            if( _view != IntPtr.Zero )
                NativeMethods.UnmapViewOfFile(_view);

            if( disposing )
            {
                if( _mapping != null )
                    _mapping.Dispose();
                if( _file != null )
                    _file.Dispose();
            }

            _disposed = true;
        }
    }

    private void CheckDisposed()
    {
        if( _disposed )
            throw new ObjectDisposedException(typeof(FileMapping).FullName);
    }

    #region IDisposable Members

    /// <summary>
    /// Cleans up resources used by the <see cref="FileMapping"/> class.
    /// </summary>
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    #endregion
}

page 1 of 1
Comments: 3 | Views: 542
Microsoft Communities