#2480 sdap_extend_map_with_list() might leak memory if used the 4th and 6th argument being the same
Closed: Fixed 2 years ago by atikhonov. Opened 6 years ago by sbose.

sdap_extend_map_with_list() increases a given map by creating a new one. If the new one is assigned to same pointer as the old one, the memory occupied by the old one might not be accessible anymore and leaked.


Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.14 beta

Fields changed

rhbz: => 0

Fields changed

priority: major => minor
sensitive: => 0

Nice to fix, but not required

milestone: SSSD 1.14 beta => SSSD 1.14 backlog

Fields changed

keywords: => easyfix

Since the 1.14 branch is transitioning into maintenance mode and new functionality is being developed in master which will become 1.15 eventually, I'm mass-moving tickets from the 1.14 backlog milestone to the "Future releases" milestone.

milestone: SSSD 1.14 backlog => SSSD Future releases (no date set yet)

Metadata Update from @sbose:
- Issue set to the milestone: SSSD Future releases (no date set yet)

3 years ago

Metadata Update from @jhrozek:
- Custom field design_review reset (from 0)
- Custom field mark reset (from 0)
- Custom field patch reset (from 0)
- Custom field review reset (from 0)
- Custom field sensitive reset (from 0)
- Custom field testsupdated reset (from 0)
- Issue close_status updated to: None
- Issue tagged with: easyfix

3 years ago

sdap_extend_map_with_list() increases a given map by creating a new one. If the new one is assigned to same pointer as the old one, the memory occupied by the old one might not be accessible anymore and leaked.

I know ticket is quite old but still... Could you please explain it?

As I can see it: sdap_extend_map_with_list merely wraps sdap_extend_map which in turn makes use of talloc_realloc.

talloc_realloc documentation is quite sparse but I expect it to behave consistently with libc realloc. Specifically, man realloc: "If the area pointed to was moved, a free(ptr) is done."
If my expectation is correct then in successful case memory pointed to by 4th argument is freed.
If there is a fail with talloc_realloc or some other functions, then no new memory allocated and 4th arguments is not overwritten.

There are still some issues with code involved (lack of const specifiers; inconsistent behavior of sdap_extend_map_with_list and sdap_extend_map in regard to _map argument in case of auxiliary functions fail, unnecessary mem copy...)

But from a quick glance I can't see described issue.

Metadata Update from @atikhonov:
- Custom field design_review reset (from false)
- Custom field mark reset (from false)
- Custom field patch reset (from false)
- Custom field review reset (from false)
- Custom field sensitive reset (from false)
- Custom field testsupdated reset (from false)

2 years ago

Hi @atikhonov,

I think I already fixed the concern I had here with commit 08bf6b4 some time ago but forgot to close this ticket.

But feel free to use this ticket to fix the other issues you found.

bye,
Sumit

Metadata Update from @sbose:
- Custom field design_review reset (from false)
- Custom field mark reset (from false)
- Custom field patch reset (from false)
- Custom field review reset (from false)
- Custom field sensitive reset (from false)
- Custom field testsupdated reset (from false)

2 years ago

I think I already fixed the concern I had here with commit 08bf6b4 some time ago but forgot to close this ticket.
But feel free to use this ticket to fix the other issues you found.

Other issues are not directly related with this bug summary.
So closing ticket as fixed.

Metadata Update from @atikhonov:
- Custom field design_review reset (from false)
- Custom field mark reset (from false)
- Custom field patch reset (from false)
- Custom field review reset (from false)
- Custom field sensitive reset (from false)
- Custom field testsupdated reset (from false)
- Issue close_status updated to: Fixed
- Issue status updated to: Closed (was: Open)

2 years ago

SSSD is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in SSSD's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/SSSD/sssd/issues/3522

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Login to comment on this ticket.

Metadata