Skip to content

GitLab

  • Projects
  • Groups
  • Snippets
  • Help
    • Loading...
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
T
tails
  • Project overview
    • Project overview
    • Details
    • Activity
    • Releases
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
  • Issues 944
    • Issues 944
    • List
    • Boards
    • Labels
    • Service Desk
    • Milestones
  • Merge Requests 13
    • Merge Requests 13
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
  • Operations
    • Operations
    • Incidents
    • Environments
  • Analytics
    • Analytics
    • CI / CD
    • Repository
    • Value Stream
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • tails
  • tails
  • Issues
  • #16034

Closed
Open
Opened Oct 08, 2018 by segfault@segfaultMaintainer

ASP: Fix race condition when writing to packages file

Originally created by @segfault on #16034 (Redmine)

From #15838 (closed):
segfault wrote:

* there is a race condition between multiple calls of add_additional_packages and/or remove_additional_packages: if the config file is written between when the packages are read and when the packages are written, this change will be overwritten by the subsequent write. You could fix this by acquiring a file lock before reading the file, for example with fcntl.flock(f, fcntl.LOCK_EX), and only releasing the lock once the file was written (example: https://gitlab.com/segfault3/onionkit/blob/master/onionkit/util.py#L21).
This way you could also get rid of the python3-atomicwrites package

As far as I understand, such lock are released on close, so I don’t see how it would actually lock the config file (see https://linux.die.net/man/2/flock).
The way the code works now, the file is opened for reading, then closed, then opened again for writing, closed, and chmod.
We don’t even have any guarantee we actually write the the same path we read, because get_packages_list_path is called again each time.
Am I wrong?
Do you think we should ensure the file is only opened once?

Yes, that’s what I meant and what the flock can be used for.

Feature Branch: bugfix/16034-asp-fix-race-condition

Related issues

  • Related to #15838 (closed)
  • Is duplicate of #15982 (closed)
Edited May 15, 2020 by segfault
To upload designs, you'll need to enable LFS and have admin enable hashed storage. More information
Assignee
Assign to
None
Milestone
None
Assign milestone
Time tracking
None
Due date
None
Reference: tails/tails#16034