shared: fix double free in unmask #5005
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Easily reproducible: 1) systemctl mask foo 2) systemctl unmask foo foo The problem here is that the *i that is put into todo[] is later freed in strv_uniq(), which is not directly visible from this patch. Somewhere further in the code, the string that *i pointed to is freed again. That happens only when multiple services with the same name/path are specified. On my system, the memory corruption revealed itself a lot later in the program, which resulted in backtraces that were pretty much misleading and useless.
Contributor
|
BTW: there is another touch hola.service
systemctl link $(pwd)/hola.service $(pwd)/hola.serviceI didn't check other install operations |
Contributor
|
While not complete yet, this looks correct, so the other double-free that @evverx found can be fixed in a separate PR. Thanks! |
evverx
reviewed
Jan 9, 2017
| return -ENOMEM; | ||
|
|
||
| todo[n_todo++] = *i; | ||
| todo[n_todo++] = strdup(*i); |
Contributor
There was a problem hiding this comment.
we should check if (!todo[n_todo-1]) return -ENOMEM
whot
pushed a commit
to whot/systemd
that referenced
this pull request
Oct 10, 2017
Easily reproducible: 1) systemctl mask foo 2) systemctl unmask foo foo The problem here is that the *i that is put into todo[] is later freed in strv_uniq(), which is not directly visible from this patch. Somewhere further in the code, the string that *i pointed to is freed again. That happens only when multiple services with the same name/path are specified. (cherry picked from commit dc7dd61) Resolves: #1409997
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Easily reproducible:
The problem here is that the *i that is put into todo[] is later freed
in strv_uniq(), which is not directly visible from this patch. Somewhere
further in the code, the string that *i pointed to is freed again. That
happens only when multiple services with the same name/path are specified.
On my system, the memory corruption revealed itself a lot later in the
program, which resulted in backtraces that were pretty much misleading
and useless.