keijiro / Smrvfx

Skinned mesh sampling with VFX Graph
The Unlicense
1.08k stars 153 forks source link

Buffer overrun in SamplePointGenerator.cs #30

Closed ivxiv closed 2 years ago

ivxiv commented 3 years ago

Hello, I discovered a buffer overrun scenario in SamplePointGenerator.Execute(). The issue is that the code can attempt to write past the end of the Output[] array by 1 in certain cases, as a result of accumulated numerical error.

Our repro case is use of a SkinnedMeshBaker component with 1 source and a Point Count of 65536

Accumulated numerical error causes SamplePointGenerator.Execute() to attempt to write to Output[65536] - which is out of bounds by 1.

When this occurs, the problem will initially manifest itself as a crash in the UnityPlayer (if running a standalone build) with a call stack that looks like this (on WIndows):

========== OUTPUTTING STACK TRACE ==================
0x00007FFA28DB348C (UnityPlayer) UnityMain
0x00007FFA28DB3A21 (UnityPlayer) UnityMain
0x00007FFA284384D3 (UnityPlayer) UnityMain
0x00007FFA28435FE4 (UnityPlayer) UnityMain
0x00007FFA284325FA (UnityPlayer) UnityMain
0x00007FFA2843580C (UnityPlayer) UnityMain
0x00007FFA28828085 (UnityPlayer) UnityMain
0x0000028A4BAFF386 (Mono JIT Code) (wrapper managed-to-native) Unity.Collections.LowLevel.Unsafe.UnsafeUtility:Malloc (long,int,Unity.Collections.Allocator)
0x0000028AA1418803 (Mono JIT Code) Unity.Collections.NativeArray`1<Smrvfx.SamplePoint>:Allocate (int,Unity.Collections.Allocator,Unity.Collections.NativeArray`1<Smrvfx.SamplePoint>&)
0x0000028AA1418713 (Mono JIT Code) Unity.Collections.NativeArray`1<Smrvfx.SamplePoint>:.ctor (int,Unity.Collections.Allocator,Unity.Collections.NativeArrayOptions)
0x0000028AA141867B (Mono JIT Code) Smrvfx.MemoryUtil:Array<Smrvfx.SamplePoint> (int)
0x0000028AA14183E3 (Mono JIT Code) Smrvfx.SamplePointGenerator:Generate (Smrvfx.CombinedMesh,int)
0x0000028AA1414AF3 (Mono JIT Code) Smrvfx.SkinnedMeshBaker:InitializeInternals ()
0x0000028AA14146AB (Mono JIT Code) Smrvfx.SkinnedMeshBaker:LateUpdate ()
0x0000028A3BAB4590 (Mono JIT Code) (wrapper runtime-invoke) object:runtime_invoke_void__this__ (object,intptr,intptr,intptr)
0x00007FFA2617E630 (mono-2.0-bdwgc) mono_get_runtime_build_info
0x00007FFA26102AC2 (mono-2.0-bdwgc) mono_perfcounters_init
0x00007FFA2610BB1F (mono-2.0-bdwgc) mono_runtime_invoke
0x00007FFA2879527D (UnityPlayer) UnityMain
0x00007FFA287924E2 (UnityPlayer) UnityMain
0x00007FFA28779FC3 (UnityPlayer) UnityMain
0x00007FFA2877A09F (UnityPlayer) UnityMain
0x00007FFA284F2100 (UnityPlayer) UnityMain
0x00007FFA286248CA (UnityPlayer) UnityMain
0x00007FFA28624970 (UnityPlayer) UnityMain
0x00007FFA286286C8 (UnityPlayer) UnityMain
  ERROR: SymGetSymFromAddr64, GetLastError: 'Attempt to access invalid address.' (Address: 00007FFA283ED29A)
0x00007FFA283ED29A (UnityPlayer) (function-name not available)
  ERROR: SymGetSymFromAddr64, GetLastError: 'Attempt to access invalid address.' (Address: 00007FFA283EB90D)
0x00007FFA283EB90D (UnityPlayer) (function-name not available)
  ERROR: SymGetSymFromAddr64, GetLastError: 'Attempt to access invalid address.' (Address: 00007FFA283F09E2)
0x00007FFA283F09E2 (UnityPlayer) (function-name not available)
0x00007FFA283F18FB (UnityPlayer) UnityMain
  ERROR: SymGetSymFromAddr64, GetLastError: 'Attempt to access invalid address.' (Address: 00007FF637AF11F2)
0x00007FF637AF11F2 (WaveCast) (function-name not available)
0x00007FFA7E677034 (KERNEL32) BaseThreadInitThunk
0x00007FFA80042651 (ntdll) RtlUserThreadStart
========== END OF STACKTRACE ===========

If running in the Editor, the problem manifests as an IndexOutOfRangeException that looks like this:

System.IndexOutOfRangeException: Index {0} is out of range of '{1}' Length.
Thrown from job: Smrvfx.SamplePointGenerator.GenerationJob
This Exception was thrown from a job compiled with Burst, which has limited exception support. Turn off burst (Jobs -> Burst -> Enable Compilation) to inspect full exceptions & stacktraces

With Burst Compilation disabled (as instructed by the original error message), running again in the Editor we get:

IndexOutOfRangeException: Index 65536 is out of range of '65536' Length.
Unity.Collections.NativeArray`1[T].FailOutOfRangeError (System.Int32 index) (at <ef667cdc82bf4a4e911d1ddea9ff581d>:0)
Unity.Collections.NativeArray`1[T].CheckElementWriteAccess (System.Int32 index) (at <ef667cdc82bf4a4e911d1ddea9ff581d>:0)
Unity.Collections.NativeArray`1[T].set_Item (System.Int32 index, T value) (at <ef667cdc82bf4a4e911d1ddea9ff581d>:0)
Smrvfx.SamplePointGenerator+GenerationJob.Execute () (at Library/PackageCache/jp.keijiro.smrvfx@1.1.6/Runtime/Internal/SamplePointGenerator.cs:111)
Unity.Jobs.IJobExtensions+JobStruct`1[T].Execute (T& data, System.IntPtr additionalPtr, System.IntPtr bufferRangePatchData, Unity.Jobs.LowLevel.Unsafe.JobRanges& ranges, System.Int32 jobIndex) (at <ef667cdc82bf4a4e911d1ddea9ff581d>:0)
Unity.Jobs.LowLevel.Unsafe.JobsUtility:Schedule_Injected(JobScheduleParameters&, JobHandle&)
Unity.Jobs.LowLevel.Unsafe.JobsUtility:Schedule(JobScheduleParameters&)
Unity.Jobs.IJobExtensions:Run(GenerationJob)
Smrvfx.SamplePointGenerator:Generate(CombinedMesh, Int32) (at Library/PackageCache/jp.keijiro.smrvfx@1.1.6/Runtime/Internal/SamplePointGenerator.cs:21)
Smrvfx.SkinnedMeshBaker:InitializeInternals() (at Library/PackageCache/jp.keijiro.smrvfx@1.1.6/Runtime/SkinnedMeshBaker.cs:68)
Smrvfx.SkinnedMeshBaker:LateUpdate() (at Library/PackageCache/jp.keijiro.smrvfx@1.1.6/Runtime/SkinnedMeshBaker.cs:38)

To work around the issue, I made the following local code change to SamplePointGenerator.Execute():

for (; acc > areaPerSample; acc -= areaPerSample)
{
    var r = rnd.NextFloat2();
    if (r.x + r.y > 1) r = 1 - r;

    // IVXIV: prevent buffer overrun
    if (offs == Output.Length)
    {
        // IVXIV: NOTE: (0,0): Burst error BC1345: Invalid argument type System.String with index 0. Only value types are supported.
        var outputLength = Output.Length;
        var indicesLength = Indices.Length;
        var verticesLength = Vertices.Length;
        // IVXIV: NOTE: Burst: loading a managed string literal is not supported.
        // Hence the use of $@ syntax here to use an interpolated verbatim string
        Debug.LogWarning($@"SamplePointGenerator.Execute() buffer overrun narrowly averted - INVESTIGATE!!
            TotalArea = {TotalArea}
            areaPerSample = {areaPerSample}
            Output.Length = {outputLength}
            Indices.Length = {indicesLength}
            Vertices.Length = {verticesLength}");
        return;
    }
    Output[offs++] =
      new SamplePoint(i1, 1 - r.x - r.y, i2, r.x, i3, r.y);
}
keijiro commented 3 years ago

FYI: Now skinned mesh sampling is officially supported, so I'd recommend using it instead of Smrvfx for better performance, stability and technical support.

https://docs.unity3d.com/Packages/com.unity.visualeffectgraph@12.0/manual/Operator-SampleMesh.html

keijiro commented 2 years ago

I'm closing this issue as I changed the main aim of this repository to provide an example of the built-in mesh sampling feature. This issue doesn't reproduce with the built-in one.