Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

device_tree: Use PluginV1 #1028

Merged
merged 2 commits into from
Mar 6, 2024
Merged

device_tree: Use PluginV1 #1028

merged 2 commits into from
Mar 6, 2024

Conversation

maringuu
Copy link
Collaborator

@maringuu maringuu commented May 4, 2023

Simelar to #1022

The alembic commit is probably not needed and superseded by #1027

@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from f707953 to 2430121 Compare June 15, 2023 12:51
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 2430121 to 705e3a5 Compare July 3, 2023 12:26
@maringuu maringuu changed the base branch from master to analysis-plugin-v1 July 3, 2023 12:26
@maringuu maringuu changed the title [DONT MERGE] device_tree: Use PluginV1 device_tree: Use PluginV1 Jul 3, 2023
@maringuu
Copy link
Collaborator Author

maringuu commented Jul 3, 2023

This is ready!
One question I ask myself is how we handle the old plugin results.
I really much dislike that we have code in the template that handles these version incompabilities.
I would suggest that we implement some logic in the frontend that compares the plugin version it is told to render to the version it received via redis.
This way the frontend can detect incompatibilities and just render a message that you should rerun your analysis.

@maringuu maringuu marked this pull request as ready for review July 3, 2023 12:34
@maringuu maringuu requested a review from jstucke July 3, 2023 12:34
@maringuu maringuu force-pushed the analysis-plugin-v1 branch 3 times, most recently from 86df387 to 5081618 Compare July 10, 2023 13:28
@maringuu maringuu force-pushed the analysis-plugin-v1 branch 2 times, most recently from a317446 to 2dd7b7c Compare July 13, 2023 12:54
@maringuu maringuu mentioned this pull request Jul 13, 2023
31 tasks
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 705e3a5 to d084823 Compare July 13, 2023 13:36
@maringuu
Copy link
Collaborator Author

@maringuu maringuu force-pushed the analysis-plugin-v1 branch 5 times, most recently from 8d96aee to edf4c71 Compare July 17, 2023 09:59
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch 2 times, most recently from 9d25aa7 to cbc00d3 Compare July 17, 2023 10:03
@maringuu maringuu force-pushed the analysis-plugin-v1 branch from edf4c71 to 4067c84 Compare July 17, 2023 10:38
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from cbc00d3 to 1476694 Compare July 17, 2023 10:40
@maringuu
Copy link
Collaborator Author

maringuu commented Jul 17, 2023

@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch 3 times, most recently from 2e5eff6 to 7f1b906 Compare July 24, 2023 08:42
@maringuu maringuu force-pushed the analysis-plugin-v1 branch from dbde681 to 503a753 Compare July 31, 2023 14:13
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 7f1b906 to 4bef560 Compare July 31, 2023 15:55
Base automatically changed from analysis-plugin-v1 to master August 3, 2023 18:56
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 4bef560 to 50eba56 Compare August 3, 2023 18:59
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 50eba56 to d41c009 Compare August 28, 2023 07:59
@jstucke
Copy link
Collaborator

jstucke commented Aug 28, 2023

I tried running it and there seem to be problems with the "architecture_detection" plugin (which depends on the device_tree plugin) not expecting None as result:

Traceback (most recent call last):
  File "/usr/lib/python3.11/multiprocessing/process.py", line 314, in _bootstrap
    self.run()
  File "src/helperFunctions/process.py", line 91, in run
    raise exception
  File "src/helperFunctions/process.py", line 86, in run
    Process.run(self)
  File "/usr/lib/python3.11/multiprocessing/process.py", line 108, in run
    self._target(*self._args, **self._kwargs)
  File "src/analysis/PluginBase.py", line 196, in process_next_object
    finished_task = self.analyze_file(task)
                    ^^^^^^^^^^^^^^^^^^^^^^^
  File "src/analysis/PluginBase.py", line 159, in analyze_file
    fo = self.process_object(file_object)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "src/plugins/analysis/architecture_detection/code/architecture_detection.py", line 37, in process_object
    arch_dict = construct_result(file_object, self._fs_organizer)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "src/plugins/analysis/architecture_detection/code/architecture_detection.py", line 50, in construct_result
    result.update(dt.construct_result(file_object))
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "src/plugins/analysis/architecture_detection/internal/dt.py", line 76, in construct_result
    for dt_dict in file_object.processed_analysis['device_tree'].get('result', {}).get('device_trees', []):
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'get'

@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 7747ebe to 1b1b284 Compare September 18, 2023 07:52
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 1b1b284 to 02638ec Compare October 12, 2023 10:09
@maringuu
Copy link
Collaborator Author

I tried running it and there seem to be problems with the "architecture_detection" plugin (which depends on the device_tree plugin) not expecting None as result:

Fixed.

@maringuu maringuu requested a review from jstucke October 12, 2023 10:28
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.

Did you try to run it? I always got an Internal Server Error when trying it out:

  File "src/web_interface/filter.py", line 439, in hide_dts_binary_data
    device_tree = re.sub(r'\[[0-9a-f ]{32,}]', '(BINARY DATA ...)', device_tree)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/re/__init__.py", line 185, in sub
    return _compile(pattern, flags).sub(repl, string, count)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: expected string or bytes-like object, got 'Undefined'

src/plugins/analysis/device_tree/code/device_tree.py Outdated Show resolved Hide resolved
src/plugins/analysis/device_tree/internal/schema.py Outdated Show resolved Hide resolved
src/plugins/analysis/device_tree/internal/schema.py Outdated Show resolved Hide resolved
src/plugins/analysis/device_tree/internal/schema.py Outdated Show resolved Hide resolved
@maringuu
Copy link
Collaborator Author

maringuu commented Nov 7, 2023

Did you try to run it? I always got an Internal Server Error when trying it out:

I just ran the tests and totally forgot about the frontend.

@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 02638ec to 3df2e7d Compare November 7, 2023 10:01
@maringuu
Copy link
Collaborator Author

maringuu commented Nov 7, 2023

Everything should work now.

@maringuu maringuu requested a review from jstucke November 7, 2023 10:02
@jstucke
Copy link
Collaborator

jstucke commented Jan 18, 2024

I still get the error message when looking at old analysis results:

  File "<template>", line 6, in template
  File "src/web_interface/filter.py", line 448, in hide_dts_binary_data
    device_tree = re.sub(r'\[[0-9a-f ]{32,}]', '(BINARY DATA ...)', device_tree)
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/re/__init__.py", line 185, in sub
    return _compile(pattern, flags).sub(repl, string, count)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Normally this should be intercepted by the version check, but since the version changed from 1.0.1 to 1.1.0, it is treated as compatible. So a "fix" would be to set the version to 2.0.0 (which would be kinda appropriate since there are breaking changes).

Apart from that everything seems to work fine.

@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 3df2e7d to aa772bd Compare February 22, 2024 15:30
@maringuu
Copy link
Collaborator Author

Bumped the version to 2.0.0. Sill the above question is unsolved.

maringuu added 2 commits March 6, 2024 10:29
Major Version bumped because the schema changed.
Defining the schema allowed us to clean up much of the logic.
While there is some arguable code left this is a step in the right
direction.
Also required a change in the architecture_detection plugin to
accomodate that the result can be None.
This is not a standart DeviceTree field, but aparently it
appreas in some firmwares.
So we just cargo cult the thing :)
@maringuu maringuu force-pushed the analysis-plugin-v1-device-tree branch from 25d5379 to 4479496 Compare March 6, 2024 09:30
@maringuu maringuu merged commit 873c1ad into master Mar 6, 2024
6 of 10 checks passed
@maringuu maringuu deleted the analysis-plugin-v1-device-tree branch March 6, 2024 09:30
maringuu added a commit to maringuu/FACT_core that referenced this pull request Mar 6, 2024
An oversight in rebasing fkie-cad#1028
jstucke pushed a commit that referenced this pull request Mar 7, 2024
An oversight in rebasing #1028
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants