Fix assorted leaks found via fuzzing#4698
Conversation
git__free is insufficient if the packet is a git_pkt_ref or another type that requires freeing referenced structures.
pks-t
left a comment
There was a problem hiding this comment.
Thanks for this PR, the fixes look obviously good to me. I'd appreciate it if you could include one more commit to get rid of this if (pkt) git_pkt_free(pkt) mess, even though it's not your fault but ours. :)
src/transports/smart_protocol.c
Outdated
|
|
||
| while (1) { | ||
| git__free(pkt); | ||
| if (pkt) { |
There was a problem hiding this comment.
We usually do not check whether a pointer is set before freeing it. But in this case it is necessary, as git_pkt_free doesn't check for NULL pointers itself
src/transports/smart_protocol.c
Outdated
|
|
||
| git__free(pkt); | ||
| if (pkt) { | ||
| git_pkt_free(pkt); |
There was a problem hiding this comment.
Huh, one more time. I know it's not your fault, but instead the existing code is to blame. But I'd highly appreciate if you did a preliminary commit which fixes git_pkt_free to return early in case a NULL pointer is being passed to it.
|
By the way, did you already see #4433? |
|
I hadn't! Thanks for the pointer, I'll look into that past attempt. |
|
Fixed |
|
Thanks for this. 🎉 |
These were all found by my work-in-progress oss-fuzz fuzzer.