[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    D11198: [libbreezecommon] add box shadow helper
From:       Vlad Zagorodniy <noreply () phabricator ! kde ! org>
Date:       2018-04-22 11:43:19
Message-ID: 20180422114319.1.0F91301FF78DCEC4 () phabricator ! kde ! org
[Download RAW message or body]

zzag updated this revision to Diff 32789.
zzag added a comment.


  Fix invalid read of size 1
  
  Valgrind output:
  
    ==8054== Invalid read of size 1
    ==8054==    at 0x1D8818C6: Breeze::BoxShadowHelper::blurAlphaNaivePass(QImage \
const&, QImage&, QVector<double> const&) (in \
/home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80)  ==8054==    by 0x1D8819F3: \
Breeze::BoxShadowHelper::blurAlphaNaive(QImage&, int) (in \
/home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80)  ==8054==    by 0x1D8822DA: \
Breeze::BoxShadowHelper::boxShadow(QPainter*, QRect const&, QPoint const&, int, \
QColor const&) (in /home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80)  ==8054==    \
by 0x1D60C8CC: Breeze::ShadowHelper::shadowTiles() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x1D60C341: \
Breeze::ShadowHelper::loadConfig() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x1D618B51: \
Breeze::Style::loadConfiguration() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x1D613D3E: \
Breeze::Style::Style() (in /home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  \
==8054==    by 0x1D63C52F: Breeze::StylePlugin::create(QString const&) (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x7B18B62: \
QStyleFactory::create(QString const&) (in /usr/lib/libQt5Widgets.so.5.10.1)  ==8054== \
by 0x7AABA9B: QApplication::style() (in /usr/lib/libQt5Widgets.so.5.10.1)  ==8054==   \
by 0x7AABDF5: QApplicationPrivate::initialize() (in /usr/lib/libQt5Widgets.so.5.10.1) \
==8054==    by 0x7AABE5A: QApplicationPrivate::init() (in \
/usr/lib/libQt5Widgets.so.5.10.1)  ==8054==  Address 0x16b332f7 is 3 bytes after a \
block of size 19,044 alloc'd  ==8054==    at 0x4C2CEDF: malloc \
(vg_replace_malloc.c:299)  ==8054==    by 0x82FDEDA: QImageData::create(QSize const&, \
QImage::Format) (in /usr/lib/libQt5Gui.so.5.10.1)  ==8054==    by 0x82FE06C: \
QImage::QImage(QSize const&, QImage::Format) (in /usr/lib/libQt5Gui.so.5.10.1)  \
==8054==    by 0x82FE0A5: QImage::QImage(int, int, QImage::Format) (in \
/usr/lib/libQt5Gui.so.5.10.1)  ==8054==    by 0x1D8819C5: \
Breeze::BoxShadowHelper::blurAlphaNaive(QImage&, int) (in \
/home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80)  ==8054==    by 0x1D8822DA: \
Breeze::BoxShadowHelper::boxShadow(QPainter*, QRect const&, QPoint const&, int, \
QColor const&) (in /home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80)  ==8054==    \
by 0x1D60C8CC: Breeze::ShadowHelper::shadowTiles() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x1D60C341: \
Breeze::ShadowHelper::loadConfig() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x1D618B51: \
Breeze::Style::loadConfiguration() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x1D613D3E: \
Breeze::Style::Style() (in /home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  \
==8054==    by 0x1D63C52F: Breeze::StylePlugin::create(QString const&) (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so)  ==8054==    by 0x7B18B62: \
QStyleFactory::create(QString const&) (in /usr/lib/libQt5Widgets.so.5.10.1)  
  The reason: I forgot that the kernel is of size `2 * radius + 1` so when \
blurAlphaNaivePass   convolving near ends it doesn't take 1 into account. Overall, \
fix looks like this  
    diff --git a/libbreezecommon/breezeboxshadowhelper.cpp \
b/libbreezecommon/breezeboxshadowhelper.cpp  index 625cb26a..17d18ecd 100644
    --- a/libbreezecommon/breezeboxshadowhelper.cpp
    +++ b/libbreezecommon/breezeboxshadowhelper.cpp
    @@ -118,7 +118,7 @@ void blurAlphaNaivePass(const QImage &src, QImage &dst, const \
QVector<qreal> &ke  }
    
             for (int x = src.width() - radius; x < src.width(); x++) {
    -            const uchar *window = in + (x - radius) * alphaStride;
    +            const uchar *window = in + (x - radius - 1) * alphaStride;
                 qreal alpha = 0;
                 const int outside = x + radius - src.width();
                 for (int k = 0; k < kernel.size() - outside; k++) {

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11198?vs=31846&id=32789

BRANCH
  arcpatch-D11198

REVISION DETAIL
  https://phabricator.kde.org/D11198

AFFECTED FILES
  CMakeLists.txt
  cmake/Modules/FindFFTW.cmake
  kstyle/CMakeLists.txt
  libbreezecommon/CMakeLists.txt
  libbreezecommon/breezeboxshadowhelper.cpp
  libbreezecommon/breezeboxshadowhelper.h
  libbreezecommon/config-breezecommon.h.cmake

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: broulik, abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, \
jensreuterberg, sebas, apol, mart


[Attachment #3 (unknown)]

<table><tr><td style="">zzag updated this revision to Diff 32789.<br />zzag added a \
comment. </td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; \
float: right; color: #464C5C; font-weight: bold; border-radius: 3px; \
background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); \
display: inline-block; border: 1px solid rgba(71,87,120,.2);" \
href="https://phabricator.kde.org/D11198">View Revision</a></tr></table><br \
/><div><div><p>Fix invalid read of size 1</p>

<p>Valgrind output:</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="text" \
data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: \
12px; margin: 0; background: rgba(71, 87, 120, 0.08);">==8054== Invalid read of size \
1 ==8054==    at 0x1D8818C6: Breeze::BoxShadowHelper::blurAlphaNaivePass(QImage \
const&amp;, QImage&amp;, QVector&lt;double&gt; const&amp;) (in \
/home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80) ==8054==    by 0x1D8819F3: \
Breeze::BoxShadowHelper::blurAlphaNaive(QImage&amp;, int) (in \
/home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80) ==8054==    by 0x1D8822DA: \
Breeze::BoxShadowHelper::boxShadow(QPainter*, QRect const&amp;, QPoint const&amp;, \
int, QColor const&amp;) (in /home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80) \
==8054==    by 0x1D60C8CC: Breeze::ShadowHelper::shadowTiles() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x1D60C341: \
Breeze::ShadowHelper::loadConfig() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x1D618B51: \
Breeze::Style::loadConfiguration() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x1D613D3E: \
Breeze::Style::Style() (in /home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) \
==8054==    by 0x1D63C52F: Breeze::StylePlugin::create(QString const&amp;) (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x7B18B62: \
QStyleFactory::create(QString const&amp;) (in /usr/lib/libQt5Widgets.so.5.10.1) \
==8054==    by 0x7AABA9B: QApplication::style() (in /usr/lib/libQt5Widgets.so.5.10.1) \
==8054==    by 0x7AABDF5: QApplicationPrivate::initialize() (in \
/usr/lib/libQt5Widgets.so.5.10.1) ==8054==    by 0x7AABE5A: \
QApplicationPrivate::init() (in /usr/lib/libQt5Widgets.so.5.10.1) ==8054==  Address \
0x16b332f7 is 3 bytes after a block of size 19,044 alloc&#039;d ==8054==    at \
0x4C2CEDF: malloc (vg_replace_malloc.c:299) ==8054==    by 0x82FDEDA: \
QImageData::create(QSize const&amp;, QImage::Format) (in \
/usr/lib/libQt5Gui.so.5.10.1) ==8054==    by 0x82FE06C: QImage::QImage(QSize \
const&amp;, QImage::Format) (in /usr/lib/libQt5Gui.so.5.10.1) ==8054==    by \
0x82FE0A5: QImage::QImage(int, int, QImage::Format) (in /usr/lib/libQt5Gui.so.5.10.1) \
==8054==    by 0x1D8819C5: Breeze::BoxShadowHelper::blurAlphaNaive(QImage&amp;, int) \
(in /home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80) ==8054==    by 0x1D8822DA: \
Breeze::BoxShadowHelper::boxShadow(QPainter*, QRect const&amp;, QPoint const&amp;, \
int, QColor const&amp;) (in /home/vlad/KDE/usr/lib64/libbreezecommon.so.5.12.80) \
==8054==    by 0x1D60C8CC: Breeze::ShadowHelper::shadowTiles() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x1D60C341: \
Breeze::ShadowHelper::loadConfig() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x1D618B51: \
Breeze::Style::loadConfiguration() (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x1D613D3E: \
Breeze::Style::Style() (in /home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) \
==8054==    by 0x1D63C52F: Breeze::StylePlugin::create(QString const&amp;) (in \
/home/vlad/KDE/usr/lib64/plugins/styles/breeze.so) ==8054==    by 0x7B18B62: \
QStyleFactory::create(QString const&amp;) (in \
/usr/lib/libQt5Widgets.so.5.10.1)</pre></div>

<p>The reason: I forgot that the kernel is of size <tt style="background: #ebebeb; \
font-size: 13px;">2 * radius + 1</tt> so when blurAlphaNaivePass <br /> convolving \
near ends it doesn&#039;t take 1 into account. Overall, fix looks like this</p>

<div class="remarkup-code-block" style="margin: 12px 0;" data-code-lang="diff" \
data-sigil="remarkup-code-block"><pre class="remarkup-code" style="font: 11px/15px \
&quot;Menlo&quot;, &quot;Consolas&quot;, &quot;Monaco&quot;, monospace; padding: \
12px; margin: 0; background: rgba(71, 87, 120, 0.08);"><span style="color: \
#000080">diff --git a/libbreezecommon/breezeboxshadowhelper.cpp \
b/libbreezecommon/breezeboxshadowhelper.cpp</span> <span style="color: #000080">index \
625cb26a..17d18ecd 100644</span> <span style="color: #a00000">--- \
a/libbreezecommon/breezeboxshadowhelper.cpp</span> <span style="color: #00a000">+++ \
b/libbreezecommon/breezeboxshadowhelper.cpp</span> <span style="color: #800080">@@ \
-118,7 +118,7 @@ void blurAlphaNaivePass(const QImage &amp;src, QImage &amp;dst, \
const QVector&lt;qreal&gt; &amp;ke</span>  }

         for (int x = src.width() - radius; x &lt; src.width(); x++) {
<span style="color: #a00000">-            const uchar *window = in + (x - radius) * \
alphaStride;</span> <span style="color: #00a000">+            const uchar *window = \
in + (x - radius - 1) * alphaStride;</span>  qreal alpha = 0;
             const int outside = x + radius - src.width();
             for (int k = 0; k &lt; kernel.size() - outside; k++) \
{</pre></div></div></div><br /><div><strong>REPOSITORY</strong><div><div>R31 \
Breeze</div></div></div><br /><div><strong>CHANGES SINCE LAST UPDATE</strong><div><a \
href="https://phabricator.kde.org/D11198?vs=31846&amp;id=32789">https://phabricator.kde.org/D11198?vs=31846&amp;id=32789</a></div></div><br \
/><div><strong>BRANCH</strong><div><div>arcpatch-D11198</div></div></div><br \
/><div><strong>REVISION DETAIL</strong><div><a \
href="https://phabricator.kde.org/D11198">https://phabricator.kde.org/D11198</a></div></div><br \
/><div><strong>AFFECTED FILES</strong><div><div>CMakeLists.txt<br /> \
cmake/Modules/FindFFTW.cmake<br /> kstyle/CMakeLists.txt<br />
libbreezecommon/CMakeLists.txt<br />
libbreezecommon/breezeboxshadowhelper.cpp<br />
libbreezecommon/breezeboxshadowhelper.h<br />
libbreezecommon/config-breezecommon.h.cmake</div></div></div><br /><div><strong>To: \
</strong>zzag, Breeze, VDG, hpereiradacosta<br /><strong>Cc: </strong>broulik, \
abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, \
jensreuterberg, sebas, apol, mart<br /></div>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic