Differential D11188

Fix use after free in RadialMap::Widget::invalidate()
ClosedPublic
Actions

Authored by janus on Mar 9 2018, 6:46 PM.

Details

Reviewers
sitter
sandsmark
Commits
R352:4cc7c44589f0: Fix use after free in RadialMap::Widget::invalidate()
Summary

Emit aboutToEmptyCache to invoke RadialMap::Widget::invalidate(),
so folder can be safely deleted.

If "Scan across system boundaries" isn't set and there is mountpoint B
contained in another mountpoint A, user may trigger segfault if scan of
A and then B is requested. It is caused by call of MainWindow class to
m_map->invalidate() method, which is invoked after m_map's m_tree member
is deleted during ScanManager::start method.

This fix causes that RadialMap::Widget::invalidate() is called twice in
fact, however second call is fine, because "invalidate" is guarded by
"isValid" method.

BUG: 389119

Diff Detail

Repository
R352 Filelight
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
janus created this revision.Mar 9 2018, 6:46 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald TranscriptMar 9 2018, 6:46 PM
janus requested review of this revision.Mar 9 2018, 6:46 PM
janus added a comment.Jul 10 2018, 11:15 AM

Hi guys, any updates on review? Should I include test plan?

sitter added a comment.Jul 10 2018, 8:03 PM

I think a test plan would be lovely and/or an actually complete backtrace as the bugreport's lacks all relevant filelight frames it seems. I am not sure I understand how the crash happens exactly.

sandsmark added a comment.Jul 16 2018, 11:36 AM

it looks correct to me (i. e. we should never delete a Folder without invalidating), but it would be nice with at least a shortened backtrace because I'm still not 100% sure how it ends up in that state.

janus added a comment.Feb 8 2019, 8:06 PM

I'm sorry, totally forgot about this. I'll look into it next week.

janus added a comment.Feb 25 2019, 3:39 PM

I've been testing it since a few days, however I failed to provide a simple, reproducible test case :< . I'll try to dig in the source code.

sandsmark added a comment.Mar 2 2019, 7:43 PM

FWIW, looking at the code briefly, I think this patch does the right thing if we end up in that bad "something went wrong"-state. But there's probably a second bug somewhere that leads us to that state, so a bit hard to reproduce, I guess.

But I don't see any reason not to merge this, so unless Harald objects, feel free to merge it.

sitter accepted this revision.Mar 4 2019, 1:56 PM
This revision is now accepted and ready to land.Mar 4 2019, 1:56 PM
Closed by commit R352:4cc7c44589f0: Fix use after free in RadialMap::Widget::invalidate() (authored by janus, committed by sitter). · Explain WhyMar 4 2019, 4:12 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/scan.cpp (1 line)

Diff 53134

View Options

src/scan.cpp

Loading...