public inbox for bzip2-devel@sourceware.org
 help / color / mirror / Atom feed
From: Mark Wielaard <mark@klomp.org>
To: jseward@acm.org, Federico Mena Quintero <federico@gnome.org>,
	 bzip2-devel@sourceware.org
Subject: Alternative nSelectors patch (Was: bzip2 1.0.7 released)
Date: Tue, 01 Jan 2019 00:00:00 -0000	[thread overview]
Message-ID: <308d9e82220760205ee673bf0505ee1815d48596.camel@klomp.org> (raw)
In-Reply-To: <9998ca428c4c7f895a543aa91941e58efb0d5291.camel@klomp.org>

[-- Attachment #1: Type: text/plain, Size: 1021 bytes --]

Hi,

On Fri, 2019-06-28 at 13:10 +0200, Mark Wielaard wrote:
> > It seems to me to be important to now split BZ_MAX_SELECTORS into these two
> > parts so as to make it clear to everybody that we're accepting (decompressing)
> > a slightly larger set of inputs than we create (a la that old saying about
> > network protocol implementations), so as to tolerate other compressors.
> 
> That seems good. The attached patch does this and makes it possible to
> decode the problematic bz2 file.

Sorry, it is a bit too late here to properly document this patch and
explain why I think it is a better one than the "split-max-selectors"
fix. But hopefully the new testsuite example and the comment in the
patch make clear what my thinking is.

This resolved both the issue with the large file reported as with the
new test suite file (lbzip2/32767.bz2). The whole testsuite passes now,
even under valgrind and with gcc -fsanitize=undefined.

Comments on the patch idea more than welcome.

Thanks,

Mark

[-- Attachment #2: limit-nSelectors-processing.patch --]
[-- Type: text/x-patch, Size: 1523 bytes --]

diff --git a/compress.c b/compress.c
index 237620d..76adee6 100644
--- a/compress.c
+++ b/compress.c
@@ -454,7 +454,7 @@ void sendMTFValues ( EState* s )
 
    AssertH( nGroups < 8, 3002 );
    AssertH( nSelectors < 32768 &&
-            nSelectors <= (2 + (900000 / BZ_G_SIZE)),
+            nSelectors <= BZ_MAX_SELECTORS,
             3003 );
 
 
diff --git a/decompress.c b/decompress.c
index 20ce493..3303499 100644
--- a/decompress.c
+++ b/decompress.c
@@ -287,7 +287,7 @@ Int32 BZ2_decompress ( DState* s )
       GET_BITS(BZ_X_SELECTOR_1, nGroups, 3);
       if (nGroups < 2 || nGroups > BZ_N_GROUPS) RETURN(BZ_DATA_ERROR);
       GET_BITS(BZ_X_SELECTOR_2, nSelectors, 15);
-      if (nSelectors < 1 || nSelectors > BZ_MAX_SELECTORS) RETURN(BZ_DATA_ERROR);
+      if (nSelectors < 1) RETURN(BZ_DATA_ERROR);
       for (i = 0; i < nSelectors; i++) {
          j = 0;
          while (True) {
@@ -296,8 +296,14 @@ Int32 BZ2_decompress ( DState* s )
             j++;
             if (j >= nGroups) RETURN(BZ_DATA_ERROR);
          }
-         s->selectorMtf[i] = j;
+         /* Having more than BZ_MAX_SELECTORS doesn't make much sense
+            since they will never be used, but some implementations might
+            "round up" the number of selectors, so just ignore those. */
+         if (i < BZ_MAX_SELECTORS)
+           s->selectorMtf[i] = j;
       }
+      if (nSelectors > BZ_MAX_SELECTORS)
+        nSelectors = BZ_MAX_SELECTORS;
 
       /*--- Undo the MTF values for the selectors. ---*/
       {

  parent reply	other threads:[~2019-06-30 23:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-01  0:00 bzip2 1.0.7 released Mark Wielaard
2019-01-01  0:00 ` Mark Wielaard
2019-01-01  0:00   ` Jeffrey Walton
2019-01-01  0:00   ` Mark Wielaard
2019-01-01  0:00     ` Federico Mena Quintero
2019-01-01  0:00       ` Julian Seward
2019-01-01  0:00         ` Mark Wielaard
2019-01-01  0:00           ` Mark Wielaard
2019-01-01  0:00             ` Federico Mena Quintero
2019-01-01  0:00               ` Mark Wielaard
2019-01-01  0:00           ` bzip2 test suite (Was: bzip2 1.0.7 released) Mark Wielaard
2019-01-01  0:00           ` Mark Wielaard [this message]
2019-01-01  0:00             ` Alternative nSelectors patch " Julian Seward
2019-01-01  0:00               ` Mark Wielaard
2019-01-01  0:00                 ` Mark Wielaard
2019-01-01  0:00   ` bzip2 1.0.7 released Federico Mena Quintero

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=308d9e82220760205ee673bf0505ee1815d48596.camel@klomp.org \
    --to=mark@klomp.org \
    --cc=bzip2-devel@sourceware.org \
    --cc=federico@gnome.org \
    --cc=jseward@acm.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).