GNU bug report logs - #75924
maint: fix s390 buffer flushes

Previous Next

Package: gzip;

Reported by: Eduard Stefes <eduard.stefes <at> ibm.com>

Date: Wed, 29 Jan 2025 14:20:02 UTC

Severity: normal

Merged with 74651, 75911

Done: Paul Eggert <eggert <at> cs.ucla.edu>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 75924 in the body.
You can then email your comments to 75924 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Wed, 29 Jan 2025 14:20:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Eduard Stefes <eduard.stefes <at> ibm.com>:
New bug report received and forwarded. Copy sent to bug-gzip <at> gnu.org. (Wed, 29 Jan 2025 14:20:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Eduard Stefes <eduard.stefes <at> ibm.com>
To: bug-gzip <bug-gzip <at> gnu.org>
Cc: Andreas Hasenack <andreas.hasenack <at> canonical.com>,
 Eduard Stefes <eduard.stefes <at> ibm.com>, Ilya Leoshkevich <iii <at> linux.ibm.com>
Subject: maint: fix s390 buffer flushes
Date: Wed, 29 Jan 2025 15:18:36 +0100
Problem reported by Nick Rosbrook in:
https://bugs.launchpad.net/ubuntu/+source/rsyslog/+bug/2083700

align the behavior of dfltcc_inflate to do the same as gzip_inflate
when it hits a premature EOF
---
 dfltcc.c    | 6 +++++-
 tests/hufts | 6 ++++--
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/dfltcc.c b/dfltcc.c
index 811c1f8..5d421bb 100644
--- a/dfltcc.c
+++ b/dfltcc.c
@@ -18,6 +18,7 @@
 #include <config.h>
 
 #include <stdlib.h>
+#include <errno.h>
 
 #ifdef HAVE_SYS_SDT_H
 # include <sys/sdt.h>
@@ -437,7 +438,10 @@ dfltcc_inflate ()
           if (fill_inbuf (1) == EOF)
             {
               /* Premature EOF.  */
-              return 2;
+              flush_outbuf ();
+              errno = 0;
+              read_error ();
+              __builtin_unreachable ();
             }
           inptr = 0;
         }
diff --git a/tests/hufts b/tests/hufts
index c464ef6..6dbb8ac 100755
--- a/tests/hufts
+++ b/tests/hufts
@@ -40,10 +40,12 @@ compare exp err || fail=1
 
 printf '\037\213\010\000\060\060\060\060\060\060\144\000\000\000' > bug33501 \
   || framework_failure_
-printf '\ngzip: stdin: invalid compressed data--format violated\n' >exp33501 \
+printf '\ngzip: stdin: invalid compressed data--format violated\n' > exp33501.1 \
+  || framework_failure_
+printf '\ngzip: stdin: unexpected end of file\n' > exp33501.2 \
   || framework_failure_
 returns_ 1 gzip -d <bug33501 >out33501 2> err33501-raw || fail=1
 sed "$clean_stderr" err33501-raw > err33501 || framework_failure_
-compare exp33501 err33501 || fail=1
+compare exp33501.1 err33501 || compare exp33501.2 err33501 || fail=1
 
 Exit $fail
-- 
2.48.1





Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Wed, 29 Jan 2025 19:00:03 GMT) Full text and rfc822 format available.

Message #8 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eduard Stefes <eduard.stefes <at> ibm.com>
Cc: 75924 <at> debbugs.gnu.org, Ilya Leoshkevich <iii <at> linux.ibm.com>,
 Andreas Hasenack <andreas.hasenack <at> canonical.com>
Subject: Re: bug#75924: maint: fix s390 buffer flushes
Date: Wed, 29 Jan 2025 10:58:59 -0800
[Message part 1 (text/plain, inline)]
Thanks for the bug report. I installed the attached, a bit simpler than 
the patch you suggested; can you please give it a try?

Also, is there a related bug near dfltcc.c line 375? That is, when 
(inptr == insize && fill_inbuf (1) == EOF && param->cf), won't insize 
then be zero, so that gzip will go into an infinite loop attempting to 
read past EOF?
[0001-gzip-fix-problem-with-s390-buffer-flushes.patch (text/x-patch, attachment)]

Merged 74651 75911 75924. Request was from Paul Eggert <eggert <at> cs.ucla.edu> to control <at> debbugs.gnu.org. (Wed, 29 Jan 2025 19:02:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Wed, 29 Jan 2025 19:51:02 GMT) Full text and rfc822 format available.

Message #13 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Eduard Stefes <Eduard.Stefes <at> ibm.com>
To: "eggert <at> cs.ucla.edu" <eggert <at> cs.ucla.edu>
Cc: "75924 <at> debbugs.gnu.org" <75924 <at> debbugs.gnu.org>,
 "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: RE: bug#75924: maint: fix s390 buffer flushes
Date: Wed, 29 Jan 2025 19:50:03 +0000
Hi,

On Wed, 2025-01-29 at 10:58 -0800, Paul Eggert wrote:
> Thanks for the bug report. I installed the attached, a bit simpler
> than 
> the patch you suggested; can you please give it a try?
changing the call fill_inbuff(1) to fill_inbuff(0) will not work.
dfltcc works on the outbuf, and fill_inbuff will eventually call
flush_window in case it hits EOF && 0. But it will NOT call
flush_outbuf() which we need in this case.



> 
> Also, is there a related bug near dfltcc.c line 375? That is, when 
> (inptr == insize && fill_inbuf (1) == EOF && param->cf), won't insize
> then be zero, so that gzip will go into an infinite loop attempting
> to 
> read past EOF?
I will think about this and try to come up with a test for it. To me it
looks like inbuf is preallocated (gzip.c:135) so it might be ok to read
past EOF and just compress zeros. The logic around it should prevent
access past inbuf[MAX]. But better be save then sorry.  

In that matter, should I extract the failing gzip tests from rsyslog
and add them to the gzip test suit?

-- 
Eduard Stefes <Eduard.Stefes <at> ibm.com>

Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Thu, 30 Jan 2025 04:59:04 GMT) Full text and rfc822 format available.

Message #16 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Hasenack <andreas.hasenack <at> canonical.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 75924 <at> debbugs.gnu.org, Eduard Stefes <eduard.stefes <at> ibm.com>,
 Ilya Leoshkevich <iii <at> linux.ibm.com>
Subject: Re: bug#75924: maint: fix s390 buffer flushes
Date: Wed, 29 Jan 2025 16:16:27 -0300
Hi,

the s390x build of gzip 1.12 with this patch applied failed[1] due to
a test failure:

FAIL: hufts
===========

+ initial_cwd_=/<<PKGBUILDDIR>>/builddir/tests
+ testdir_prefix_
+ printf gt
+ pfx_=gt
+ mktempd_ /<<PKGBUILDDIR>>/builddir/tests gt-hufts.XXXX
+ destdir_=/<<PKGBUILDDIR>>/builddir/tests
+ template_=gt-hufts.XXXX
+ MAX_TRIES_=4
+ destdir_slash_=/<<PKGBUILDDIR>>/builddir/tests/
+ unset TMPDIR
+ d=/<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ :
+ test -d /<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ ls -dgo /<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ perms=drwx------ 2 4096 Jan 29 19:11
/<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ :
+ echo /<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ return
+ test_dir_=/<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ cd /<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ srcdir=../../../tests
+ builddir=..
+ export srcdir builddir
+ gl_init_sh_nl_=

+ IFS=

+ expr 1 + 128
+ eval trap 'Exit 129' 1
+ trap Exit 129 1
+ expr 2 + 128
+ eval trap 'Exit 130' 2
+ trap Exit 130 2
+ expr 3 + 128
+ eval trap 'Exit 131' 3
+ trap Exit 131 3
+ expr 13 + 128
+ eval trap 'Exit 141' 13
+ trap Exit 141 13
+ expr 15 + 128
+ eval trap 'Exit 143' 15
+ trap Exit 143 15
+ saved_IFS=

+ IFS=:
+ new_PATH=
+ sep_=
+ test -d /<<PKGBUILDDIR>>/builddir/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir
+ sep_=:
+ test -d /usr/local/sbin/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin
+ sep_=:
+ test -d /usr/local/bin/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin
+ sep_=:
+ test -d /usr/sbin/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin
+ sep_=:
+ test -d /usr/bin/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin
+ sep_=:
+ test -d /sbin/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin
+ sep_=:
+ test -d /bin/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
+ sep_=:
+ test -d /usr/games/.
+ new_PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
+ sep_=:
+ IFS=

+ PATH=/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
+ export PATH
+ trap remove_tmp_ 0
+ path_prepend_ ..
+ test 1 != 0
+ path_dir_=..
+ abs_path_dir_=/<<PKGBUILDDIR>>/builddir/tests/..
+ PATH=/<<PKGBUILDDIR>>/builddir/tests/..:/<<PKGBUILDDIR>>/builddir:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games
+ create_exe_shims_ /<<PKGBUILDDIR>>/builddir/tests/..
+ return 0
+ shift
+ test 0 != 0
+ export PATH
+ printf \n...: invalid compressed data--format violated\n
+ fail=0
+ returns_ 1 gzip -dc /<<PKGBUILDDIR>>/builddir/../tests/hufts-segv.gz
+ compare /dev/null out
+ compare_dev_null_ /dev/null out
+ test 2 = 2
+ test x/dev/null = x/dev/null
+ test -s out
+ return 0
+ return 0
+ clean_stderr=
  s/.*hufts-segv.gz: /...: /
  /^+/d
  /Operation-Ending-Supplemental Code/d

+ sed
  s/.*hufts-segv.gz: /...: /
  /^+/d
  /Operation-Ending-Supplemental Code/d
 err-raw
+ compare exp err
+ compare_dev_null_ exp err
+ test 2 = 2
+ test xexp = x/dev/null
+ test xerr = x/dev/null
+ return 2
+ compare_ exp err
+ diff -u exp err
+ printf \037\213\010\000\060\060\060\060\060\060\144\000\000\000
+ printf \ngzip: stdin: invalid compressed data--format violated\n
+ returns_ 1 gzip -d
+ sed
  s/.*hufts-segv.gz: /...: /
  /^+/d
  /Operation-Ending-Supplemental Code/d
 err33501-raw
+ compare exp33501 err33501
+ compare_dev_null_ exp33501 err33501
+ test 2 = 2
+ test xexp33501 = x/dev/null
+ test xerr33501 = x/dev/null
+ return 2
+ compare_ exp33501 err33501
+ diff -u exp33501 err33501
--- exp33501 2025-01-29 19:11:54.875303836 +0000
+++ err33501 2025-01-29 19:11:54.875303836 +0000
@@ -1,2 +1,2 @@

-gzip: stdin: invalid compressed data--format violated
+gzip: stdin: unexpected end of file
+ fail=1
+ Exit 1
+ set +e
+ exit 1
+ exit 1
+ remove_tmp_
+ __st=1
+ cleanup_
+ :
+ test  = yes
+ cd /<<PKGBUILDDIR>>/builddir/tests
+ chmod -R u+rwx /<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ rm -rf /<<PKGBUILDDIR>>/builddir/tests/gt-hufts.whyU
+ exit 1
FAIL hufts (exit status: 1)

============================================================================
Testsuite summary for gzip 1.12
============================================================================
# TOTAL: 26
# PASS:  25
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0


1. https://launchpadlibrarian.net/773464868/buildlog_ubuntu-plucky-s390x.gzip_1.12-1.1ubuntu3~ppa1_BUILDING.txt.gz

On Wed, Jan 29, 2025 at 3:59 PM Paul Eggert <eggert <at> cs.ucla.edu> wrote:
>
> Thanks for the bug report. I installed the attached, a bit simpler than
> the patch you suggested; can you please give it a try?
>
> Also, is there a related bug near dfltcc.c line 375? That is, when
> (inptr == insize && fill_inbuf (1) == EOF && param->cf), won't insize
> then be zero, so that gzip will go into an infinite loop attempting to
> read past EOF?




Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Wed, 05 Feb 2025 15:47:01 GMT) Full text and rfc822 format available.

Message #19 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Eduard Stefes <Eduard.Stefes <at> ibm.com>
To: "eggert <at> cs.ucla.edu" <eggert <at> cs.ucla.edu>
Cc: "75924 <at> debbugs.gnu.org" <75924 <at> debbugs.gnu.org>,
 "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: Re:  bug#75924: maint: fix s390 buffer flushes
Date: Wed, 5 Feb 2025 15:46:15 +0000
On Wed, 2025-01-29 at 19:50 +0000, Eduard Stefes via GNU gzip
discussion and bug reports. wrote:
> Hi,
> 
> On Wed, 2025-01-29 at 10:58 -0800, Paul Eggert wrote:
> > Thanks for the bug report. I installed the attached, a bit simpler
> > than 
> > the patch you suggested; can you please give it a try?
> changing the call fill_inbuff(1) to fill_inbuff(0) will not work.
> dfltcc works on the outbuf, and fill_inbuff will eventually call
> flush_window in case it hits EOF && 0. But it will NOT call
> flush_outbuf() which we need in this case.
> 
> 
> 
> > 
> > Also, is there a related bug near dfltcc.c line 375? That is, when 
> > (inptr == insize && fill_inbuf (1) == EOF && param->cf), won't
> > insize
> > then be zero, so that gzip will go into an infinite loop attempting
> > to 
> > read past EOF?
> I will think about this and try to come up with a test for it. To me
> it
> looks like inbuf is preallocated (gzip.c:135) so it might be ok to
> read
> past EOF and just compress zeros. The logic around it should prevent
> access past inbuf[MAX]. But better be save then sorry.  

So I thought about this and talked with Ilya. We think that the only
possibility for a problem here, is if faulty hardware updates param->cf
incorrect. 

> 
> In that matter, should I extract the failing gzip tests from rsyslog
> and add them to the gzip test suit?

question remains: should I extract the failing rsyslog test cases for
gzip? This tests test if partially written or truncated gz files can be
extracted as expected.

I read rfc1952 and there is no explicit statement saying that
incomplete gz files *are corrupt or invalid*. Point 2.3.1.2. of the rfc
does not require the CRC32 and ISIZE field to be checked by a
decompressor. And Point 4. recommends that systems using gz should
*provide some means of validating the integrity of the compressed
data*. 

This all lets me conclude that it is indeed ok and valid to have
truncated gz files. And that it would be good to test this cases.


> 
> -- 
> Eduard Stefes <Eduard.Stefes <at> ibm.com>

-- 
Eduard Stefes <Eduard.Stefes <at> ibm.com>

Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Thu, 06 Feb 2025 18:21:01 GMT) Full text and rfc822 format available.

Message #22 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eduard Stefes <Eduard.Stefes <at> ibm.com>
Cc: "75924 <at> debbugs.gnu.org" <75924 <at> debbugs.gnu.org>,
 "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: Re: bug#75924: maint: fix s390 buffer flushes
Date: Thu, 6 Feb 2025 10:20:18 -0800
On 2025-02-05 07:46, Eduard Stefes wrote:

> We think that the only
> possibility for a problem here, is if faulty hardware updates param->cf
> incorrect.

I'd rather not worry about hardware going bad, unless the hardware 
failure is a known real-world problem. So how about if we make the 
following further patch?

  diff --git a/dfltcc.c b/dfltcc.c
  index 9f86581..a360ce2 100644
  --- a/dfltcc.c
  +++ b/dfltcc.c
  @@ -372,7 +372,7 @@ dfltcc_deflate (int pack_level)
	 /* Read the input data.  */
	 if (inptr == insize)
	   {
  -          if (fill_inbuf (1) == EOF && !param->cf)
  +          if (fill_inbuf (1) == EOF)
	       break;
	     inptr = 0;
	   }



> I read rfc1952 and there is no explicit statement saying that
> incomplete gz files *are corrupt or invalid*.

My reading is that the RFC does not specify any behavior for incomplete 
gz files. However, gzip goes beyond what the RFC requires, and is 
supposed to complain "unexpected end of file" for incomplete files, 
versus "invalid compressed data--format violated" for files that are not 
prefixes of valid compressed files. We should maintain that behavior as 
it is valuable information to give to the user.

I installed the patch to dfltcc.c that you suggested in 
<https://debbugs.gnu.org/cgi/bugreport.cgi?bug=75924#5>, superseding the 
incorrect patch I installed a couple of days ago. However, I'm worried 
about the patch to test/hufts that you also suggested, as it would mean 
gzip's diagnostics would differ on s390. As I understand it, test/hufts 
checks for invalid data not unexpected EOF, so for these tests gzip 
should say "invalid compressed data" rather than "unexpected end of 
file", which means that there's still a bug in the s390 port that 
relates to which diagnostic to give in this case.

Am I understanding this correctly?




Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Mon, 17 Feb 2025 10:39:01 GMT) Full text and rfc822 format available.

Message #25 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Eduard Stefes <Eduard.Stefes <at> ibm.com>
To: "eggert <at> cs.ucla.edu" <eggert <at> cs.ucla.edu>
Cc: "75924 <at> debbugs.gnu.org" <75924 <at> debbugs.gnu.org>,
 Eduard Stefes <Eduard.Stefes <at> ibm.com>, "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: RE: bug#75924: maint: fix s390 buffer flushes
Date: Mon, 17 Feb 2025 10:38:28 +0000
Hi,
On Thu, 2025-02-06 at 10:20 -0800, Paul Eggert wrote:
> On 2025-02-05 07:46, Eduard Stefes wrote:
> 
> > We think that the only
> > possibility for a problem here, is if faulty hardware updates
> > param->cf
> > incorrect.
> 
> I'd rather not worry about hardware going bad, unless the hardware 
> failure is a known real-world problem. So how about if we make the 
> following further patch?
> 
>    diff --git a/dfltcc.c b/dfltcc.c
>    index 9f86581..a360ce2 100644
>    --- a/dfltcc.c
>    +++ b/dfltcc.c
>    @@ -372,7 +372,7 @@ dfltcc_deflate (int pack_level)
>  /* Read the input data.  */
>  if (inptr == insize)
>     {
>    -          if (fill_inbuf (1) == EOF && !param->cf)
>    +          if (fill_inbuf (1) == EOF)
>         break;
>       inptr = 0;
>     }
> 
> 
We really need to check the cf(continuation flag) before we can return.
here a cite [1] from he spec: 
> ... when one, indicates the operation is partially complete ...

if we do not wait for the hardware to lower the flag, we risk to
corrupt and overwrite parts of the output buffer.


> 
> > I read rfc1952 and there is no explicit statement saying that
> > incomplete gz files *are corrupt or invalid*.
> 
> My reading is that the RFC does not specify any behavior for
> incomplete 
> gz files. However, gzip goes beyond what the RFC requires, and is 
> supposed to complain "unexpected end of file" for incomplete files, 
> versus "invalid compressed data--format violated" for files that are
> not 
> prefixes of valid compressed files. We should maintain that behavior
> as 
> it is valuable information to give to the user.
> 
> I installed the patch to dfltcc.c that you suggested in 
> <
> https://debbugs.gnu.org/c 
> gi_bugreport.cgi-3Fbug-3D75924-
> 235&d=DwICaQ&c=BSDicqBQBDjDI9RkVyTcHQ&r=9u0E1SPaj7JeThb7S74vZ9s-
> VX1c_JTZ3i5rEh89cLU&m=mYHcCIO2Z8sEAayQMo8pBmY3pxQ16wbf8KMW2NPiDknJMFz
> 4qEFFyhpCJvbRLyu9&s=VxHLexQ8YkPuK0dODBtBIOdJSZNN3UOOaFzlPLnFd7s&e= >,
> superseding the 
> incorrect patch I installed a couple of days ago. However, I'm
> worried 
> about the patch to test/hufts that you also suggested, as it would
> mean 
> gzip's diagnostics would differ on s390. As I understand it,
> test/hufts 
> checks for invalid data not unexpected EOF, so for these tests gzip 
> should say "invalid compressed data" rather than "unexpected end of 
> file", which means that there's still a bug in the s390 port that 
> relates to which diagnostic to give in this case.
> 
> Am I understanding this correctly?

this is a tricky one. the hardware will not start emitting proper OESCs
(Operation-Ending-Supplemental Code) before a complete huffman tree was
built up. Until then we have to rely on the CC (Condition Code).
However DFLTCC_CC_OP2_TOO_SHORT and DFLTCC_CC_OP2_CORRUPT are
overloaded[1]. 

I came up with an ugly hack: 
in case of an EOF we could check if bytes_out == 0 and then return
2(invalid data). If we already wrote some bytes we continue with the
EOF error code. This new *if-branch* would only be hit in this special
case, where parsing of the initial huffman tree failed on the hardware.

It would *not* affect the situation when we have corrupt data *after*
the initial huffman tree was parsed by the hardware, because that is
covered here[2].

This makes hufts pass and does not affect any other test case. I will
do more exhaustive tests just to be sure that nothing else is affected.

[1] dfltcc.c:42
[2] dfltcc.c:457-464


-- 
Eduard Stefes <eduard.stefes <at> ibm.com>




Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Mon, 17 Feb 2025 10:42:01 GMT) Full text and rfc822 format available.

Message #28 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eduard Stefes <Eduard.Stefes <at> ibm.com>
Cc: "75924 <at> debbugs.gnu.org" <75924 <at> debbugs.gnu.org>,
 "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: Re: bug#75924: maint: fix s390 buffer flushes
Date: Mon, 17 Feb 2025 02:40:54 -0800
On 2025-02-17 02:38, Eduard Stefes wrote:
> I came up with an ugly hack:

Thanks, it sounds much nicer than the hack I was thinking of, which was 
to redo the entire run in software....




Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Mon, 17 Feb 2025 10:45:02 GMT) Full text and rfc822 format available.

Message #31 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Eduard Stefes <eduard.stefes <at> ibm.com>
Cc: 75924 <at> debbugs.gnu.org, Andreas Hasenack <andreas.hasenack <at> canonical.com>,
 Eduard Stefes <eduard.stefes <at> ibm.com>, Ilya Leoshkevich <iii <at> linux.ibm.com>
Subject: gzip: add special eof handling for s390x
Date: Mon, 17 Feb 2025 11:44:09 +0100
due to hardware limitations we cannot rely on the OESC error code
before the hardware parsed the initial huffman tree. For this case
we have to add a branch to the EOF handling. we return an errorcode 2
(invalid data) if we where still in the initial phase (bytes_out == 0).
Otherwise we continue with the default EOF handling.
---
 dfltcc.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/dfltcc.c b/dfltcc.c
index 9f86581..e45b3ce 100644
--- a/dfltcc.c
+++ b/dfltcc.c
@@ -18,6 +18,7 @@
 #include <config.h>
 
 #include <stdlib.h>
+#include <errno.h>
 
 #ifdef HAVE_SYS_SDT_H
 # include <sys/sdt.h>
@@ -438,6 +439,8 @@ dfltcc_inflate ()
             {
               /* Premature EOF.  */
               flush_outbuf ();
+              if (bytes_out == 0)
+                return 2;
               errno = 0;
               read_error ();
             }
-- 
2.48.1





Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Tue, 18 Feb 2025 13:31:02 GMT) Full text and rfc822 format available.

Message #34 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Hasenack <andreas.hasenack <at> canonical.com>
To: Eduard Stefes <eduard.stefes <at> ibm.com>
Cc: 75924 <at> debbugs.gnu.org, Ilya Leoshkevich <iii <at> linux.ibm.com>
Subject: Re: gzip: add special eof handling for s390x
Date: Tue, 18 Feb 2025 10:30:12 -0300
Hi,

This is on top of the previous patch I presume? It doesn't have the
__builtin_unreachable() line the previous patch added, but I might
have missed an update.

On Mon, Feb 17, 2025 at 7:44 AM Eduard Stefes <eduard.stefes <at> ibm.com> wrote:
>
> due to hardware limitations we cannot rely on the OESC error code
> before the hardware parsed the initial huffman tree. For this case
> we have to add a branch to the EOF handling. we return an errorcode 2
> (invalid data) if we where still in the initial phase (bytes_out == 0).
> Otherwise we continue with the default EOF handling.
> ---
>  dfltcc.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/dfltcc.c b/dfltcc.c
> index 9f86581..e45b3ce 100644
> --- a/dfltcc.c
> +++ b/dfltcc.c
> @@ -18,6 +18,7 @@
>  #include <config.h>
>
>  #include <stdlib.h>
> +#include <errno.h>
>
>  #ifdef HAVE_SYS_SDT_H
>  # include <sys/sdt.h>
> @@ -438,6 +439,8 @@ dfltcc_inflate ()
>              {
>                /* Premature EOF.  */
>                flush_outbuf ();
> +              if (bytes_out == 0)
> +                return 2;
>                errno = 0;
>                read_error ();
>              }
> --
> 2.48.1
>




Information forwarded to bug-gzip <at> gnu.org:
bug#75924; Package gzip. (Thu, 20 Feb 2025 11:34:01 GMT) Full text and rfc822 format available.

Message #37 received at 75924 <at> debbugs.gnu.org (full text, mbox):

From: Eduard Stefes <Eduard.Stefes <at> ibm.com>
To: "eggert <at> cs.ucla.edu" <eggert <at> cs.ucla.edu>
Cc: "75924 <at> debbugs.gnu.org" <75924 <at> debbugs.gnu.org>,
 "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: RE: bug#75924: maint: fix s390 buffer flushes
Date: Thu, 20 Feb 2025 11:33:13 +0000
Hi,

On Mon, 2025-02-17 at 10:38 +0000, Eduard Stefes wrote:
> 
> ...
> This makes hufts pass and does not affect any other test case. I will
> do more exhaustive tests just to be sure that nothing else is
> affected.
> ...

So, I did some tests. I setup afl++ with a static seed and went through
some minutes of fuzzing on both x86 and s390x.

Results are as expected: 
- many tests(~50%) result in the same error message
- others result in different error messages
- I could not find a crash (note that i tested only a limited time)

at this stage I am hesitating to go forward and try to fix all cases
where hardware and software implementation generate different error
messages. 
I think we are at a solid state here, as both implementations run with
the same test suit. 
Investing more time to get a 100% error message overlap looks like a
bigger effort. We know that the hardware state machine does not simply
map the software, so I fear that it will result in very very cumbersome
error handling code.

> 
> 
> -- 
> Eduard Stefes <eduard.stefes <at> ibm.com>


Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Tue, 08 Apr 2025 17:56:02 GMT) Full text and rfc822 format available.

Notification sent to Eduard Stefes <eduard.stefes <at> ibm.com>:
bug acknowledged by developer. (Tue, 08 Apr 2025 17:56:03 GMT) Full text and rfc822 format available.

Message #42 received at 75924-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Eduard Stefes <Eduard.Stefes <at> ibm.com>
Cc: "75924 <at> debbugs.gnu.org" <75924-done <at> debbugs.gnu.org>,
 "iii <at> linux.ibm.com" <iii <at> linux.ibm.com>,
 "andreas.hasenack <at> canonical.com" <andreas.hasenack <at> canonical.com>
Subject: Re: bug#75924: maint: fix s390 buffer flushes
Date: Tue, 8 Apr 2025 10:55:29 -0700
On 2025-02-20 03:33, Eduard Stefes wrote:
> I think we are at a solid state here, as both implementations run with
> the same test suit.
> Investing more time to get a 100% error message overlap looks like a
> bigger effort. We know that the hardware state machine does not simply
> map the software, so I fear that it will result in very very cumbersome
> error handling code.

OK, thanks for the analysis. Closing the bug report.




Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Tue, 08 Apr 2025 17:56:03 GMT) Full text and rfc822 format available.

Notification sent to Eduard Stefes <eduard.stefes <at> ibm.com>:
bug acknowledged by developer. (Tue, 08 Apr 2025 17:56:03 GMT) Full text and rfc822 format available.

Reply sent to Paul Eggert <eggert <at> cs.ucla.edu>:
You have taken responsibility. (Tue, 08 Apr 2025 17:56:03 GMT) Full text and rfc822 format available.

Notification sent to Eduard Stefes <eduard.stefes <at> ibm.com>:
bug acknowledged by developer. (Tue, 08 Apr 2025 17:56:03 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 07 May 2025 11:24:09 GMT) Full text and rfc822 format available.

This bug report was last modified 41 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.