Fixed some memory leaks and UB #2
Conversation
|
Thanks, as mentioned on the mailing list some of the changes here were not entirely correct. I did a pass over ASAN and UBSAN sanitizers and fixed everything that showed up, which shows up as clean now in master. I've attributed some of the reported problems to you, and based a patch on a change here. I'm closing this now. |
Have you read my answer explaining why they are correct? |
| @@ -136,6 +138,7 @@ trigdef_update_start(enum trigdef_update_flags uf) | |||
|
|
|||
| setcloexec(fileno(trig_new_deferred), newfn.buf); | |||
| } | |||
| free(triggersdir); | |||
|
|
|||
There was a problem hiding this comment.
These were fixed with commit 35d024e, as the variable cannot be freed as it needs to be used by the _done function.
| @@ -1114,7 +1114,7 @@ void process_archive(const char *filename) { | |||
| deb_verify(filename); | |||
|
|
|||
| /* Get the control information directory. */ | |||
| cidir = get_control_dir(cidir); | |||
| cidir = get_control_dir(cidir); // todo: Memory leak!!! cannot free because it is used after exitting this func. | |||
| cidirrest = cidir + strlen(cidir); | |||
There was a problem hiding this comment.
This was fixed with commit 60dba33.
| } | ||
| if(tar.err.str) // todo: during normal operation no error strings should be set! | ||
| free(tar.err.str); | ||
|
|
||
| if (fd_skip(p1[0], -1, &err) < 0) |
There was a problem hiding this comment.
This didn't make sense. If tar.err is set but no error has been reported then something else is wrong. Otherwise there should be no need to free it. If you have found an instance where this happens, I'd appreciate a description of how/where that happens.
| @@ -485,6 +488,8 @@ tar_extractor(struct tar_archive *tar) | |||
| } | |||
|
|
|||
| if (h.name[0] == '\0') { | |||
| if(tar->err.str != NULL) | |||
| free(tar->err.str); | |||
| status = dpkg_put_error(&tar->err, | |||
There was a problem hiding this comment.
This didn't make sense. I don't see any code paths were tar->err is set and the loop continues. If there's one such occurrence then that needs to stop the loop, not continue and overwrite the error variable. If you have found one such occurrence I'd appreciate a description of how/where that happens.
There was a problem hiding this comment.
Line 337 in cf8fe0d
I have to admit - my fix was incorrect. The correct fix would be to fix that line.
The one that was fine, I fixed at the time. The others I still do not see how would be possible. I'd appreciate more information in case I'm missing something. |
No description provided.