Download raw body.
fix invalid imsg_free() in got_privsep_recv_printed_commits()
This is probably the cause of the crash stsp observed by ctrl-c'ing
gotadmin pack -a (https://paste.apache.org/jpr92), while also fixing a
possible memory leak too.
if got_privsep_recv_imsg() fails before reading the imsg, the imsg
struct is still un-initialized and can't be passed to imsg_free().
Luckily, there are only two offenders in the tree which may call
imsg_free() in the error path of got_privsep_recv_imsg().
(technically this introduces a possible leak in libexec/got-read-patch
but it's just once per process so it should be acceptable.)
I think that got_privsep_recv_imsg() should free the imsg on error after
it has read something.
ok?
p.s. as a follow-up now that I noticed i think we should swap the order
of the checks in got_privsep_recv_imsg(), so to have the GOT_IMSG_ERROR
case first, then the length.
diff /home/op/w/got
commit - 7a86002db34d49472a7d75c1802ee99c2120ef3c
path + /home/op/w/got
blob - 79db43c49ad84782e167cf267f4958c7fa944561
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -141,12 +141,16 @@ got_privsep_recv_imsg(struct imsg *imsg, struct imsgbu
return got_error_from_errno("imsg_get");
}
- if (imsg->hdr.len < IMSG_HEADER_SIZE + min_datalen)
+ if (imsg->hdr.len < IMSG_HEADER_SIZE + min_datalen) {
+ imsg_free(imsg);
return got_error(GOT_ERR_PRIVSEP_LEN);
+ }
if (imsg->hdr.type == GOT_IMSG_ERROR) {
size_t datalen = imsg->hdr.len - IMSG_HEADER_SIZE;
- return recv_imsg_error(imsg, datalen);
+ err = recv_imsg_error(imsg, datalen);
+ imsg_free(imsg);
+ return err;
}
return NULL;
@@ -3510,10 +3514,8 @@ got_privsep_recv_painted_commits(struct got_object_id_
for (;;) {
err = got_privsep_recv_imsg(&imsg, ibuf, 0);
- if (err){
- imsg_free(&imsg);
+ if (err)
return err;
- }
datalen = imsg.hdr.len - IMSG_HEADER_SIZE;
if (imsg.hdr.type == GOT_IMSG_COMMIT_PAINTING_DONE) {
blob - 508821d598050b15d1b623785750f1a0c1040d10
file + libexec/got-read-patch/got-read-patch.c
--- libexec/got-read-patch/got-read-patch.c
+++ libexec/got-read-patch/got-read-patch.c
@@ -682,8 +682,8 @@ main(int argc, char **argv)
goto done;
}
err = got_privsep_flush_imsg(&ibuf);
-done:
imsg_free(&imsg);
+done:
if (fd != -1 && close(fd) == -1 && err == NULL)
err = got_error_from_errno("close");
if (fp != NULL && fclose(fp) == EOF && err == NULL)
fix invalid imsg_free() in got_privsep_recv_printed_commits()