Differential D25414

xattr: fix crash on dangling symlinks
ClosedPublic
Actions

Authored by iasensio on Nov 19 2019, 8:41 PM.

Details

Reviewers
astippich
bruns
Commits
R286:dea539c7ed34: xattr: fix crash on dangling symlinks
R286:4bb4195a6fc6: xattr: fix crash on dangling symlinks
Summary

When requesting metadata on a dangling symlink, the framestack ends up calling
k_queryAttributes() with the symlink path, where the listxattr syscall
returns size==-1 and errno==ENOENT (No such file or directory).
This case was not covered before, and provoked a segfault on QByteArray.
Full traceback on: https://bugs.kde.org/show_bug.cgi?id=414227

It might be also a good idea to always protect the function when size==-1

BUG: 414227

Test Plan
  • bin/usermetadatawritertest : added test
  • On dolphin, with panel information open, hover over a dangling symlink

Diff Detail

Repository
R286 KFileMetaData
Branch
fix_symlink
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 18973
Build 18991: arc lint + arc unit
iasensio created this revision.Nov 19 2019, 8:41 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptNov 19 2019, 8:41 PM
Restricted Application added subscribers: Baloo, kde-frameworks-devel. · View Herald Transcript
iasensio requested review of this revision.Nov 19 2019, 8:41 PM
iasensio edited the summary of this revision. (Show Details)Nov 19 2019, 8:44 PM
bruns added a subscriber: bruns.Nov 19 2019, 9:16 PM
bruns added inline comments.
src/xattr_p.h
228
if (size < 0) {
  if (errno == E2BIG) {
        return UserMetaData::Attribute::All;
  }

  return UserMetaData::Attribute::None;
}
236

The size > 0 can be removed here then.

bruns requested changes to this revision.Nov 19 2019, 9:16 PM
This revision now requires changes to proceed.Nov 19 2019, 9:16 PM
bruns added a comment.Nov 19 2019, 9:17 PM

wrap the summary at <80 characters

iasensio edited the summary of this revision. (Show Details)Nov 19 2019, 9:21 PM
bruns added a comment.Nov 19 2019, 9:21 PM

Also add a testcase covering this, autotests/usermetadatawritertest.cpp

iasensio updated this revision to Diff 70029.Nov 19 2019, 10:02 PM
iasensio marked 2 inline comments as done.
  • Protect from size < 0
  • Add test case for dangling symlink
iasensio updated this revision to Diff 70031.Nov 19 2019, 10:17 PM
  • Use temporal symlink
iasensio edited the test plan for this revision. (Show Details)Nov 19 2019, 10:19 PM
bruns added inline comments.Nov 19 2019, 10:54 PM
autotests/usermetadatawritertest.cpp
44

You can just use the static overload and provide an invalid target name.
https://doc.qt.io/qt-5/qfile.html#link-1

iasensio updated this revision to Diff 70035.Nov 19 2019, 11:18 PM
iasensio marked an inline comment as done.
  • Simplify symlink creation
iasensio added inline comments.Nov 19 2019, 11:18 PM
autotests/usermetadatawritertest.cpp
44

You are right. I tried that with an empty string and it didn't work, but it does with a non-existent filename.

bruns accepted this revision.Dec 12 2019, 8:17 PM
This revision is now accepted and ready to land.Dec 12 2019, 8:17 PM
Closed by commit R286:4bb4195a6fc6: xattr: fix crash on dangling symlinks (authored by iasensio). · Explain WhyDec 12 2019, 8:50 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 70035

View Options

autotests/usermetadatawritertest.h

Loading...
View Options

autotests/usermetadatawritertest.cpp

Loading...
View Options

src/xattr_p.h

Loading...