Skip to content

Prevent segfault #1610

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Oct 1, 2020
Merged

Prevent segfault #1610

merged 4 commits into from
Oct 1, 2020

Conversation

@jan-cerny
Copy link
Member

@jan-cerny jan-cerny commented Sep 30, 2020

Prevent segfault if there are circular dependencies between OVAL
definitions. This segfault happened when an OVAL definition references
itself in the extend_definition element. It can also happen when the
referenced OVAL definition references back to the parent OVAL definition
and therefore there is a circular dependency between OVAL definitions.
See the test OVALs for examples of such definitions. The circle wasn't
detected and it caused infinite recursion which caused a segfault.

The fix uses a list which stores the IDs of already evaluated
definitions and it checks if a definition isn't evaluated for the second
time.

Fixes: rhbz#1812476

@jan-cerny
Prevent segfault
1ce40c8 
Prevent segfault if there are circular dependencies between OVAL
definitions.  This segfault happened when an OVAL definition references
itself in the extend_definition element. It can also happen when the
referenced OVAL definition references back to the parent OVAL definition
and therefore  there is a circular dependency between OVAL definitions.
See the test OVALs for examples of such definitions. The circle wasn't
detected and it caused infinite recursion which caused a segfault.

The fix uses a list which stores the IDs of already evaluated
definitions and it checks if a definition isn't evaluated for the second
time.

Fixes: rhbz#1812476
@jan-cerny jan-cerny added this to the 1.3.4 milestone Sep 30, 2020
@evgenyz
Copy link
Contributor

evgenyz commented Oct 1, 2020

@jan-cerny It fails to pass its own test on Ubuntu, can you please check it out?

@evgenyz
Copy link
Contributor

evgenyz commented Oct 1, 2020 •
edited
Loading

Okay, we have to add libdbus-1-dev to dependencies in our Ubuntu CI. But it still fails. We actually have it already, it was just my VM.

Because, for some reason, we have

Definition oval:ssg-service_chronyd_enabled:def:1: false
Evaluation done.

insetad of

Definition oval:ssg-service_chronyd_enabled:def:1: not evaluated
...

as the test expects.

@evgenyz
Copy link
Contributor

evgenyz commented Oct 1, 2020

Also, can we maybe test it with a less exotic probe/test?

@jan-cerny
Copy link
Member Author

jan-cerny commented Oct 1, 2020

OK, I'll look into this. I think any probe can be used, it should be reproducible regardless of the used probe because the problem was on the OVAL level. The systemd probe was used in the original reproducer in the BZ so it was created from that.

@jan-cerny
Copy link
Member Author

jan-cerny commented Oct 1, 2020

I tried to change the test to make it more general and to use the OVAL file test insted of OVAL systemdunit test in the test files. Let's take a look on the new CI run now.

evgenyz
evgenyz reviewed Oct 1, 2020
View reviewed changes
evgenyz
evgenyz reviewed Oct 1, 2020
View reviewed changes
@evgenyz
Copy link
Contributor

evgenyz commented Oct 1, 2020

Otherwise LGTM.

@jan-cerny
Copy link
Member Author

jan-cerny commented Oct 1, 2020

I have removed the superfluous braces.

@jan-cerny
Copy link
Member Author

jan-cerny commented Oct 1, 2020

@openscap-ci test this please

@evgenyz evgenyz merged commit 85348e3 into OpenSCAP:maint-1.3 Oct 1, 2020
jan-cerny added a commit to jan-cerny/openscap that referenced this pull request Oct 5, 2020
@jan-cerny
Plug a memory leak
d5518f3 
Addressing:

8 bytes in 1 blocks are indirectly lost in loss record 7 of 235
   at 0x483A809: malloc (vg_replace_malloc.c:307)
   by 0x48F15CA: oval_collection_new (oval_collection.c:64)
   by 0x48F4FCC: oval_result_criteria_node_new (oval_resultCriteriaNode.c:106)
   by 0x48F5580: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:249)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F7F41: oval_result_system_get_new_definition_with_check (oval_resultSystem.c:217)
   by 0x48F5686: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:279)
   by 0x48F55BD: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:260)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F8794: oval_result_system_prepare_definition (oval_resultSystem.c:395)
   by 0x48F86A6: oval_result_system_eval_definition (oval_resultSystem.c:369)
   by 0x48C23FD: oval_agent_eval_definition (oval_agent.c:181)

8 bytes in 1 blocks are definitely lost in loss record 8 of 235
   at 0x483A809: malloc (vg_replace_malloc.c:307)
   by 0x48F1799: oval_collection_iterator (oval_collection.c:120)
   by 0x48CCE4C: oval_criteria_node_get_subnodes (oval_criteriaNode.c:161)
   by 0x48F5590: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:255)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F7F41: oval_result_system_get_new_definition_with_check (oval_resultSystem.c:217)
   by 0x48F5686: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:279)
   by 0x48F55BD: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:260)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F8794: oval_result_system_prepare_definition (oval_resultSystem.c:395)
   by 0x48F86A6: oval_result_system_eval_definition (oval_resultSystem.c:369)
   by 0x48C23FD: oval_agent_eval_definition (oval_agent.c:181)

48 (40 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 125 of 235
   at 0x483A809: malloc (vg_replace_malloc.c:307)
   by 0x48F4F50: oval_result_criteria_node_new (oval_resultCriteriaNode.c:98)
   by 0x48F5580: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:249)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F7F41: oval_result_system_get_new_definition_with_check (oval_resultSystem.c:217)
   by 0x48F5686: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:279)
   by 0x48F55BD: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:260)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F8794: oval_result_system_prepare_definition (oval_resultSystem.c:395)
   by 0x48F86A6: oval_result_system_eval_definition (oval_resultSystem.c:369)
   by 0x48C23FD: oval_agent_eval_definition (oval_agent.c:181)
   by 0x48C2671: oval_agent_eval_system (oval_agent.c:286)

This leak has been created by OpenSCAP#1610.
@jan-cerny jan-cerny mentioned this pull request Oct 5, 2020
Merged
tom-seewald pushed a commit to tom-seewald/openscap that referenced this pull request Nov 2, 2020
@jan-cerny @tom-seewald
Plug a memory leak
81c86ba 
Addressing:

8 bytes in 1 blocks are indirectly lost in loss record 7 of 235
   at 0x483A809: malloc (vg_replace_malloc.c:307)
   by 0x48F15CA: oval_collection_new (oval_collection.c:64)
   by 0x48F4FCC: oval_result_criteria_node_new (oval_resultCriteriaNode.c:106)
   by 0x48F5580: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:249)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F7F41: oval_result_system_get_new_definition_with_check (oval_resultSystem.c:217)
   by 0x48F5686: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:279)
   by 0x48F55BD: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:260)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F8794: oval_result_system_prepare_definition (oval_resultSystem.c:395)
   by 0x48F86A6: oval_result_system_eval_definition (oval_resultSystem.c:369)
   by 0x48C23FD: oval_agent_eval_definition (oval_agent.c:181)

8 bytes in 1 blocks are definitely lost in loss record 8 of 235
   at 0x483A809: malloc (vg_replace_malloc.c:307)
   by 0x48F1799: oval_collection_iterator (oval_collection.c:120)
   by 0x48CCE4C: oval_criteria_node_get_subnodes (oval_criteriaNode.c:161)
   by 0x48F5590: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:255)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F7F41: oval_result_system_get_new_definition_with_check (oval_resultSystem.c:217)
   by 0x48F5686: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:279)
   by 0x48F55BD: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:260)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F8794: oval_result_system_prepare_definition (oval_resultSystem.c:395)
   by 0x48F86A6: oval_result_system_eval_definition (oval_resultSystem.c:369)
   by 0x48C23FD: oval_agent_eval_definition (oval_agent.c:181)

48 (40 direct, 8 indirect) bytes in 1 blocks are definitely lost in loss record 125 of 235
   at 0x483A809: malloc (vg_replace_malloc.c:307)
   by 0x48F4F50: oval_result_criteria_node_new (oval_resultCriteriaNode.c:98)
   by 0x48F5580: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:249)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F7F41: oval_result_system_get_new_definition_with_check (oval_resultSystem.c:217)
   by 0x48F5686: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:279)
   by 0x48F55BD: make_result_criteria_node_from_oval_criteria_node (oval_resultCriteriaNode.c:260)
   by 0x48F6B51: make_result_definition_from_oval_definition (oval_resultDefinition.c:130)
   by 0x48F8794: oval_result_system_prepare_definition (oval_resultSystem.c:395)
   by 0x48F86A6: oval_result_system_eval_definition (oval_resultSystem.c:369)
   by 0x48C23FD: oval_agent_eval_definition (oval_agent.c:181)
   by 0x48C2671: oval_agent_eval_system (oval_agent.c:286)

This leak has been created by OpenSCAP#1610.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@evgenyz evgenyz evgenyz left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.3.4

Development

Successfully merging this pull request may close these issues.

2 participants

@jan-cerny @evgenyz