Skip to content
GitLab
Projects Groups Topics Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in
  • KAlarm KAlarm
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributor statistics
    • Graph
    • Compare revisions
  • Issues 0
    • Issues 0
    • List
    • Boards
    • Service Desk
    • Milestones
  • Bugzilla
    • Bugzilla
  • Merge requests 1
    • Merge requests 1
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Artifacts
    • Schedules
  • Deployments
    • Deployments
    • Releases
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Repository
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • PIMPIM
  • KAlarmKAlarm
  • Merge requests
  • !15

Fix crash in FileResourceConfigManager when doing removeResource

  • Review changes

  • Download
  • Patches
  • Plain diff
Closed Jiří Paleček requested to merge jpalecek/kalarm:master into master Apr 05, 2022
  • Overview 7
  • Commits 2
  • Pipelines 2
  • Changes 5

Hello

this merge request seeks to fix a crash that I encountered when deleting an old calendar file from KAlarm. Here is the backtrace and the result of valgrind pertaining to this crash.

As you can see, it is a use after free when running ResourceSelector::removeResource(). It deletes an instance of FileResourceSettings and then uses it.

The problematic code is here:

                    Resource resource(createResource(settings));
                    manager->mResources[settings->id()] = ResourceData(resource, settings);

where the first call creates the resource from settings.data() in FileResourceManage::createResource. This Resource is then stored in the global Resources instance, and holds a pointer to settings. Another pointer to settings is held in manager->mResources. But only one is managed by a QSharedPointer which means that on removal, if the first one to go is the managed one, the second one will be dangling and (in this case) used after free. This merge request fixes it by using the shared pointer throughout.

As an aside, I looked at other suspicious usages of data() and found only one. The second commit removes that as well in favor of normal shared pointer usage.

Assignee
Assign to
Reviewers
Request review from
Time tracking
Source branch: master