summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStephan Bergmann <sbergman@redhat.com>2021-06-03 07:54:27 +0200
committerStephan Bergmann <sbergman@redhat.com>2021-06-03 10:30:17 +0200
commitf5e1314ffa564077c27fb9c954c792b498bcae12 (patch)
tree1a54e01136572da3420454d0e3676b573b0cd7da
parent91c23f55bb795156c2ea3b2c4458e70758a425c4 (diff)
external/cairo: Fix previous -fsanitize=alignment fix
13f6d80330208eeb45fe9a03bb462941fb4eda2a "external/cairo: Support building with ASan/UBSan" had added the src/cairo-image-source.c blob to external/cairo/cairo/san.patch.0 to fix > > cairo-image-source.c:512:10: runtime error: load of misaligned address 0x6180037aee6f for type 'uint32_t' (aka 'unsigned int'), which requires 4 byte alignment > during UITest_writer_tests7 I had created that patch attempting to do a faithful emulation of the original code (which I had naively assumed to be correct, modulo the alignment issue), reading four consecutive bytes and interpreting them as an uint32_t in the system's byte order. I had not noticed back then that all that CAIRO_FORMAT_RGB24_888 was added by external/cairo/cairo/cairo-1.10.2.patch, introduced with 54596087e57ea533253e19eea594d9b6c06e8d26 "vcl-svp: add 24-bit (3-byte) RGB surface support to Cairo". Its > + * @CAIRO_FORMAT_RGB24_888: each pixel is a 24-bit quantity, > + * with Red, Green, Blue taking 8-bits each, in that order. (Since 1.1x) and > + * Need this until CAIRO_FORMAT_RGB24_888 is in some official release. > + * Otherwise we can't reliably check if this is available or we should > + * convert from 24-bit RGB to 32-bit RGB before passing to Cairo. comments make it sound as if this code might come from some official cairo upstream branch, but I cannot find anything at git.freedesktop.org/git/cairo. I have no idea about the provenance and quality of that code. However, I now (a) from the above comments naively assume that the intent of CAIRO_FORMAT_RGB24_888 is to store R, G, B in that order in three bytes with increasing addresses (i.e., independent of the system's byte order for larger- than-byte words); (b) thus consider the original src/cairo-image-source.c code in external/cairo/cairo/cairo-1.10.2.patch broken (as it attempts to read all three values in one read, in system byte order, which would fail for big endian); and (c) thus consider the faithful emulation code in external/cairo/cairo/san.patch.0 to be broken, too. Based on the above, I here fix at least the external/cairo/cairo/san.patch.0 code (which is applied unconditionally, even for non-ASan/UBSan builds, thus always overriding the original external/cairo/cairo/cairo-1.10.2.patch code). The following line > pixel &= 0x00ffffff; /* ignore next pixel bits */ should no longer be necessary now, and it is probably better to directly fix the original code in external/cairo/cairo/cairo-1.10.2.patch, but I'll leave that for a potential follow-up fix, once the provenance and assumed quality of that original CAIRO_FORMAT_RGB24_888 code is clarified. (I came across this code now with a --without-system-cairo ASan/UBSan Linux build, where JunitTest_toolkit_unoapi_1 failed with > ==1060406==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60300146fb48 at pc 0x7fff9b45b338 bp 0x7ffffffef1d0 sp 0x7ffffffef1c8 > READ of size 1 at 0x60300146fb48 thread T0 > Detaching from process 1060652 > #0 in _pixel_to_solid at workdir/UnpackedTarball/cairo/src/cairo-image-source.c:515:207 > #1 in _pixman_image_for_surface at workdir/UnpackedTarball/cairo/src/cairo-image-source.c:1309:22 > #2 in _pixman_image_for_pattern at workdir/UnpackedTarball/cairo/src/cairo-image-source.c:1574:9 > #3 in _cairo_image_source_create_for_pattern at workdir/UnpackedTarball/cairo/src/cairo-image-source.c:1619:2 > #4 in composite_aligned_boxes at workdir/UnpackedTarball/cairo/src/cairo-spans-compositor.c:678:8 > #5 in clip_and_composite_boxes at workdir/UnpackedTarball/cairo/src/cairo-spans-compositor.c:882:11 > #6 in _cairo_spans_compositor_mask at workdir/UnpackedTarball/cairo/src/cairo-spans-compositor.c:999:14 > #7 in _cairo_compositor_mask at workdir/UnpackedTarball/cairo/src/cairo-compositor.c:106:11 > #8 in _cairo_image_surface_mask at workdir/UnpackedTarball/cairo/src/cairo-image-surface.c:952:12 > #9 in _cairo_surface_mask at workdir/UnpackedTarball/cairo/src/cairo-surface.c:2247:14 > #10 in _cairo_gstate_mask at workdir/UnpackedTarball/cairo/src/cairo-gstate.c:1136:11 > #11 in _cairo_default_context_mask at workdir/UnpackedTarball/cairo/src/cairo-default-context.c:993:12 > #12 in cairo_mask at workdir/UnpackedTarball/cairo/src/cairo.c:2283:14 > #13 in SvpSalGraphics::drawAlphaBitmap(SalTwoRect const&, SalBitmap const&, SalBitmap const&) at vcl/headless/svpgdi.cxx:745:5 > #14 in SalGraphics::DrawAlphaBitmap(SalTwoRect const&, SalBitmap const&, SalBitmap const&, OutputDevice const&) at vcl/source/gdi/salgdilayout.cxx:832:16 > #15 in OutputDevice::DrawDeviceAlphaBitmap(Bitmap const&, AlphaMask const&, Point const&, Size const&, Point const&, Size const&) at vcl/source/outdev/bitmap.cxx:704:29 > #16 in OutputDevice::DrawDeviceBitmap(Point const&, Size const&, Point const&, Size const&, BitmapEx&) at vcl/source/outdev/bitmap.cxx:516:9 > #17 in OutputDevice::DrawBitmapEx(Point const&, Size const&, Point const&, Size const&, BitmapEx const&, MetaActionType) at vcl/source/outdev/bitmap.cxx:391:9 > #18 in OutputDevice::DrawBitmapEx(Point const&, Size const&, BitmapEx const&) at vcl/source/outdev/bitmap.cxx:292:9 > #19 in OutputDevice::DrawTransformedBitmapEx(basegfx::B2DHomMatrix const&, BitmapEx const&, double) at vcl/source/outdev/bitmap.cxx:1315:9 > #20 in drawinglayer::processor2d::VclProcessor2D::RenderBitmapPrimitive2D(drawinglayer::primitive2d::BitmapPrimitive2D const&) at drawinglayer/source/processor2d/vclprocessor2d.cxx:394:21 > #21 in drawinglayer::processor2d::VclPixelProcessor2D::processBitmapPrimitive2D(drawinglayer::primitive2d::BitmapPrimitive2D const&) at drawinglayer/source/processor2d/vclpixelprocessor2d.cxx:524:5 > #22 in drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(drawinglayer::primitive2d::BasePrimitive2D const&) at drawinglayer/source/processor2d/vclpixelprocessor2d.cxx:252:13 > #23 in drawinglayer::processor2d::BaseProcessor2D::process(drawinglayer::primitive2d::Primitive2DContainer const&) at drawinglayer/source/processor2d/baseprocessor2d.cxx:69:25 > #24 in drawinglayer::processor2d::VclProcessor2D::RenderTransformPrimitive2D(drawinglayer::primitive2d::TransformPrimitive2D const&) at drawinglayer/source/processor2d/vclprocessor2d.cxx:912:5 > #25 in drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(drawinglayer::primitive2d::BasePrimitive2D const&) at drawinglayer/source/processor2d/vclpixelprocessor2d.cxx:317:13 > #26 in drawinglayer::processor2d::BaseProcessor2D::process(drawinglayer::primitive2d::Primitive2DContainer const&) at drawinglayer/source/processor2d/baseprocessor2d.cxx:69:25 > #27 in drawinglayer::processor2d::BaseProcessor2D::process(drawinglayer::primitive2d::BasePrimitive2D const&) at drawinglayer/source/processor2d/baseprocessor2d.cxx:46:13 > #28 in drawinglayer::processor2d::VclPixelProcessor2D::processBasePrimitive2D(drawinglayer::primitive2d::BasePrimitive2D const&) at drawinglayer/source/processor2d/vclpixelprocessor2d.cxx:434:13 > #29 in drawinglayer::processor2d::BaseProcessor2D::process(drawinglayer::primitive2d::Primitive2DContainer const&) at drawinglayer/source/processor2d/baseprocessor2d.cxx:69:25 > #30 in sdr::contact::ObjectContactOfPageView::DoProcessDisplay(sdr::contact::DisplayInfo&) at svx/source/sdr/contact/objectcontactofpageview.cxx:279:31 > #31 in sdr::contact::ObjectContactOfPageView::ProcessDisplay(sdr::contact::DisplayInfo&) at svx/source/sdr/contact/objectcontactofpageview.cxx:116:21 > #32 in SdrPageWindow::RedrawAll(sdr::contact::ViewObjectContactRedirector*) at svx/source/svdraw/sdrpagewindow.cxx:353:28 > #33 in SdrPageView::CompleteRedraw(SdrPaintWindow&, vcl::Region const&, sdr::contact::ViewObjectContactRedirector*) at svx/source/svdraw/svdpagv.cxx:239:18 > #34 in SdrPaintView::DoCompleteRedraw(SdrPaintWindow&, vcl::Region const&, sdr::contact::ViewObjectContactRedirector*) at svx/source/svdraw/svdpntv.cxx:606:21 > #35 in SdrPaintView::CompleteRedraw(OutputDevice*, vcl::Region const&, sdr::contact::ViewObjectContactRedirector*) at svx/source/svdraw/svdpntv.cxx:519:5 > #36 in sd::View::CompleteRedraw(OutputDevice*, vcl::Region const&, sdr::contact::ViewObjectContactRedirector*) at sd/source/ui/view/sdview.cxx:475:17 > #37 in sd::DrawView::CompleteRedraw(OutputDevice*, vcl::Region const&, sdr::contact::ViewObjectContactRedirector*) at sd/source/ui/view/drawview.cxx:520:21 > #38 in sd::DrawViewShell::Paint(tools::Rectangle const&, sd::Window*) at sd/source/ui/view/drviews5.cxx:420:17 > #39 in sd::Window::Paint(OutputDevice&, tools::Rectangle const&) at sd/source/ui/view/sdwindow.cxx:212:22 > #40 in PaintHelper::DoPaint(vcl::Region const*) at vcl/source/window/paint.cxx:313:20 > #41 in vcl::Window::ImplCallPaint(vcl::Region const*, ImplPaintFlags) at vcl/source/window/paint.cxx:616:17 > #42 in PaintHelper::~PaintHelper() at vcl/source/window/paint.cxx:552:30 > #43 in vcl::Window::ImplCallPaint(vcl::Region const*, ImplPaintFlags) at vcl/source/window/paint.cxx:622:1 > #44 in PaintHelper::~PaintHelper() at vcl/source/window/paint.cxx:552:30 > #45 in vcl::Window::ImplCallPaint(vcl::Region const*, ImplPaintFlags) at vcl/source/window/paint.cxx:622:1 > #46 in PaintHelper::~PaintHelper() at vcl/source/window/paint.cxx:552:30 > #47 in vcl::Window::ImplCallPaint(vcl::Region const*, ImplPaintFlags) at vcl/source/window/paint.cxx:622:1 > #48 in PaintHelper::~PaintHelper() at vcl/source/window/paint.cxx:552:30 > #49 in vcl::Window::ImplCallPaint(vcl::Region const*, ImplPaintFlags) at vcl/source/window/paint.cxx:622:1 > #50 in vcl::Window::PaintImmediately() at vcl/source/window/paint.cxx:1325:24 > #51 in vcl::Window::PaintImmediately() at vcl/source/window/paint.cxx:1269:39 > #52 in SvImpLBox::MyUserEvent(void*) at vcl/source/treelist/svimpbox.cxx:3063:18 > #53 in SvImpLBox::LinkStubMyUserEvent(void*, void*) at vcl/source/treelist/svimpbox.cxx:3057:1 > #54 in Link<void*, void>::Call(void*) const at include/tools/link.hxx:111:45 > #55 in ImplHandleUserEvent(ImplSVEvent*) at vcl/source/window/winproc.cxx:1991:30 > #56 in ImplWindowFrameProc(vcl::Window*, SalEvent, void const*) at vcl/source/window/winproc.cxx:2561:13 > #57 in SalFrame::CallCallback(SalEvent, void const*) const at vcl/inc/salframe.hxx:306:29 > #58 in SvpSalInstance::ProcessEvent(SalUserEventList::SalUserEvent) at vcl/headless/svpinst.cxx:312:22 > #59 in non-virtual thunk to SvpSalInstance::ProcessEvent(SalUserEventList::SalUserEvent) at vcl/headless/svpinst.cxx (instdir/program/libvcllo.so +0xae7a222) > #60 in SalUserEventList::DispatchUserEvents(bool)::$_0::operator()() const at vcl/source/app/salusereventlist.cxx:119:58 > #61 in SalUserEventList::DispatchUserEvents(bool) at vcl/source/app/salusereventlist.cxx:120:13 > #62 in SvpSalInstance::DoYield(bool, bool) at vcl/headless/svpinst.cxx:461:19 > #63 in ImplYield(bool, bool) at vcl/source/app/svapp.cxx:465:48 > #64 in Application::Yield() at vcl/source/app/svapp.cxx:532:5 > #65 in Application::Execute() at vcl/source/app/svapp.cxx:444:9 > #66 in desktop::Desktop::Main() at desktop/source/app/app.cxx:1587:13 > #67 in ImplSVMain() at vcl/source/app/svmain.cxx:198:35 > #68 in SVMain() at vcl/source/app/svmain.cxx:230:12 > #69 in soffice_main at desktop/source/app/sofficemain.cxx:98:12 > #70 in sal_main at desktop/source/app/main.c:49:15 > #71 in main at desktop/source/app/main.c:47:1 > #72 in __libc_start_main at /usr/src/debug/glibc-2.33-8.fc34.x86_64/csu/../csu/libc-start.c:332:16 > #73 in _start at <null> (instdir/program/soffice.bin +0x251acd) > > 0x60300146fb48 is located 0 bytes to the right of 24-byte region [0x60300146fb30,0x60300146fb48) > allocated by thread T0 here: > #0 in operator new[](unsigned long) at ~/github.com/llvm/llvm-project/compiler-rt/lib/asan/asan_new_delete.cpp:102:3 > #1 in ImplCreateDIB(Size const&, vcl::PixelFormat, BitmapPalette const&) at vcl/headless/svpbmp.cxx:126:24 > #2 in SvpSalBitmap::Create(Size const&, vcl::PixelFormat, BitmapPalette const&) at vcl/headless/svpbmp.cxx:156:13 > #3 in Bitmap::Bitmap(Size const&, vcl::PixelFormat, BitmapPalette const*) at vcl/source/bitmap/bitmap.cxx:135:15 > #4 in Bitmap::Crop(tools::Rectangle const&) at vcl/source/bitmap/bitmap.cxx:489:20 > #5 in BitmapEx::Crop(tools::Rectangle const&) at vcl/source/bitmap/BitmapEx.cxx:360:25 > #6 in drawinglayer::primitive2d::DiscreteShadow::getLeft() const at drawinglayer/source/primitive2d/discreteshadowprimitive2d.cxx:148:61 > #7 in drawinglayer::primitive2d::DiscreteShadowPrimitive2D::create2DDecomposition(drawinglayer::primitive2d::Primitive2DContainer&, drawinglayer::geometry::ViewInformation2D const&) const at drawinglayer/source/primitive2d/discreteshadowprimitive2d.cxx:251:72 > #8 in drawinglayer::primitive2d::BufferedDecompositionPrimitive2D::get2DDecomposition(drawinglayer::primitive2d::Primitive2DDecompositionVisitor&, drawinglayer::geometry::ViewInformation2D const&) const at drawinglayer/source/primitive2d/baseprimitive2d.cxx:122:9 > #9 in drawinglayer::primitive2d::DiscreteMetricDependentPrimitive2D::get2DDecomposition(drawinglayer::primitive2d::Primitive2DDecompositionVisitor&, drawinglayer::geometry::ViewInformation2D const&) const at drawinglayer/source/primitive2d/primitivetools2d.cxx:48:47 trying to read the fourth byte of a presumed uint32_t starting at offset 21 of a 24-byte buffer.) Change-Id: Ibe8bc39e3736c64ace61af2217100c9d8bb96d5c Reviewed-on: https://gerrit.libreoffice.org/c/core/+/116637 Tested-by: Jenkins Reviewed-by: Stephan Bergmann <sbergman@redhat.com>
-rw-r--r--external/cairo/cairo/san.patch.08
1 files changed, 2 insertions, 6 deletions
diff --git a/external/cairo/cairo/san.patch.0 b/external/cairo/cairo/san.patch.0
index 9e187240f240..bbf8f2d626c0 100644
--- a/external/cairo/cairo/san.patch.0
+++ b/external/cairo/cairo/san.patch.0
@@ -57,16 +57,12 @@
static inline cairo_bool_t
--- src/cairo-image-source.c
+++ src/cairo-image-source.c
-@@ -509,7 +509,11 @@
+@@ -509,7 +509,7 @@
return pixman_image_create_solid_fill (&color);
case CAIRO_FORMAT_RGB24_888:
- pixel = *(uint32_t *) (image->data + y * image->stride + 3 * x);
-+#ifdef WORDS_BIGENDIAN
-+ pixel = (uint32_t)(image->data + y * image->stride + 3 * x)[3] | ((uint32_t)(image->data + y * image->stride + 3 * x)[2] << 8) | ((uint32_t)(image->data + y * image->stride + 3 * x)[1] << 16) | ((uint32_t)(image->data + y * image->stride + 3 * x)[0] << 24);
-+#else
-+ pixel = (uint32_t)(image->data + y * image->stride + 3 * x)[0] | ((uint32_t)(image->data + y * image->stride + 3 * x)[1] << 8) | ((uint32_t)(image->data + y * image->stride + 3 * x)[2] << 16) | ((uint32_t)(image->data + y * image->stride + 3 * x)[3] << 24);
-+#endif
++ pixel = (uint32_t)(image->data + y * image->stride + 3 * x)[0] | ((uint32_t)(image->data + y * image->stride + 3 * x)[1] << 8) | ((uint32_t)(image->data + y * image->stride + 3 * x)[2] << 16);
pixel &= 0x00ffffff; /* ignore next pixel bits */
if (pixel == 0)
return _pixman_black_image ();