'Re: Review Request 126507: Fix leaked file description and potential use-after-free in kdelibs4suppo'

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

List:       kde-frameworks-devel
Subject:    Re: Review Request 126507: Fix leaked file description and potential use-after-free in kdelibs4suppo
From:       "Michael Pyne" <mpyne () kde ! org>
Date:       2016-01-02 23:29:35
Message-ID: 20160102232935.30660.80608 () mimi ! kde ! org
[Download RAW message or body]

--===============7861052559071916722==
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126507/
-----------------------------------------------------------

(Updated Jan. 2, 2016, 11:29 p.m.)

Status
------

This change has been marked as submitted.

Review request for KDE Frameworks.

Changes
-------

Submitted with commit f4e9dbf2b4ee2770e554c735b7604637e7b5ec54 by Michael Pyne to \
branch master.

Repository: kdelibs4support

Description
-------

Fix a couple of Coverity issues:

1. CID 1175508; file descriptors used in KLockFile could be leaked in
error conditions. Even when KLockFile sets mustCloseFd, the dtor's impl
also checks that the lock has been taken, which is only considered true
if LockOK had been returned in our lock function. Instead close() the fd
ourselves unless we make it to LockOK.

2. CID 1175555; The standard mis-use of QCache. QCache::insert can, in
theory, delete our object as soon as we insert it into cache, so we have
to check for that. Even ::contains() and ::object() can be risky (the
pointers returned by object() have no lifetime guarantee), but since
this is GUI code I assume it's only used single-threaded and not
re-entrant. Otherwise we'd need even more paranoia...

Diffs
-----

  src/kdecore/klockfile_unix.cpp 67283a5 
  src/kdeui/k4style.cpp a1a2ab1 

Diff: https://git.reviewboard.kde.org/r/126507/diff/

Testing
-------

Everything builds and appears to still work, though it's hard to test K4Style as I'm \
not sure what uses it right at this point.

Thanks,

Michael Pyne

--===============7861052559071916722==
MIME-Version: 1.0
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: 7bit

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 \
solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">  \
<tr>  <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://git.reviewboard.kde.org/r/126507/">https://git.reviewboard.kde.org/r/126507/</a>
  </td>
    </tr>
   </table>
   <br />

<table bgcolor="#e0e0e0" width="100%" cellpadding="12" style="border: 1px gray solid; \
border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">  <tr>
  <td>
   <h1 style="margin: 0; padding: 0; font-size: 10pt;">This change has been marked as \
submitted.</h1>  </td>
 </tr>
</table>
<br />

<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: \
1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; \
-webkit-border-radius: 6px;">  <tr>
  <td>

<div>Review request for KDE Frameworks.</div>
<div>By Michael Pyne.</div>

<p style="color: grey;"><i>Updated Jan. 2, 2016, 11:29 p.m.</i></p>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Submitted with commit f4e9dbf2b4ee2770e554c735b7604637e7b5ec54 by \
Michael Pyne to branch master.</pre>  </td>
 </tr>
</table>

<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs4support
</div>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Fix a couple of Coverity issues:</p> <ol \
style="padding: 0;text-rendering: inherit;margin: 0 0 0 2em;line-height: \
inherit;white-space: normal;"> <li style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: normal;"> <p style="padding: 0;text-rendering: \
inherit;margin: 0;line-height: inherit;white-space: inherit;">CID 1175508; file \
descriptors used in KLockFile could be leaked in error conditions. Even when \
KLockFile sets mustCloseFd, the dtor's impl also checks that the lock has been taken, \
which is only considered true if LockOK had been returned in our lock function. \
Instead close() the fd ourselves unless we make it to LockOK.</p>
</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: normal;"> <p style="padding: 0;text-rendering: inherit;margin: \
0;line-height: inherit;white-space: inherit;">CID 1175555; The standard mis-use of \
QCache. QCache::insert can, in theory, delete our object as soon as we insert it into \
cache, so we have to check for that. Even ::contains() and ::object() can be risky \
(the pointers returned by object() have no lifetime guarantee), but since
this is GUI code I assume it's only used single-threaded and not
re-entrant. Otherwise we'd need even more paranoia...</p>
</li>
</ol></pre>
  </td>
 </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: \
inherit;white-space: inherit;">Everything builds and appears to still work, though \
it's hard to test K4Style as I'm not sure what uses it right at this point.</p></pre> \
</td>  </tr>
</table>

<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>src/kdecore/klockfile_unix.cpp <span style="color: grey">(67283a5)</span></li>

 <li>src/kdeui/k4style.cpp <span style="color: grey">(a1a2ab1)</span></li>

</ul>

<p><a href="https://git.reviewboard.kde.org/r/126507/diff/" style="margin-left: \
3em;">View Diff</a></p>

  </td>
 </tr>
</table>

  </div>
 </body>
</html>

--===============7861052559071916722==--

[Attachment #3 (text/plain)]

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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