| Summary: | Crash on saving an image > 65.535 (2^16-1) due to insufficient math ranges | ||
|---|---|---|---|
| Product: | [Applications] krita | Reporter: | Tiar <tamtamy.tymona> |
| Component: | General | Assignee: | Tiar <tamtamy.tymona> |
| Status: | RESOLVED FIXED | ||
| Severity: | crash | CC: | halla |
| Priority: | NOR | ||
| Version: | 4.4.2 | ||
| Target Milestone: | --- | ||
| Platform: | Mint (Ubuntu based) | ||
| OS: | Linux | ||
| Latest Commit: | https://invent.kde.org/graphics/krita/commit/0817c0f56c1b947d4cc76f824f330af6b8d318f4 | Version Fixed In: | |
| Sentry Crash Report: | |||
Let's confirm it since there are at least three people who encountered it, me, windragon and wayneqq from https://forum.kde.org/viewtopic.php?f=139&t=169787 Let's assign it as well :P Git commit fc52cc9ca602b75d423a5b8b4f5bbea882393938 by Agata Cacko. Committed on 30/03/2021 at 12:11. Pushed by tymond into branch 'master'. Fix crash on saving a huge image to .kra Before this commit, an image of size bigger than 66k in any dimension would crash Krita because it tries to scale the projection to 256x256 image, which means that the scale would be less than 1/256. KisTransformWorker uses KisFixedPoint to scale, which means that the scale would (in KisFixedPoint maths) equal 0, which means there is division by 0. M +9 -0 libs/image/kis_image.cc https://invent.kde.org/graphics/krita/commit/fc52cc9ca602b75d423a5b8b4f5bbea882393938 Git commit c6608d08c24f8a5417425e13e4c040cde891b43b by Agata Cacko. Committed on 30/03/2021 at 12:11. Pushed by tymond into branch 'master'. Ensure that transform worker won't try to scale to 0 Transform Worker did have a check for scale being 0... but it's only in the double value. KisFilterWeightsBuffer converts those values to KisFixedPoint, which has much different precision (the scale cannot be lower than 1/256, otherwise it gets rounded to 0). This commit adds a check after converting to KisFixedPoint. Note: it would be best if the KisFixedPoint had higher precision. I added the conversion check there, because it's an easy operation, but it could be KisFilterWeightsBuffer and KisFilterWeightsApplicator checking the values to make sure they're correct transformation-wise and KisTransfromWorker only checking some kind of buffer->valid() and applicator->valid() result. M +8 -3 libs/image/kis_transform_worker.cc https://invent.kde.org/graphics/krita/commit/c6608d08c24f8a5417425e13e4c040cde891b43b Git commit 0817c0f56c1b947d4cc76f824f330af6b8d318f4 by Agata Cacko. Committed on 03/06/2021 at 21:21. Pushed by tymond into branch 'krita/4.3'. Fix crash on saving a huge image to .kra Before this commit, an image of size bigger than 66k in any dimension would crash Krita because it tries to scale the projection to 256x256 image, which means that the scale would be less than 1/256. KisTransformWorker uses KisFixedPoint to scale, which means that the scale would (in KisFixedPoint maths) equal 0, which means there is division by 0. M +9 -0 libs/image/kis_image.cc https://invent.kde.org/graphics/krita/commit/0817c0f56c1b947d4cc76f824f330af6b8d318f4 Git commit d621189b910fec56b38890dab54f7e218c6e6b3b by Agata Cacko. Committed on 03/06/2021 at 21:21. Pushed by tymond into branch 'krita/4.3'. Ensure that transform worker won't try to scale to 0 Transform Worker did have a check for scale being 0... but it's only in the double value. KisFilterWeightsBuffer converts those values to KisFixedPoint, which has much different precision (the scale cannot be lower than 1/256, otherwise it gets rounded to 0). This commit adds a check after converting to KisFixedPoint. Note: it would be best if the KisFixedPoint had higher precision. I added the conversion check there, because it's an easy operation, but it could be KisFilterWeightsBuffer and KisFilterWeightsApplicator checking the values to make sure they're correct transformation-wise and KisTransfromWorker only checking some kind of buffer->valid() and applicator->valid() result. M +8 -3 libs/image/kis_transform_worker.cc https://invent.kde.org/graphics/krita/commit/d621189b910fec56b38890dab54f7e218c6e6b3b |
SUMMARY If you create a file 4.000 x 90.000, you will get a crash while saving. STEPS TO REPRODUCE 1. Create 4.000 x 90.000 file 2. Try to save. OBSERVED RESULT Crash EXPECTED RESULT No crash, saved file EXPLANATION OF THE CRASH: The crash appears because of insufficient maths in KisFixedPoint. Krita tries to scale the image down to 256x256 image to make a preview. The scale ends up being ~0.0027499999999999998. The value gets multiplied 2^8 times to end up integer since it's KisFixedPoint class, so it's used to get fixed point arithmetics. But in this case, it's outside of the range: 0.0027499999999999998*(2^8) = ~0.7, which is less than 1, which means the actual number assigned is 0. Then of course something inside complains about dividing by 0. SOFTWARE/OS VERSIONS Krita Version: 5.0.0-prealpha (git cffe4e7) Languages: en_US, en, en_US, en, en_US, en, pl_PL, pl, pl_PL, pl Hidpi: true Qt Version (compiled): 5.11.1 Version (loaded): 5.11.1 OS Information Build ABI: x86_64-little_endian-lp64 Build CPU: x86_64 CPU: x86_64 Kernel Type: linux Kernel Version: 5.3.7-050307-generic Pretty Productname: Linux Mint 19.3 Product Type: linuxmint Product Version: 19.3 Desktop: X-Cinnamon ADDITIONAL INFORMATION Crash log: --- Thread 70 "Thread (pooled)" received signal SIGFPE, Arithmetic exception. [Switching to Thread 0x7fffb8ff9700 (LWP 2088)] 0x00007ffff5d3c94a in KisFixedPoint::operator/= (x=..., this=<synthetic pointer>) at /home/tymon/kritadev/krita/libs/image/kis_fixed_point_maths.h:98 98 d /= x.d; (gdb) bt #0 0x00007ffff5d3c94a in KisFixedPoint::operator/=(KisFixedPoint const&) (x=..., this=<synthetic pointer>) at /home/tymon/kritadev/krita/libs/image/kis_fixed_point_maths.h:98 #1 0x00007ffff5d3c94a in boost::operators_impl::operator/(KisFixedPoint const&, KisFixedPoint const&) (rhs=..., lhs=<synthetic pointer>...) at /usr/include/boost/operators.hpp:262 #2 0x00007ffff5d3c94a in KisFilterWeightsBuffer::KisFilterWeightsBuffer(KisFilterStrategy*, double) (this=0x7fffb8ff7f70, filterStrategy=0x555568450800, realScale=<optimized out>) at /home/tymon/kritadev/krita/libs/image/kis_filter_weights_buffer.h:174 #3 0x00007ffff5d3d0eb in KisTransformWorker::transformPass<KisSharedPtr<KisHLineIteratorNG> >(KisPaintDevice*, KisPaintDevice*, double, double, double, KisFilterStrategy*, int) (this=this@entry=0x7fffb8ff8560, src=0x7fff8c006330, dst=0x7fff8c006330, floatscale=0.0027499999999999998, shear=0, dx=0, filterStrategy=0x555568450800, portion=portion@entry=50) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qglobal.h:581 #4 0x00007ffff5d3be81 in KisTransformWorker::runPartial(QRect const&) (this=0x7fffb8ff8560, processRect=...) at /home/tymon/kritadev/krita/libs/image/kis_transform_worker.cc:344 #5 0x00007ffff5d3c6ff in KisTransformWorker::run() (this=0x7fffb8ff8560) at /home/tymon/kritadev/krita/libs/global/kis_shared_ptr.h:167 #6 0x00007ffff5c3b5f7 in KisImage::convertToQImage(QSize const&, KoColorProfile const*) (this=<optimized out>, scaledImageSize=..., profile=0x0) at /home/tymon/kritadev/krita/libs/image/kis_image.cc:1582 #7 0x00007ffff701dc45 in KisDocument::generatePreview(QSize const&) (this=<optimized out>, size=...) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qflags.h:120 #8 0x00007fffbb4b1557 in KraConverter::savePreview(KoStore*) (this=0x7fffb8ff8a60, store=0x7fff8c004400) at /usr/include/x86_64-linux-gnu/qt5/QtCore/qsize.h:119 #9 0x00007fffbb4b21d8 in KraConverter::saveRootDocuments(KoStore*) (this=0x7fffb8ff8a60, store=0x7fff8c004400) at /home/tymon/kritadev/krita/plugins/impex/kra/kra_converter.cpp:225 #10 0x00007fffbb4b261d in KraConverter::buildFile(QIODevice*, QString const&) (this=0x7fffb8ff8a60, io=<optimized out>, filename=...) at /home/tymon/kritadev/krita/plugins/impex/kra/kra_converter.cpp:152 #11 0x00007fffbb4aa001 in KraExport::convert(KisDocument*, QIODevice*, KisPinnedSharedPtr<KisPropertiesConfiguration>) (this=0x55556d421bd0, document=<optimized out>, io=0x7fffb8ff8b50) at /home/tymon/kritadev/krita/plugins/impex/kra/kra_export.cpp:49 #12 0x00007ffff702dfde in KisImportExportManager::doExportImpl(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>) (this=0x55555fc8e710, location=..., filter=..., exportConfiguration=...) at /usr/include/c++/8/bits/atomic_base.h:295 #13 0x00007ffff702e340 in KisImportExportManager::doExport(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool) (this=0x55555fc8e710, location=..., filter=..., exportConfiguration=..., alsoAsKra=<optimized out>) at /usr/include/c++/8/bits/atomic_base.h:295 #14 0x00007ffff70308af in std::__invoke_impl<KisImportExportErrorCode, KisImportExportErrorCode (KisImportExportManager::*&)(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool), KisImportExportManager*&, QString&, QSharedPointer<KisImportExportFilter>&, KisPinnedSharedPtr<KisPropertiesConfiguration>&, bool&>(std::__invoke_memfun_deref, KisImportExportErrorCode (KisImportExportManager::*&)(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool), KisImportExportManager*&, QString&, QSharedPointer<KisImportExportFilter>&, KisPinnedSharedPtr<KisPropertiesConfiguration>&, bool&) (__t=@0x55556f918ec0: 0x55555fc8e710, __f= @0x55556f918e88: (KisImportExportErrorCode (KisImportExportManager::*)(KisImportExportManager * const, const QString &, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)) 0x7ffff702e2c0 <KisImportExportManager::doExport(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)>) at /usr/include/c++/8/bits/atomic_base.h:295 #15 0x00007ffff70308af in std::__invoke<KisImportExportErrorCode (KisImportExportManager::*&)(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool), KisImportExportManager*&, QString&, QSharedPointer<KisImportExportFilter>&, KisPinnedSharedPtr<KisPropertiesConfiguration>&, bool&>(KisImportExportErrorCode (KisImportExportManager::*&)(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool), KisImportExportManager*&, QString&, QSharedPointer<KisImportExportFilter>&, KisPinnedSharedPtr<KisPropertiesConfiguration>&, bool&) (__fn= @0x55556f918e88: (KisImportExportErrorCode (KisImportExportManager::*)(KisImportExportManager * const, const QString &, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)) 0x7ffff702e2c0 <KisImportExportManager::doExport(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)>) at /usr/include/c++/8/bits/invoke.h:96 #16 0x00007ffff70308af in std::_Bind<KisImportExportErrorCode (KisImportExportManager::*(KisImportExportManager*, QString, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool))(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)>::__call<KisImportExportErrorCode, , 0ul, 1ul, 2ul, 3ul, 4ul>(std::tuple<>&&, std::_Index_tuple<0ul, 1ul, 2ul, 3ul, 4ul>) (__args=..., this=0x55556f918e88) at /usr/include/c++/8/functional:402 #17 0x00007ffff70308af in std::_Bind<KisImportExportErrorCode (KisImportExportManager::*(KisImportExportManager*, QString, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool))(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)>::operator()<, KisImportExportErrorCode>() (this=0x55556f918e88) at /usr/include/c++/8/functional:484 #18 0x00007ffff70308af in QtConcurrent::StoredFunctorCall0<KisImportExportErrorCode, std::_Bind<KisImportExportErrorCode (KisImportExportManager::*(KisImportExportManager*, QString, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool))(QString const&, QSharedPointer<KisImportExportFilter>, KisPinnedSharedPtr<KisPropertiesConfiguration>, bool)> >::runFunctor() (this=0x55556f918e40) at /usr/include/x86_64-linux-gnu/qt5/QtConcurrent/qtconcurrentstoredfunctioncall.h:60 #19 0x00007ffff70308af in QtConcurrent::RunFunctionTask<KisImportExportErrorCode>::run() (this=0x55556f918e40) at /usr/include/x86_64-linux-gnu/qt5/QtConcurrent/qtconcurrentrunbase.h:108 #20 0x00007ffff45f7f71 in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #21 0x00007ffff45ffc87 in () at /usr/lib/x86_64-linux-gnu/libQt5Core.so.5 #22 0x00007ffff19a7182 in start_thread (arg=<optimized out>) at pthread_create.c:486 #23 0x00007ffff3eceb1f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95