这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@dorpvom
Copy link
Member

@dorpvom dorpvom commented Oct 30, 2018

Problem of race condition has appeared since binary deletion was activated recently.
Whenever a firmware is deleted, issuing a cascade of file deletions, a new analysis can overlap with the files. Then a file can be extracted during analysis, deleted shortly after due to the firmware deletion, and analyzed later on at which point the file has been deleted.
This happens basically every time a re-do anlysis is issued, but can happen in other scenarios as well.

A locking of extracted files should prevent that from happening. That is:

  1. Unpacker finds file
  2. file is locked
  3. file is stored on fs
  4. file is analyzed
  5. analysis is stored in database
  6. lock is released

As of now, if a deletion request is issued in between steps 3 and 5, the file is remove as no database link exists to it. Once step 5 is processed, a new link in the database exists and the lock can be safely removed. As long as the lock is in place, any deletion attempt will be discarded.

@dorpvom dorpvom added the bug label Oct 30, 2018
@dorpvom dorpvom self-assigned this Oct 30, 2018
@dorpvom dorpvom requested a review from jstucke October 30, 2018 13:36
@codecov-io
Copy link

codecov-io commented Oct 30, 2018

Codecov Report

Merging #171 into master will increase coverage by 0.01%.
The diff coverage is 97.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #171      +/-   ##
==========================================
+ Coverage   95.73%   95.75%   +0.01%     
==========================================
  Files         311      312       +1     
  Lines       15498    15586      +88     
==========================================
+ Hits        14837    14924      +87     
- Misses        661      662       +1
Impacted Files Coverage Δ
src/test/unit/unpacker/test_unpacker.py 100% <100%> (ø) ⬆️
...t/integration/scheduler/test_unpack_and_analyse.py 100% <100%> (ø) ⬆️
src/test/integration/storage/test_db_interface.py 97.99% <100%> (+0.13%) ⬆️
src/test/integration/common.py 93.54% <100%> (+0.44%) ⬆️
src/intercom/back_end_binding.py 100% <100%> (+0.76%) ⬆️
src/test/unit/scheduler/test_unpack.py 100% <100%> (ø) ⬆️
src/storage/db_interface_common.py 95.43% <100%> (+0.19%) ⬆️
src/scheduler/Unpacking.py 86.13% <100%> (+0.72%) ⬆️
src/test/integration/scheduler/test_unpack_only.py 100% <100%> (ø) ⬆️
.../integration/intercom/test_intercom_delete_file.py 100% <100%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3c53914...42181e9. Read the comment docs.

Copy link
Collaborator

@jstucke jstucke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the database with the locks should be dropped at startup to prevent undeletable artifacts of cancelled or crashed analyses

return uid in self.locks

def release_unpacking_lock(self, uid):
self.locks.remove(uid)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

function is unused


def test_file_is_locked(self):
assert not self.unpacker.db_interface.check_unpacking_lock(self.test_fo.uid)
file_pathes = ['{}/get_files_test/testfile1'.format(get_test_data_dir())]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

@fkie-cad fkie-cad deleted a comment from jstucke Oct 31, 2018
@jstucke jstucke merged commit 2260506 into master Oct 31, 2018
@dorpvom dorpvom deleted the locking-unpacked-files branch November 21, 2018 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants