From: Omar Polo <op@omarpolo.com>
Subject: fix GOT_IMSG_COMMIT_TRAVERSAL_REQUEST
To: gameoftrees@openbsd.org
Date: Thu, 23 Feb 2023 20:14:58 +0100

The serialization/deserialization were sending data differently to
what got_lib_privsep.h described.  This also fixes an object id sent
as SHA1_DIGEST_LENGTH instead of as whole struct got_object_id.

I decided to keep the path_len field (before was unused) and the
struct got_imsg_commit_traversal_request: it's easier to document the
data for GOT_IMSG_COMMIT_TRAVERSAL_REQUEST this way, instead of just a
got_imsg_packed_object plus an optional path.

There's the weird len > 1 case to handle though, since we're sending a
NUL-terminated string.

It also adds the usual comment "Keep in sync with struct ..." that
it's been helpful in other places to me in the past.
 
diff aaff679f20511b583d1cb1711d90549400df530c c829fe838002b73c360e33c6ddd13b047d54286b
commit - aaff679f20511b583d1cb1711d90549400df530c
commit + c829fe838002b73c360e33c6ddd13b047d54286b
blob - 7599c74782932b66b186f9bb91980bd32cd5c663
blob + d517f36f2ccbda4c48348016a20e6d6e87fbd1a4
--- lib/got_lib_privsep.h
+++ lib/got_lib_privsep.h
@@ -576,8 +576,7 @@ struct got_imsg_commit_traversal_request {
 
 /* Structure for GOT_IMSG_COMMIT_TRAVERSAL_REQUEST  */
 struct got_imsg_commit_traversal_request {
-	uint8_t id[SHA1_DIGEST_LENGTH];
-	int idx;
+	struct got_imsg_packed_object iobj;
 	size_t path_len;
 	/* Followed by path_len bytes of path data */
 } __attribute__((__packed__));
blob - b8ea370fd2e88931bfc3f441dded6c848d55b3bd
blob + 428c52ef5d63343f90856dcfbdd533a85e01e94d
--- lib/privsep.c
+++ lib/privsep.c
@@ -2657,12 +2657,19 @@ got_privsep_send_commit_traversal_request(struct imsgb
 	if (wbuf == NULL)
 		return got_error_from_errno(
 		    "imsg_create COMMIT_TRAVERSAL_REQUEST");
-	if (imsg_add(wbuf, id->sha1, SHA1_DIGEST_LENGTH) == -1)
+	/*
+	 * Keep in sync with struct got_imsg_commit_traversal_request
+	 * and struct got_imsg_packed_object.
+	 */
+	if (imsg_add(wbuf, id, sizeof(*id)) == -1)
 		return got_error_from_errno("imsg_add "
 		    "COMMIT_TRAVERSAL_REQUEST");
 	if (imsg_add(wbuf, &idx, sizeof(idx)) == -1)
 		return got_error_from_errno("imsg_add "
 		    "COMMIT_TRAVERSAL_REQUEST");
+	if (imsg_add(wbuf, &path_len, sizeof(path_len)) == -1)
+		return got_error_from_errno("imsg_add "
+		    "COMMIT_TRAVERSAL_REQUEST");
 	if (imsg_add(wbuf, path, path_len) == -1)
 		return got_error_from_errno("imsg_add "
 		    "COMMIT_TRAVERSAL_REQUEST");
blob - ed4f9039ad8c4841bba30c73ac1ec2a413e881e7
blob + 8a1d148c63d9c40486bab560c3e7d77131742c67
--- libexec/got-read-pack/got-read-pack.c
+++ libexec/got-read-pack/got-read-pack.c
@@ -602,31 +602,30 @@ commit_traversal_request(struct imsg *imsg, struct ims
     struct got_object_cache *objcache)
 {
 	const struct got_error *err = NULL;
-	struct got_imsg_packed_object iobj;
+	struct got_imsg_commit_traversal_request ctreq;
 	struct got_object_qid *pid;
 	struct got_commit_object *commit = NULL, *pcommit = NULL;
 	struct got_parsed_tree_entry *entries = NULL, *pentries = NULL;
 	size_t nentries = 0, nentries_alloc = 0;
 	size_t pnentries = 0, pnentries_alloc = 0;
 	struct got_object_id id;
-	size_t datalen, path_len;
+	size_t datalen;
 	char *path = NULL;
 	const int min_alloc = 64;
 	int changed = 0, ncommits = 0, nallocated = 0;
 	struct got_object_id *commit_ids = NULL;
 
 	datalen = imsg->hdr.len - IMSG_HEADER_SIZE;
-	if (datalen < sizeof(iobj))
+	if (datalen < sizeof(ctreq))
 		return got_error(GOT_ERR_PRIVSEP_LEN);
-	memcpy(&iobj, imsg->data, sizeof(iobj));
-	memcpy(&id, &iobj.id, sizeof(id));
+	memcpy(&ctreq, imsg->data, sizeof(ctreq));
+	memcpy(&id, &ctreq.iobj.id, sizeof(id));
 
-	path_len = datalen - sizeof(iobj) - 1;
-	if (path_len < 0)
+	if (datalen != sizeof(ctreq) + ctreq.path_len)
 		return got_error(GOT_ERR_PRIVSEP_LEN);
-	if (path_len > 0) {
-		path = imsg->data + sizeof(iobj);
-		if (path[path_len] != '\0')
+	if (ctreq.path_len > 1) { /* one is for the NUL byte */
+		path = imsg->data + sizeof(ctreq);
+		if (path[ctreq.path_len - 1] != '\0')
 			return got_error(GOT_ERR_PRIVSEP_LEN);
 	}