On Sat, Jul 02, 2022 at 12:40:32PM +0200, Omar Polo wrote:
> Sebastien Marie <semarie@online.fr> wrote:
> 
> I think I got it.  In send_tree_entries (lib/privsep.c) we're computing
> the size of the got_parsed_tree_entry (pte) that we're about to send as
> 
> 	size_t len = sizeof(*pte) + pte->namelen;
> 
> but it's wrong!  The struct is defined as: (lib/got_lib_object_parse.h)
> 
> 	struct got_parsed_tree_entry {
> 		const char *name; /* Points to name in parsed buffer */
> 		size_t namelen; /* strlen(name) */
> 		mode_t mode; /* Mode parsed from tree buffer. */
> 		uint8_t *id; /* Points to ID in parsed tree buffer. */
> 	};
> 
> and we serialize it in send_tree_entries_batch by writing the id (20
> bytes), mode, namelen and name in the wbuf.
> 
> On amd64 the size of the two pointers hides the issue but on i386 this
> error in computing the size of the batch means we try to add to an imsg
> more data than what allocated.
> 
> The following diff seems to fix the issue for me, `got co' proceeds past
> databases/.  (i'm currently at devel/p5-*, this machine is slow)
> 

yes, it solves the problem here.

$ got up		# got from ports
got-read-tree: imsg_add TREE_ENTRY: Result too large
got: Result too large
$ ~/bin/got up		# got from build with your patch
Already up-to-date

I am unsure to fully understand your explanation, but it seems to make sense.

The diff would be fine with me, but maybe wait for someone else to comment to :)

> diff -s /home/op/w/got
> commit - db0dfdd7e5c2c5a38ed7c3291a0615132bcb5945
> path + /home/op/w/got (staged changes)
> blob - 70eb167c5ee71b29c045bdae0de5c7b7859403f7
> blob + 31bc523dadb98ca90e8fcdfc390e7beba909f667
> --- lib/privsep.c
> +++ lib/privsep.c
> @@ -1471,7 +1471,8 @@ send_tree_entries(struct imsgbuf *ibuf, struct got_par
>  	i = 0;
>  	for (j = 0; j < nentries; j++) {
>  		struct got_parsed_tree_entry *pte = &entries[j];
> -		size_t len = sizeof(*pte) + pte->namelen;
> +		size_t len = SHA1_DIGEST_LENGTH + sizeof(pte->mode) +
> +		    sizeof(pte->namelen) + pte->namelen;
>  
>  		if (j > 0 &&
>  		    entries_len + len > MAX_IMSGSIZE - IMSG_HEADER_SIZE) {
> 
> 
> 
> 

Thanks.
-- 
Sebastien Marie