Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Register memory ranges that might cause access violation and throw manged exception #24767

Open
ayende opened this issue Jan 22, 2018 · 3 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Milestone

Comments

@ayende
Copy link
Contributor

ayende commented Jan 22, 2018

Rational

When using memory mapped files, there is the need to handle the standard set of I/O issues, however, the CoreCLR currently expose not such way to do so. In the Full .Net Framework, there is HandleProcessCorruptedStateExceptions, which provide some answer to that.

However, that require to modify all call sites and is a bit of throwing the baby with the bath waters.

The Core CLR should be able to handle such scenarios cleanly and easily.

Example problem usage:

var mmf = MemoryMappedFile.CreateFromFile("file-with-bad-sectory.dat");
var accessor = mmf.CreateViewAccessor();
accessor.ReadByte(positionOfBadSectory);

This code will error with an SEHException on Windows, killing the entire process. As it currently stands, there is no good way to protect against this at all.

Proposed solution

Provide a way to register memory ranges with the execution engine that will not be subject to the same behavior. When an access violation occurs, the CLR will first check if the memory range of the error occurred in one of these ranges, and if so, will translate the error into a normal manage exception, subject to all the usual rules of such exceptions.
In particular, this will not be a process killer.

This is similar to how a memory access on the first 64KB of the address space is translated to a managed NullReferenceException.

Proposed API

namespace System.Runtime
{
        public static class MemoryAccessViolationBehavior
        {
                public static IDisposable ThrowOnInvalidMemoryAccessInRange(IntPtr start, long length, object errorState);
       }
}

Sample usage:

using(MemoryAccessViolationBehavior.ThrowOnInvalidMemoryAccessInRange(start, len))
{
    // in here we an access violation to in [start ... start + len] will throw
}

The exception thrown

The exception thrown in this case will be a new one:

namespace System.Runtime
{
         public class InvalidMemoryAccessException : Exception
         {
                   // the marker object that was used on the ThrowOnInvalidMemoryAccessInRange call
                   public object ErrorState { get;set; }
                   // the memory range that caused the error
                   public IntPtr MemoryLocation  { get; set; }
                   // the native error code that caused this issue, SEH code or the si_errno
                   public int NativeErrorCode { get; set; } 

                  // standard exception impl
         }
}

Semantics

  • This must be thread safe and thread agnostic. Multiple threads may use a range concurrently and expect to be able to enter and exit it at the same time.
  • Another use case if long running memory range that is mapped once and then used for a long duration. This is common for larger sections (a mmap on a 100GB file, for example, that may live for as long as the application itself)
  • It is fine to have overlapping ranges, if the error happens in any range that matches a registered one, the exception will be thrown instead of killing the process. The ErrorState object used can be any of the registered objects for the range.
  • The reasoning for the ErrorState is that it would make it much easier for users to be able to track down the error if they could tag the range with some meaningful value, such as the file name, for example.

Implementation details

  • Internally each range should have ref count, and removed when there are no more interested parties to it.
  • A question occurs about what to do when the range neither held nor disposed, such as in this case:
    MemoryAccessViolationBehavior.ThrowOnInvalidMemoryAccessInRange(start, len).
    In this case, there are two options. First, we can have the return object implement a finalizer, which will clear the range protection. Alternatively, we can threat this as intentional sign from the user that they never want to clear the protection of this range. I'm inclined to the first option, because assuming that the user intentionally ignored the disposable is probably a bad idea.
  • This proposal is intentionally not tied to the MemoryMappedFile API, because it should be useful for other types of memory. Either calling directly to the native API (mmap / CreateViewOfFile) or using other types of shared memory (shm_open, etc).

Potential issues with this proposal

A user may register a range, then free it and then it is used for some other purpose, which can raise memory access errors, these errors would then be erroneously reported as exception instead of crashing the process.

Given that this is an explicit (and very low level) API, I don't think that this can happen by accident, and the error is explicit enough that it should be obvious what happened, especially given the optional state.

Other considerations

This behavior should ideally apply for managed code and native code alike. Calling a method such as memcpy on a range of mmap memory can trigger the error as easily as if it was accessed directly.
That said, what about the behavior when this occurs during GC, for example? That would be a clear error, and I'm not sure what the behavior of that should be.

Note that this is in contrast to how NullReferencException works right now, consider:

 [DllImport("msvcrt.dll", EntryPoint = "memcmp")]
 public static extern int Compare(byte* b1, byte* b2, long count);

 static void Main(string[] args)
 {
     byte* f = (byte*)0;

     try
     {
         *f = 1;
     }
     catch (NullReferenceException)
     {
         Console.WriteLine("NRE");
     }

     Compare(f, f, 1);
 }

The managed code accessing the invalid null pointer will get a NullReferenceExecption while native code will raise an AccessViolationException in the debugger and kill the process otherwise.

I don't know if this behavior can be applied to native code as well in the same manner (gut feeling is probably not), but even just in managed code, that would be sufficient.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@ericstj ericstj modified the milestones: 5.0.0, 6.0.0 Jul 29, 2020
@ericstj
Copy link
Member

ericstj commented Jul 29, 2020

cc @carlossanlop (as this is IO scenario)

I don't think there is time to consider this for 5.0 however it does seem like an interesting scenario. @jkotas what are your thoughts on this?

@jkotas
Copy link
Member

jkotas commented Jul 30, 2020

I do like the idea, but the issues mentioned in "Other considerations" paragraph may make it a non-starter.

@ayende
Copy link
Contributor Author

ayende commented Jul 30, 2020

If this work just for managed code, that would still be a great feature, and NRE handling already does the right thing here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Runtime
Projects
None yet
Development

No branches or pull requests

5 participants