Bug 357933 - Kleopatra 15.12.1 (2.2): load certifications (pgp) - segfault after infinite recursion
Summary: Kleopatra 15.12.1 (2.2): load certifications (pgp) - segfault after infinite ...
Status: RESOLVED FIXED
Alias: None
Product: kleopatra
Classification: Applications
Component: general (show other bugs)
Version: 2.2.0
Platform: Gentoo Packages Linux
: NOR normal
Target Milestone: ---
Assignee: Andre Heinecke
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-13 16:49 UTC by Till Schäfer
Modified: 2016-03-20 15:21 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 16.04
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Till Schäfer 2016-01-13 16:49:00 UTC
Kleopatra segfaults when pressing the button "Load Certification" for pgp keys

Steps to Reproduce:
1. Double clock an OpenPGP key 
2. Go to the tab: "User IDs & Certifications"
3. Press button "Load Certification (may take a while)"

It seem that there is an infinite recursion (full trace is attached): 

Core was generated by `kleopatra'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007ffa2a0647bb in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20118, 
    recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=afterIsUninitialized@entry=true)
    at itemviews/qtreeview.cpp:3313
[Current thread is 1 (Thread 0x7ffa18362840 (LWP 20915))]
#0  0x00007ffa2a0647bb in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20118, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3313
#1  0x00007ffa2a064eb8 in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20117, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=<optimized out>, afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3374
#2  0x00007ffa2a064eb8 in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20116, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=<optimized out>, afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3374
#3  0x00007ffa2a064eb8 in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20115, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=<optimized out>, afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3374
#4  0x00007ffa2a064eb8 in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20114, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=<optimized out>, afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3374
#5  0x00007ffa2a064eb8 in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20113, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=<optimized out>, afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3374
#6  0x00007ffa2a064eb8 in QTreeViewPrivate::layout (this=this@entry=0x1ffff30, i=i@entry=20112, recursiveExpanding=recursiveExpanding@entry=true, afterIsUninitialized=<optimized out>, afterIsUninitialized@entry=true) at itemviews/qtreeview.cpp:3374


Reproducible: Always
Comment 1 Andre Heinecke 2016-01-13 17:07:39 UTC
Thanks for reporting this. I've already seen it and it's on my Todo but there was no report about this. This is a regression from the Qt5 / KF5 port but I think the bug is that the underlying model in kleo is not correct.
Comment 2 Till Schäfer 2016-01-13 17:12:41 UTC
I will attach the stack trace as soon as this is ready. it takes a while to print a gdb stacktrace with 20k lines :-)
Comment 3 Andre Heinecke 2016-01-13 17:31:33 UTC
There is no need for that. The info you have posted is already enough and the problem is clear and reproducible.

The problem is likely that there is a cycle in the model which qt wants to layout as a tree.

I should probably just try to fix it instead of talking about it ;-)
Comment 4 Andre Heinecke 2016-02-19 14:25:05 UTC
Git commit 35bbcb687537628e0c7facc83ca2c175e3d22d3b by Andre Heinecke.
Committed on 19/02/2016 at 14:22.
Pushed by aheinecke into branch 'master'.

Refactor UserID Certificate details

The old code was obscurely optimized to avoid copying data
for the useridmodel. The userID model is fairly small as it
only works with the Signatures of the UIDs of a single key.
In most cases this is not much data. Keys with more then
a thousand signatures are rare and even those modern Systems
handle easily.

Reflecting this the option to specifically load certifications
is gone. Even for keys with lots of signatures liek
0xE3EDFAE3 or 0xD21739E9 this feels like instant. So no need
to hassle a user that explicitly selects UserID and Certification
details with yet another click to load them.

This also changes the UserIDListModel API to a Qt API.
REVIEW: 127059

M  +14   -32   kleopatra/dialogs/certificatedetailsdialog.cpp
M  +0    -1    kleopatra/dialogs/certificatedetailsdialog.h
M  +0    -10   kleopatra/dialogs/certificatedetailsdialog.ui
M  +185  -239  kleopatra/models/useridlistmodel.cpp
M  +10   -32   kleopatra/models/useridlistmodel.h

http://commits.kde.org/kdepim/35bbcb687537628e0c7facc83ca2c175e3d22d3b
Comment 5 Andre Heinecke 2016-02-19 14:27:43 UTC
I've reworked the model for this so that it is more usable in the future. In the long term I have in mind that I also want to show certifications better in the treeview. Now at least it does not crash anymore.

I've also changed that you no longer have to click "load certifications" as I this is very quick in my experience even for extremely large keys. And saving a click if a user already navigated into the details is good.
Comment 6 Till Schäfer 2016-02-19 15:21:29 UTC
neat, thx
Comment 7 marvin24 2016-03-20 15:21:41 UTC
is a backport to 15.12 branch possible?