Segmentation fault when stride is greater than image width

[elahav]

Hello,

I built ThorVG on QNX (just needed a small change to meson.build). Everything works well, until I set a canvas target where the stride is greater than the width of the window. I have traced the problem to SwSurface* SwRenderer::request(), which allocates a buffer for the compositor surface using width instead of stride.

The following change was sufficient to avoid the segmentation fault, but I have no idea if it is correct:

diff --git a/src/renderer/sw_engine/tvgSwRenderer.cpp b/src/renderer/sw_engine/tvgSwRenderer.cpp
index f8fb47e5..228d419b 100644
--- a/src/renderer/sw_engine/tvgSwRenderer.cpp
+++ b/src/renderer/sw_engine/tvgSwRenderer.cpp
@@ -550,7 +550,7 @@ const RenderSurface* SwRenderer::mainSurface()
 SwSurface* SwRenderer::request(int channelSize, bool square)
 {
     SwSurface* cmp = nullptr;
-    uint32_t w, h;
+    uint32_t w, h, stride;
 
     if (square) {
         //Same Dimensional Size is demanded for the Post Processing Fast Flipping
@@ -574,12 +574,13 @@ SwSurface* SwRenderer::request(int channelSize, bool square)
     //New Composition
     if (!cmp) {
         //Inherits attributes from main surface
+        stride = surface->stride;
         cmp = new SwSurface(surface);
         cmp->compositor = new SwCompositor;
-        cmp->compositor->image.data = (pixel_t*)malloc(channelSize * w * h);
+        cmp->compositor->image.data = (pixel_t*)malloc(channelSize * stride* h);
         cmp->w = cmp->compositor->image.w = w;
         cmp->h = cmp->compositor->image.h = h;
-        cmp->compositor->image.stride = w;
+        cmp->compositor->image.stride = stride;
         cmp->compositor->image.direct = true;
         cmp->compositor->valid = true;
         cmp->channelSize = cmp->compositor->image.channelSize = channelSize;

Thoughts?

--Elad

[hermet]

@elahav Hi, thanks for your report. Could you share the call stack you have? I believe the root cause might be an out-of-range access there.

[elahav]

Here you go:

Program received signal SIGSEGV, Segmentation fault.
0x000000010008449d in _mm256_storeu_si256(long long __vector(4)*, long long __vector(4)) (__A=..., __P=0x9751fe4) at /system/lib/gcc/x86_64-pc-nto-qnx
8.0.0/12.2.0/include/avxintrin.h:935
935       *__P = __A;
(gdb) p __P                                                                
$27 = (__m256i_u *) 0x9751fe4
(gdb) bt               
#0  0x000000010008449d in _mm256_storeu_si256(long long __vector(4)*, long long __vector(4)) (__A=..., __P=0x9751fe4)
    at /system/lib/gcc/x86_64-pc-nto-qnx8.0.0/12.2.0/include/avxintrin.h:935
#1  avxRasterPixel32 (dst=0x9751fe4, val=0, offset=1166773, len=643) at ../src/renderer/sw_engine/tvgSwRasterAvx.h:93
#2  0x000000010008a0b7 in rasterPixel32 (dst=0x92de010, val=0, offset=1166773, len=643) at ../src/renderer/sw_engine/tvgSwRaster.cpp:1609
#3  0x000000010008a25a in rasterClear (surface=0x91afa00, x=437, y=232, w=643, h=848, val=0) at ../src/renderer/sw_engine/tvgSwRaster.cpp:1652
#4  0x0000000100090308 in tvg::SwRenderer::target (this=0x90667a0, region=..., cs=tvg::ColorSpace::ARGB8888, flags=tvg::Masking)
    at ../src/renderer/sw_engine/tvgSwRenderer.cpp:630
#5  0x000000010006b901 in tvg::Scene::Impl::render (this=0x9101550, renderer=0x90667a0) at ../src/renderer/tvgScene.h:133
#6  0x00000001000685e7 in tvg::Paint::Impl::render (this=0x9101550, renderer=0x90667a0) at ../src/renderer/tvgPaint.cpp:194
#7  0x000000010006ba8a in tvg::Scene::Impl::render (this=0x90db8a0, renderer=0x90667a0) at ../src/renderer/tvgScene.h:138
#8  0x00000001000685e7 in tvg::Paint::Impl::render (this=0x90db8a0, renderer=0x90667a0) at ../src/renderer/tvgPaint.cpp:194
#9  0x000000010006b0ca in tvg::Picture::Impl::render (this=0x1004045e0 <__main_arena+5664>, renderer=0x90667a0) at ../src/renderer/tvgPicture.h:263
#10 0x0000000100068607 in tvg::Paint::Impl::render (this=0x1004045e0 <__main_arena+5664>, renderer=0x90667a0) at ../src/renderer/tvgPaint.cpp:194
#11 0x000000010006ba8a in tvg::Scene::Impl::render (this=0x90666d0, renderer=0x90667a0) at ../src/renderer/tvgScene.h:138
#12 0x00000001000685e7 in tvg::Paint::Impl::render (this=0x90666d0, renderer=0x90667a0) at ../src/renderer/tvgPaint.cpp:194
#13 0x000000010006522e in tvg::Canvas::Impl::draw (this=0x91094d0, clear=true) at ../src/renderer/tvgCanvas.h:102
#14 0x0000000100064c29 in tvg::Canvas::draw (this=0x9066690, clear=true) at ../src/renderer/tvgCanvas.cpp:51
#15 0x0000000008049c39 in event_loop () at /home/elahav/src/projects/lottie/lottie.cc:287
#16 0x0000000008049df6 in main (argc=3, argv=0x81818) at /home/elahav/src/projects/lottie/lottie.cc:350

And yes, it is accessing beyond the buffer allocated to the compositor. The stride is 1088, while the width of the image is 1080.

[hermet]

@elahav Hello, I tried to reproduce the issue, but I couldn't find any memory violation with the canvas stride set to 1088 and a canvas width of 1080 with a certain . We might need more information to clarify this. If you could provide a sample to reproduce it, it would be very helpful. Thanks.

Please have a look, this is what I configured.
//meson setup builddir -Dexamples=true -Dloaders=all -Dsimd=true

diff --git a/examples/Animation.cpp b/examples/Animation.cpp
index e4f218bc..ba0a79dd 100644
--- a/examples/Animation.cpp
+++ b/examples/Animation.cpp
@@ -90,5 +90,5 @@ struct UserExample : tvgexam::Example

 int main(int argc, char **argv)
 {
-    return tvgexam::main(new UserExample, argc, argv, false, 1024, 1024);
+    return tvgexam::main(new UserExample, argc, argv, false, 1088, 1024);
 }
\ No newline at end of file
diff --git a/examples/Example.h b/examples/Example.h
index 33dc9a76..4d963025 100644
--- a/examples/Example.h
+++ b/examples/Example.h
@@ -294,7 +294,8 @@ struct SwWindow : Window
         if (!surface) return;

         //Set the canvas target and draw on it.
-        verify(static_cast<tvg::SwCanvas*>(canvas)->target((uint32_t*)surface->pixels, surface->w, surface->pitch / 4, surface->h, tvg::ColorSpace::ARGB8888));
+        printf("stride = %d, width = %d\n", surface->pitch / 4, 1080);
+        verify(static_cast<tvg::SwCanvas*>(canvas)->target((uint32_t*)surface->pixels, surface->pitch / 4, 1080, surface->h, tvg::ColorSpace::ARGB8888));
     }

     void refresh() override

[elahav]

• Scenario: playing a 1080x1080 Lottie animation file. The window's buffer (due to alignment considerations) is 1088x1080 (i.e., a stride of 1088). This is on QNX x86_64 and I built thorVG with AVX support (though I don't think that matters).
• SwRenderer::request is called, which calls malloc() on a buffer of size 4x1080x1080. Note that this is a second surface, not the one to which the canvas was attached. I think this extra surface is used for composition, but I'm not a graphics guy.
• In the backtrace I sent, the surface for which this compositor buffer was allocated is at address 0x91afa00, and the buffer returned by `malloc() is at 0x92de010
• The call to rasterClear() ends up going beyond the end of this buffer as a result of the following code, which uses the stride to calculate the offset:

        //partial clear
        } else {
            for (uint32_t i = 0; i < h; i++) {
                rasterPixel32(surface->buf32, val, (surface->stride * y + x) + (surface->stride * i), w);
            }
        }

Let me know if you need anything else.

[hermet]

@elahav Can you share the Lottie sample you have? I can't find the rasterClear() and AVX have any issues themselves. The best is to reproduce the crash issue you have here. I've come across another rendering problem, but it might not be related to your crash. so need to clarify that.

[hermet]

@elahav please cherry-pick and test with this commit. This could help your issue. thanks.

[elahav]

The segmentation fault is gone with this fix.

[hermet]

@elahav The fix will be in v0.15.9. thanks. cc: @mgrudzinska

[mgrudzinska]

https://github.com/thorvg/thorvg/releases/tag/v0.15.9