[PATCH] mkfs: Fix input offset counting in headerball mode

[Gao Xiang]

Hi Mike,

On 2024/11/9 00:37, Mike Baynton wrote:
> Thanks for the update, Gao! Appreciate your review.
> 
> Mike

I'm just back from vacation.

> 
> On Fri, Oct 25, 2024 at 8:36 PM Gao Xiang <hsiangkao at linux.alibaba.com> wrote:
>>
>> Hi Mike,
>>
>> On 2024/10/25 03:58, Mike Baynton wrote:
>>> When using --tar=headerball, most files included in the headerball are
>>> not included in the EROFS image. mkfs.erofs typically exits prematurely,
>>> having processed non-USTAR blocks as USTAR and believing they are
>>> end-of-archive markers. (Other failure modes are probably also possible
>>> if the input stream doesn't look like end-of-archive markers at the
>>> locations that are being read.)
>>>
>>> This is because we lost correct count of bytes that are read from the
>>> input stream when in headerball (or ddtaridx) modes. We were assuming that
>>> in these modes no data would be read following the ustar block, but in
>>> case of things like PAX headers, lots more data may be read without
>>> incrementing tar->offset.
>>>
>>> This corrects by always incrementing the offset counter, and then
>>> decrementing it again in the one case where headerballs differ -
>>> regular file data blocks are not present.
>>>
>>> Signed-off-by: Mike Baynton <mike at mbaynton.com>
>>
>> Sorry for late reply, I'm busy in personal stuffs now.
>> I will look into these cases and reply again later.

Yeah, thanks for the nice catch!  Yet I'm not sure if
decreasing `tar->offset` so late is safe (because there
are potential early exits here).

So how about the following diff? If it looks good to you
too, could you submit another version for this?

diff --git a/lib/tar.c b/lib/tar.c
index b32abd4..990c6cb 100644
--- a/lib/tar.c
+++ b/lib/tar.c
@@ -808,13 +808,14 @@ out_eot:
  	}

  	dataoff = tar->offset;
-	if (!(tar->headeronly_mode || tar->ddtaridx_mode))
-		tar->offset += st.st_size;
+	tar->offset += st.st_size;
  	switch(th->typeflag) {
  	case '0':
  	case '7':
  	case '1':
  		st.st_mode |= S_IFREG;
+		if (tar->headeronly_mode || tar->ddtaridx_mode)
+			tar->offset -= st.st_size;
  		break;
  	case '2':
  		st.st_mode |= S_IFLNK;

Thanks,
Gao Xiang