Omar Polo <op@omarpolo.com> wrote:
> Omar Polo <op@omarpolo.com> wrote:
> > It seems that i was a bit too much optimistic in c98b0f0.  With got
> > freshly compiled from ports i sometimes get a 'bad offset in pack file'
> > failure.  This happened in 'got status', 'got diff', ... and even in
> > tog, in got.git and src.git.  This happens very frequently thought, but
> > not always.
> > 
> > Curiously, this didn't happened in my local builds.
> > 
> > Diff below fixes the issue for me.  It moves the sanity check in only in
> > the case where the object was effectively packed.
> > 
> > This is just fixing the behaviour thought, i don't know why/how
> > pack_offset was negative.  i tried with
> > 
> > 	if (obj->pack_offset < 0)
> > 		abort();
> > 			
> > in got_privsep_send_obj but couldn't get it to segfault.
> 
> ...because the libexec is sending good data.  

* because the libexec is attempting to send good data.

> got_privsep_send_obj
> copies the data from the given struct got_object into a stack-allocated
> struct got_imsg_object.  Except that this struct isn't zeroed and so we
> read garbage value when the object is not packed.
> 
> so please discard my previous diff, the solution is to fix this at the
> sending side:

and eventually we may want to zero also the other structs?  most of
these only have one or two fields, but others like the delta have a
number of them.

every one of them has all the fields always initialized thought, only
got_privsep_send_obj was optionally initializing some.

diff 0fe55807cf233278482e51afcee4b60d5c974340 /home/op/w/got
blob - 2d5319f731933605f168211ff0d2be2f135b57b2
file + lib/privsep.c
--- lib/privsep.c
+++ lib/privsep.c
@@ -297,6 +297,7 @@ got_privsep_send_raw_obj(struct imsgbuf *ibuf, off_t s
 	size_t len = sizeof(iobj);
 	struct ibuf *wbuf;
 
+	memset(&iobj, 0, sizeof(iobj));
 	iobj.hdrlen = hdrlen;
 	iobj.size = size;
 
@@ -387,6 +388,7 @@ got_privsep_send_commit_req(struct imsgbuf *ibuf, int 
 	void *data;
 	size_t len;
 
+	memset(&iobj, 0, sizeof(iobj));
 	if (pack_idx != -1) { /* commit is packed */
 		iobj.idx = pack_idx;
 		memcpy(iobj.id, id->sha1, sizeof(iobj.id));
@@ -445,6 +447,7 @@ got_privsep_send_tag_req(struct imsgbuf *ibuf, int fd,
 	void *data;
 	size_t len;
 
+	memset(&iobj, 0, sizeof(iobj));
 	if (pack_idx != -1) { /* tag is packed */
 		iobj.idx = pack_idx;
 		memcpy(iobj.id, id->sha1, sizeof(iobj.id));
@@ -471,6 +474,7 @@ got_privsep_send_blob_req(struct imsgbuf *ibuf, int in
 	void *data;
 	size_t len;
 
+	memset(&iobj, 0, sizeof(iobj));
 	if (pack_idx != -1) { /* blob is packed */
 		iobj.idx = pack_idx;
 		memcpy(iobj.id, id->sha1, sizeof(iobj.id));
@@ -1423,6 +1427,8 @@ send_tree_entries_batch(struct imsgbuf *ibuf,
 	struct got_imsg_tree_entries ientries;
 	int i;
 
+	memset(&ientries, 0, sizeof(ientries));
+
 	wbuf = imsg_create(ibuf, GOT_IMSG_TREE_ENTRIES, 0, 0, len);
 	if (wbuf == NULL)
 		return got_error_from_errno("imsg_create TREE_ENTRY");
@@ -1495,6 +1501,7 @@ got_privsep_send_tree(struct imsgbuf *ibuf,
 	const struct got_error *err = NULL;
 	struct got_imsg_tree_object itree;
 
+	memset(&itree, 0, sizeof(itree));
 	itree.nentries = nentries;
 	if (imsg_compose(ibuf, GOT_IMSG_TREE, 0, 0, -1, &itree, sizeof(itree))
 	    == -1)
@@ -1685,6 +1692,7 @@ got_privsep_send_blob(struct imsgbuf *ibuf, size_t siz
 {
 	struct got_imsg_blob iblob;
 
+	memset(&iblob, 0, sizeof(iblob));
 	iblob.size = size;
 	iblob.hdrlen = hdrlen;
 
@@ -1964,6 +1972,9 @@ got_privsep_init_pack_child(struct imsgbuf *ibuf, stru
 	struct got_imsg_pack ipack;
 	int fd;
 
+	memset(&ipackidx, 0, sizeof(ipackidx));
+	memset(&ipack, 0, sizeof(ipack));
+
 	ipackidx.len = packidx->len;
 	ipackidx.packfile_size = pack->filesize;
 	fd = dup(packidx->fd);
@@ -2002,6 +2013,7 @@ got_privsep_send_packed_obj_req(struct imsgbuf *ibuf, 
 {
 	struct got_imsg_packed_object iobj;
 
+	memset(&iobj, 0, sizeof(iobj));
 	iobj.idx = idx;
 	memcpy(iobj.id, id->sha1, sizeof(iobj.id));
 
@@ -2019,6 +2031,7 @@ got_privsep_send_packed_raw_obj_req(struct imsgbuf *ib
 {
 	struct got_imsg_packed_object iobj;
 
+	memset(&iobj, 0, sizeof(iobj));
 	iobj.idx = idx;
 	memcpy(iobj.id, id->sha1, sizeof(iobj.id));
 
@@ -2983,6 +2996,7 @@ got_privsep_send_raw_delta_req(struct imsgbuf *ibuf, i
 {
 	struct got_imsg_raw_delta_request dreq;
 
+	memset(&dreq, 0, sizeof(dreq));
 	dreq.idx = idx;
 	memcpy(dreq.id, id->sha1, SHA1_DIGEST_LENGTH);
 
@@ -3007,6 +3021,7 @@ got_privsep_send_raw_delta(struct imsgbuf *ibuf, uint6
 	struct got_imsg_raw_delta idelta;
 	int ret;
 
+	memset(&idelta, 0, sizeof(idelta));
 	idelta.base_size = base_size;
 	idelta.result_size = result_size;
 	idelta.delta_size = delta_size;
@@ -3089,6 +3104,8 @@ send_idlist(struct imsgbuf *ibuf, struct got_object_id
 	struct ibuf *wbuf;
 	size_t i;
 
+	memset(&idlist, 0, sizeof(idlist));
+
 	if (nids > GOT_IMSG_OBJ_ID_LIST_MAX_NIDS)
 		return got_error(GOT_ERR_NO_SPACE);
 
@@ -3224,6 +3241,8 @@ got_privsep_send_reused_deltas(struct imsgbuf *ibuf,
 	struct got_imsg_reused_deltas ideltas;
 	size_t i;
 
+	memset(&ideltas, 0, sizeof(ideltas));
+
 	if (ndeltas > GOT_IMSG_REUSED_DELTAS_MAX_NDELTAS)
 		return got_error(GOT_ERR_NO_SPACE);