fix(pkg/rootless): avoid memleak during init() contructor #25049
Conversation
|
Mmmh i am asked to add a new test, but well i am a bit unsure how to test this one :) |
661a6ce to
114dfb3
Compare
You mean from |
Yes I do not see why this is there at all but maybe there is something about c I am not understanding. Properly best to wait for @giuseppe to comment. |
pkg/rootless/rootless_linux.c
Outdated
| { | ||
| can_shortcut = can_use_shortcut(argv); | ||
| // It was freed by can_use_shortcut() call |
There was a problem hiding this comment.
can we drop the autocleanup from can_use_shortcut so we don't have to deal with the special case here?
There was a problem hiding this comment.
Pushed the changes (i added the coauthor, hope it is fine)
|
sorry, I added a comment but forgot to send the review. Fixed now |
no prob, it happens all the time 😆 |
| @@ -496,6 +494,8 @@ static void __attribute__((constructor)) init() | |||
| fprintf(stderr, "cannot retrieve cmd line"); | |||
| _exit (EXIT_FAILURE); | |||
| } | |||
| // Even if unused, this is needed to ensure we properly free the memory | |||
| argv0 = argv[0]; | |||
There was a problem hiding this comment.
We always let init be in charge of freeing argv0 now.
|
Can you please squash the commits? I don't think the previous commit need to be part of the history. |
`argv[0]`, ie: the full buffer allocated by `get_cmd_line_args`, was going to be freed only if `can_use_shortcut()` was called. Instead, let `init()` always manage `argv0` lifecycle. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
b3b3406 to
51fd6e9
Compare
|
Sure! Done |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP, giuseppe, Luap99 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
argv[0], ie: the full buffer allocated byget_cmd_line_args, was going to be freed only ifcan_use_shortcut()was called. Since that is not always the case, add a fallback cleanup method.Spotted using
-fsanitize=address:Fix was tested by running once again (after applying the patch) with asan.
Does this PR introduce a user-facing change?