Bug 432182 - Crash on saving an image > 65.535 (2^16-1) due to insufficient math ranges
Summary: Crash on saving an image > 65.535 (2^16-1) due to insufficient math ranges
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: General (show other bugs)
Version: 4.4.2
Platform: Mint (Ubuntu based) Linux
: NOR crash
Target Milestone: ---
Assignee: Tiar
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-01-26 23:22 UTC by Tiar
Modified: 2021-06-03 21:32 UTC (History)
1 user (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tiar 2021-01-26 23:22:26 UTC
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
Comment 1 Tiar 2021-01-26 23:31:09 UTC
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
Comment 2 Halla Rempt 2021-01-27 09:38:50 UTC
Let's assign it as well :P
Comment 3 Tiar 2021-03-30 12:11:51 UTC
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
Comment 4 Tiar 2021-03-30 12:11:59 UTC
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
Comment 5 Tiar 2021-06-03 21:32:19 UTC
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
Comment 6 Tiar 2021-06-03 21:32:26 UTC
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