[SRU] Citical bug cherrypicks from SVN

Bug #110881 reported by John Dong on 2007-04-28
Affects Status Importance Assigned to Milestone
ktorrent (Ubuntu)
John Dong

Bug Description

Binary package hint: ktorrent

I have taken the time to cherrypick all the crash and data loss and other severe bug fixes from KTorrent 2.1-stable SVN. These patches are all fixes that have been incorporated in newer KTorrent 2.1 STABLE releases, such as 2.1.1-2.1.4. However, upstream's releases usually clump a bunch of other unrelated feature additions and behavioral changes not appropriate for SRU.

Changelog for SRU:
ktorrent (2.1-0ubuntu2.1~prop1) feisty-proposed; urgency=low

  * SRU: Cherrypick SVN data loss and crash related bugs. (Closes LP Bugs:
    109184, 104705, 110480, 99741, 95018, and duplicates)
     - debian/patches/kubuntu_04_svn632043_auth_crash.patch: Fixes crashes reported in authentication monitor
     - debian/patches/kubuntu_05_svn634519_chunkdownload_crash.patch: Fix SIGABORT crashes when loading a torrent. (KDE Bug 141758)
     - debian/patches/kubuntu_06_svn634843_retransmit_timeout.patch: Fixed bug with handling timeouts, which caused retransmits not to be sent
     - debian/patches/kubuntu_07_current_chunks_crash.patch: Fix crash caused by current_chunks being corrupt (KDE bug 142777)
     - debian/patches/kubuntu_08_largefile_crash.patch: fix dataloss associated with >= 4GB files
     - debian/patches/kubuntu_09_self_DHT_cpuspin.patch: Ignore DHT replies from self; reported to cause CPU pegging on certain NAT configurations
     - debian/patches/kubuntu_10_multifile_4GB.patch: Fix bug with multifile torrents containing >4GB files being truncated.
     - debian/patches/kubuntu_11_DHT_safetycheck.patch: Perform more validation on incoming DHT packets to prevent malformed DHT packets from crashing KTorrent.
     - debian/patches/kubuntu_12_saveIndexFile_exceptions.patch: Fix bug with saveIndexFile exceptions not being caught, resulting in potential crashes.

 -- John Dong <email address hidden> Sat, 28 Apr 2007 17:46:44 -0400

John Dong (jdong) wrote :
John Dong (jdong) wrote :
John Dong (jdong) wrote :

I confirm that this patch builds in a Feisty pbuilder

Martin Pitt (pitti) wrote :

Oh dear, that's a lot of patches; some are trivial and obvious, but some change a lot of logic. Can they be related to actual Malone bugs? Can this be stripped down to patches which actually affect a large number of people and are thus SRU worthy?

Also, please remove the ~proposed suffix, as per updated SRU process.

John Dong (jdong) wrote :

The single MOST important fix is " - debian/patches/kubuntu_11_DHT_safetycheck.patch: Perform more validation on incoming DHT packets to prevent malformed DHT packets from crashing KTorrent."

Which gets reported, confirmed, or duped at least once or twice per day (i.e. bug 109184). However, from other Malone crashes (basically most open crasher bugs) and upstream's forum reports and talking to upstream devs, many of these scenarios are likely to cause crashing. I've already filtered out this from upstream SVN's big stream of changesets to bugs that I have seen others complain about that are actually pretty serious.

I use and participate upstream in KTorrent on a daily basis, and I am VERY confident that these changes will not introduce any regressions.

Martin Pitt (pitti) wrote :

This still looks fishy:

+- if (ab->isFinished())
++ if (!ab || ab->isFinished())
+ {
+ ab->deleteLater();

i. e. if ab == NULL, the ->deleteLater() will still crash

kubuntu_10_multifile_4GB.patch removes a lot of comments and is a bit hard to read; this could be stripped down a bit.

The rest looks fine.

John Dong (jdong) wrote :

I updated the debdiff:

* Version number fixed
* Cleaned up kubuntu_10_multifile_4GB.patch to remove less random comments.
* Updated the auth_crash patch to guard against NULL pointers as mentioned, isolated from upstream SVN devel branch under upstream's recommendations.

Built in pbuilder, and downloaded a torrent with 700 simultaneous connections for an hour with no issues.

Martin Pitt (pitti) wrote :

This now looks good. However, please format the changelog properly before ujploading (no lines longer than 80 characters). Also, whenever possible please sort the bugs to the relevant patches.

Changed in ktorrent:
assignee: nobody → jdong
status: Unconfirmed → In Progress
John Dong (jdong) wrote :

I rebased the debdiff against our 2.1 revision from the recent security update, reformatted the changelog, and bumped the version to 2.2

testbuild still succeeds and works, could a sponsor please upload this? Thanks!

Martin Pitt (pitti) wrote :

Accepted into -proposed. Please go ahead with QA testing according to https://wiki.ubuntu.com/StableReleaseUpdates.

Changed in ktorrent:
status: In Progress → Fix Committed
John Dong (jdong) wrote :

I have verified this on i386 and amd64 and it looks correct to me.

For Testers:

The best way of testing this is to start up KTorrent, enable DHT in the options, then torrent for 2 or so hours. If KTorrent doesn't crash despite the number of DHT nodes in the status bar increasing, then the crash is fixed.

Daniel Holbach (dholbach) wrote :

Any progress on testing?

Changed in ktorrent:
status: Fix Committed → Incomplete
Daniel Holbach (dholbach) wrote :

Unsubscribing Ubuntu Sponsors for main from this bug for the time being.

Martin Pitt (pitti) wrote :

Copied to -updates, based on John's own verification and the successful tests in the other bugs mentioned in the changelog.

Changed in ktorrent:
status: Incomplete → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers

Bug attachments