Skip to content

Fix double free after comps_objmrtree_copy_shallow() - #167

Merged
m-blaha merged 1 commit into
rpm-software-management:masterfrom
prownd:fix-double-free-after-comps_objmrtree_copy_shallow-s01
Oct 27, 2026
Merged

m-blaha merged 1 commit into
rpm-software-management:masterfrom
prownd:fix-double-free-after-comps_objmrtree_copy_shallow-s01

Conversation

@prownd

@prownd prownd commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

comps_objmrtree_copy_shallow() copied the value lists of the first level, but put the very same lists (without taking a reference) into all deeper nodes, so destroying the copy freed lists still used by the original. MDict.copy() followed by deleting the copy crashed. Copy the lists on all levels.

Comment thread libcomps/src/comps_objmradix.c Outdated
NULL);
new_data_list = ((COMPS_ObjMRTreeData*)it->data)->data;
new_data_list = (COMPS_ObjList*)
COMPS_OBJECT_COPY(((COMPS_ObjMRTreeData*)it->data)->data);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But this COMPS_OBJECT_COPY will do a deep copy, contradicting the function "*_copy_shallow" name.

Shouldn't we use the COMPS_OBJECT_INCREF as in similar ObjRTree code?

if (((COMPS_ObjRTreeData*)it->data)->data != NULL)
new_data = COMPS_OBJECT_INCREF(((COMPS_ObjRTreeData*)it->data)->data);
else
new_data = NULL;

comps_objmrtree_copy_shallow() put the value lists of all nodes below
the first level into the copy without taking a reference, so destroying
the copy freed lists still used by the original. MDict.copy() followed
by deleting the copy crashed. Take a reference to the lists, as
comps_objrtree_copy_shallow() does.

Signed-off-by: hanjinpeng <hanjinpeng@kylinos.cn>
@prownd
prownd force-pushed the fix-double-free-after-comps_objmrtree_copy_shallow-s01 branch from 5bf616a to 5e47d2b Compare October 27, 2026 12:42
@prownd

prownd commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Thanks, changed to COMPS_OBJECT_INCREF as you suggested.

@m-blaha m-blaha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@m-blaha
m-blaha merged commit 603c49c into rpm-software-management:master Oct 27, 2026
3 checks passed
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