[PATCH] erofs-utils: tar: fix negative GNU base-256 number parsing

[PATCH] erofs-utils: tar: fix negative GNU base-256 number parsing

[Vansh Choudhary]

GNU base-256 fields use a 0xff prefix for negative values, but
tarerofs_parsenum() currently accumulates them in signed long long.
That does not sign-extend negative values correctly and can also
trigger signed-overflow undefined behavior while shifting.

Handle positive and negative GNU base-256 fields separately and do the
byte accumulation in unsigned long long instead.

This fixes GNU base-256 decoding for negative tar metadata values such
as mtime, uid, gid and device numbers.

Fixes: 95d315fd7958 ("erofs-utils: introduce tarerofs")
Signed-off-by: Vansh Choudhary <ch@vnsh.in>
---
 lib/tar.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/lib/tar.c b/lib/tar.c
index 871779a..05d1a74 100644
--- a/lib/tar.c
+++ b/lib/tar.c
@@ -328,17 +328,26 @@ static long long tarerofs_otoi(const char *ptr, int len)
 
 static long long tarerofs_parsenum(const char *ptr, int len)
 {
+	const u8 *p = (const u8 *)ptr;
+
 	errno = 0;
 	/*
 	 * For fields containing numbers or timestamps that are out of range
 	 * for the basic format, the GNU format uses a base-256 representation
 	 * instead of an ASCII octal number.
 	 */
-	if (*(char *)ptr == '\200' || *(char *)ptr == '\377') {
-		long long res = 0;
+	if (*(char *)ptr == '\200') {
+		unsigned long long res = 0;
 
 		while (--len)
-			res = (res << 8) | (u8)*(++ptr);
+			res = (res << 8) | *(++p);
+		return res;
+	}
+	if (*(char *)ptr == '\377') {
+		unsigned long long res = -1ULL;
+
+		while (len--)
+			res = (res << 8) | *(p++);
 		return res;
 	}
 	return tarerofs_otoi(ptr, len);
-- 
2.43.0

Re: [PATCH] erofs-utils: tar: fix negative GNU base-256 number parsing

[Gao Xiang]

On 2026/4/5 18:18, Vansh Choudhary wrote:
> GNU base-256 fields use a 0xff prefix for negative values, but
> tarerofs_parsenum() currently accumulates them in signed long long.
> That does not sign-extend negative values correctly and can also
> trigger signed-overflow undefined behavior while shifting.
> 
> Handle positive and negative GNU base-256 fields separately and do the
> byte accumulation in unsigned long long instead.
> 
> This fixes GNU base-256 decoding for negative tar metadata values such
> as mtime, uid, gid and device numbers.
> 
> Fixes: 95d315fd7958 ("erofs-utils: introduce tarerofs")
> Signed-off-by: Vansh Choudhary <ch@vnsh.in>

Provide a testcase for this?

Thanks,
Gao Xiang

> ---
>   lib/tar.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/tar.c b/lib/tar.c
> index 871779a..05d1a74 100644
> --- a/lib/tar.c
> +++ b/lib/tar.c
> @@ -328,17 +328,26 @@ static long long tarerofs_otoi(const char *ptr, int len)
>   
>   static long long tarerofs_parsenum(const char *ptr, int len)
>   {
> +	const u8 *p = (const u8 *)ptr;
> +
>   	errno = 0;
>   	/*
>   	 * For fields containing numbers or timestamps that are out of range
>   	 * for the basic format, the GNU format uses a base-256 representation
>   	 * instead of an ASCII octal number.
>   	 */
> -	if (*(char *)ptr == '\200' || *(char *)ptr == '\377') {
> -		long long res = 0;
> +	if (*(char *)ptr == '\200') {
> +		unsigned long long res = 0;
>   
>   		while (--len)
> -			res = (res << 8) | (u8)*(++ptr);
> +			res = (res << 8) | *(++p);
> +		return res;
> +	}
> +	if (*(char *)ptr == '\377') {
> +		unsigned long long res = -1ULL;
> +
> +		while (len--)
> +			res = (res << 8) | *(p++);
>   		return res;
>   	}
>   	return tarerofs_otoi(ptr, len);

[PATCH] erofs-utils: tests: add test for negative GNU tar mtimes

[Vansh Choudhary]

Add a regression test for negative GNU tar mtimes.

It creates a tarball with a file whose mtime is -1, checks that the
mtime field is encoded in GNU base-256 format, and verifies that the
timestamp is preserved after building and extracting the image.

Signed-off-by: Vansh Choudhary <ch@vnsh.in>
---
 tests/Makefile.am   |  3 +++
 tests/erofs/030     | 57 +++++++++++++++++++++++++++++++++++++++++++++
 tests/erofs/030.out |  2 ++
 3 files changed, 62 insertions(+)
 create mode 100755 tests/erofs/030
 create mode 100644 tests/erofs/030.out

diff --git a/tests/Makefile.am b/tests/Makefile.am
index d8ac067..28edc0d 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -126,6 +126,9 @@ TESTS += erofs/028
 # 029 - test FUSE daemon and kernel error handling on corrupted inodes
 TESTS += erofs/029
 
+# 030 - regression test for negative GNU tar mtimes
+TESTS += erofs/030
+
 # NEW TEST CASE HERE
 # TESTS += erofs/999
 
diff --git a/tests/erofs/030 b/tests/erofs/030
new file mode 100755
index 0000000..6d57225
--- /dev/null
+++ b/tests/erofs/030
@@ -0,0 +1,57 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+#
+# 030 - regression test for negative GNU tar mtimes
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$(echo $0 | awk '{print $((NF-1))"/"$NF}' FS="/")
+
+# get standard environment, filters and checks
+. "${srcdir}/common/rc"
+
+cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+echo "QA output created by $seq"
+
+if [ -z "$SCRATCH_DEV" ]; then
+	SCRATCH_DEV=$tmp/erofs_$seq.img
+	rm -f $SCRATCH_DEV
+fi
+
+localdir="$tmp/$seq"
+rm -rf $localdir
+mkdir -p $localdir/src
+
+: > $localdir/src/foo
+touch -d @-1 $localdir/src/foo >> $seqres.full 2>&1 || \
+	_notrun "touch -d @-1 is not supported"
+tar --format=gnu -C $localdir/src -cf $localdir/foo.tar foo >> $seqres.full 2>&1 || \
+	_fail "failed to create tarball"
+
+mtime=$(od -An -t x1 -j 136 -N 12 $localdir/foo.tar | tr -d '[:space:]')
+[ "$mtime" = "ffffffffffffffffffffffff" ] || \
+	_notrun "tar did not encode a negative GNU mtime"
+
+rm -f $SCRATCH_DEV
+$MKFS_EROFS_PROG --tar=f $SCRATCH_DEV $localdir/foo.tar \
+	>> $seqres.full 2>&1 || _fail "failed to mkfs tarball"
+
+rm -rf $localdir/out
+mkdir -p $localdir/out
+$FSCK_EROFS_PROG --extract=$localdir/out $SCRATCH_DEV >> $seqres.full 2>&1 || \
+	_fail "failed to extract image"
+
+mtime=$(stat -c '%Y' $localdir/out/foo)
+[ "$mtime" = "-1" ] || _fail "negative GNU mtime was not preserved"
+
+echo Silence is golden
+status=0
+exit 0
diff --git a/tests/erofs/030.out b/tests/erofs/030.out
new file mode 100644
index 0000000..06a1c8f
--- /dev/null
+++ b/tests/erofs/030.out
@@ -0,0 +1,2 @@
+QA output created by 030
+Silence is golden
-- 
2.43.0

[PATCH v2] erofs-utils: tests: add test for negative GNU tar mtimes

[Gao Xiang]

From: Vansh Choudhary <ch@vnsh.in>

Add a regression test for negative GNU tar mtimes.

It creates a tarball with a file whose mtime is -1, checks that the
mtime field is encoded in GNU base-256 format, and verifies that the
timestamp is preserved after building and extracting the image.

Signed-off-by: Vansh Choudhary <ch@vnsh.in>
Signed-off-by: Gao Xiang <xiang@kernel.org>
---
v2:
 - use dump.erofs instead of fsck.erofs;
 - switch to MIT license.

 tests/Makefile.am   |  3 +++
 tests/erofs/030     | 55 +++++++++++++++++++++++++++++++++++++++++++++
 tests/erofs/030.out |  2 ++
 3 files changed, 60 insertions(+)
 create mode 100755 tests/erofs/030
 create mode 100644 tests/erofs/030.out

diff --git a/tests/Makefile.am b/tests/Makefile.am
index d8ac067805e8..28edc0d744d1 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -126,6 +126,9 @@ TESTS += erofs/028
 # 029 - test FUSE daemon and kernel error handling on corrupted inodes
 TESTS += erofs/029
 
+# 030 - regression test for negative GNU tar mtimes
+TESTS += erofs/030
+
 # NEW TEST CASE HERE
 # TESTS += erofs/999
 
diff --git a/tests/erofs/030 b/tests/erofs/030
new file mode 100755
index 000000000000..72765cb19aee
--- /dev/null
+++ b/tests/erofs/030
@@ -0,0 +1,55 @@
+#!/bin/sh
+# SPDX-License-Identifier: MIT
+#
+# 030 - regression test for negative GNU tar mtimes
+#
+seq=`basename $0`
+seqres=$RESULT_DIR/$(echo $0 | awk '{print $((NF-1))"/"$NF}' FS="/")
+
+# get standard environment, filters and checks
+. "${srcdir}/common/rc"
+
+cleanup()
+{
+	cd /
+	rm -rf $tmp.*
+}
+
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
+# real QA test starts here
+echo "QA output created by $seq"
+
+if [ -z "$SCRATCH_DEV" ]; then
+	SCRATCH_DEV=$tmp/erofs_$seq.img
+	rm -f $SCRATCH_DEV
+fi
+
+localdir="$tmp/$seq"
+rm -rf $localdir
+mkdir -p $localdir/src
+
+touch -d @-1 $localdir/src/testfile >> $seqres.full 2>&1 || \
+	_notrun "touch -d @-1 is not supported"
+tar --format=gnu -C $localdir/src -cf $localdir/foo.tar testfile >> $seqres.full 2>&1 || \
+	_fail "failed to create tarball"
+
+mtime=$(od -An -t x1 -j 136 -N 12 $localdir/foo.tar | tr -d '[:space:]')
+[ "$mtime" = "ffffffffffffffffffffffff" ] || \
+	_notrun "tar did not encode a negative GNU mtime"
+
+$MKFS_EROFS_PROG --tar=f $SCRATCH_DEV $localdir/foo.tar \
+	>> $seqres.full 2>&1 || _fail "failed to mkfs tarball"
+
+output=$($DUMP_EROFS_PROG --path=/testfile $SCRATCH_DEV 2>&1)
+[ $? -eq 0 ] || _fail "failed to dump testfile"
+echo "$output" >> $seqres.full
+
+mtime=$(grep '^Timestamp:' <<< $output | sed 's/^.*: //')
+[ "x$mtime" = "x1970-01-01 07:59:59.000000000" ] || \
+	_fail "negative GNU mtime was not preserved"
+
+echo Silence is golden
+status=0
+exit 0
diff --git a/tests/erofs/030.out b/tests/erofs/030.out
new file mode 100644
index 000000000000..06a1c8fe02bb
--- /dev/null
+++ b/tests/erofs/030.out
@@ -0,0 +1,2 @@
+QA output created by 030
+Silence is golden
-- 
2.43.5

Re: [PATCH] erofs-utils: tar: fix negative GNU base-256 number parsing

[Gao Xiang]

On 2026/4/5 18:18, Vansh Choudhary wrote:
> GNU base-256 fields use a 0xff prefix for negative values, but
> tarerofs_parsenum() currently accumulates them in signed long long.
> That does not sign-extend negative values correctly and can also
> trigger signed-overflow undefined behavior while shifting.
> 
> Handle positive and negative GNU base-256 fields separately and do the
> byte accumulation in unsigned long long instead.
> 
> This fixes GNU base-256 decoding for negative tar metadata values such
> as mtime, uid, gid and device numbers.
> 
> Fixes: 95d315fd7958 ("erofs-utils: introduce tarerofs")
> Signed-off-by: Vansh Choudhary <ch@vnsh.in>

I ran the test but it succeeds, so is it just a UB or it really
impacts end users?

Thanks,
Gao Xiang

> ---
>   lib/tar.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/tar.c b/lib/tar.c
> index 871779a..05d1a74 100644
> --- a/lib/tar.c
> +++ b/lib/tar.c
> @@ -328,17 +328,26 @@ static long long tarerofs_otoi(const char *ptr, int len)
>   
>   static long long tarerofs_parsenum(const char *ptr, int len)
>   {
> +	const u8 *p = (const u8 *)ptr;
> +
>   	errno = 0;
>   	/*
>   	 * For fields containing numbers or timestamps that are out of range
>   	 * for the basic format, the GNU format uses a base-256 representation
>   	 * instead of an ASCII octal number.
>   	 */
> -	if (*(char *)ptr == '\200' || *(char *)ptr == '\377') {
> -		long long res = 0;
> +	if (*(char *)ptr == '\200') {
> +		unsigned long long res = 0;
>   
>   		while (--len)
> -			res = (res << 8) | (u8)*(++ptr);
> +			res = (res << 8) | *(++p);
> +		return res;
> +	}
> +	if (*(char *)ptr == '\377') {
> +		unsigned long long res = -1ULL;
> +
> +		while (len--)
> +			res = (res << 8) | *(p++);
>   		return res;
>   	}
>   	return tarerofs_otoi(ptr, len);

Re: [PATCH] erofs-utils: tar: fix negative GNU base-256 number parsing

[Vansh Choudhary]

Yes, this definitely fixes the signed-overflow UB in tarerofs_parsenum(),
and also makes the negative GNU base-256 decoding logic correct.  But
from the current testcase alone, I cannot firmly show a visible end-user
failure on the old code path.

So I think this is safest to describe as a correctness + UB fix.

Thanks,
Vansh