mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
@ 2026-09-12 18:17 Matt Turner
  2026-09-14  2:07 ` Andrew Morton
  2026-09-14  2:09 ` Andrew Morton
  0 siblings, 2 replies; 8+ messages in thread
From: Matt Turner @ 2026-09-12 18:17 UTC (permalink / raw)
  To: Andrew Morton, H. Peter Anvin, Alain Knaff
  Cc: linux-kernel, stable, Matt Turner

The run-length path rejects a block when dbufCount+t equals dbufSize,
but the loop that follows writes exactly t bytes starting at dbufCount,
so a block that fills the buffer exactly is legal. bzip2 allows it too:
its decompressor bounds a block at 100000 * blockSize100k and checks
that limit per byte appended. Use > instead of >=.

bzip2's encoder stops filling a block 19 bytes early, so nothing it
produces ever reaches the limit and the bug stays hidden. Compressors
that use the full block size do reach it: an lbzip2 -9 image whose block
ends on a run fails to decode, and a self-extracting kernel built that
way does not boot.

This code came from busybox, which fixed the same line in 2013 in commit
932e233a491b ("bunzip2: fix off-by-one check").

Fixes: bc22c17e12c1 ("bzip2/lzma: library support for gzip, bzip2 and lzma decompression")
Cc: stable@vger.kernel.org
Signed-off-by: Matt Turner <mattst88@gmail.com>
---
lib/decompress_bunzip2: fix off-by-one in run-length bounds check
---
 lib/decompress_bunzip2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/decompress_bunzip2.c b/lib/decompress_bunzip2.c
index 1288f146661f..aaa75404250e 100644
--- a/lib/decompress_bunzip2.c
+++ b/lib/decompress_bunzip2.c
@@ -439,7 +439,7 @@ static int INIT get_next_block(struct bunzip_data *bd)
 		   array.) */
 		if (runPos) {
 			runPos = 0;
-			if (dbufCount+t >= dbufSize)
+			if (dbufCount+t > dbufSize)
 				return RETVAL_DATA_ERROR;
 
 			uc = symToByte[mtfSymbol[0]];

---
base-commit: 893e11787f78e43b534e252249ac3fff4d1333f8
change-id: 20260912-b4-bunzip2-blocksize-fix-7333376abd40

Best regards,
-- 
Matt Turner <mattst88@gmail.com>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-12 18:17 [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check Matt Turner
@ 2026-09-14  2:07 ` Andrew Morton
  2026-09-14  2:16   ` Matt Turner
  2026-09-14  2:09 ` Andrew Morton
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2026-09-14  2:07 UTC (permalink / raw)
  To: Matt Turner; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:

> The run-length path rejects a block when dbufCount+t equals dbufSize,
> but the loop that follows writes exactly t bytes starting at dbufCount,
> so a block that fills the buffer exactly is legal. bzip2 allows it too:
> its decompressor bounds a block at 100000 * blockSize100k and checks
> that limit per byte appended. Use > instead of >=.
> 
> bzip2's encoder stops filling a block 19 bytes early, so nothing it
> produces ever reaches the limit and the bug stays hidden. Compressors
> that use the full block size do reach it: an lbzip2 -9 image whose block
> ends on a run fails to decode, and a self-extracting kernel built that
> way does not boot.
> 
> This code came from busybox, which fixed the same line in 2013 in commit
> 932e233a491b ("bunzip2: fix off-by-one check").

Thanks.

> Fixes: bc22c17e12c1 ("bzip2/lzma: library support for gzip, bzip2 and lzma decompression")
> Cc: stable@vger.kernel.org

Why is a backport proposed?  Hopefully because downstream users need
this change, but the changelog doesn't tell anyone the reasons why.



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-12 18:17 [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check Matt Turner
  2026-09-14  2:07 ` Andrew Morton
@ 2026-09-14  2:09 ` Andrew Morton
  2026-09-14  2:18   ` Matt Turner
  1 sibling, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2026-09-14  2:09 UTC (permalink / raw)
  To: Matt Turner; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:

> The run-length path rejects a block when dbufCount+t equals dbufSize,
> but the loop that follows writes exactly t bytes starting at dbufCount,
> so a block that fills the buffer exactly is legal. bzip2 allows it too:
> its decompressor bounds a block at 100000 * blockSize100k and checks
> that limit per byte appended. Use > instead of >=.
> 
> bzip2's encoder stops filling a block 19 bytes early, so nothing it
> produces ever reaches the limit and the bug stays hidden. Compressors
> that use the full block size do reach it: an lbzip2 -9 image whose block
> ends on a run fails to decode, and a self-extracting kernel built that
> way does not boot.
> 
> This code came from busybox, which fixed the same line in 2013 in commit
> 932e233a491b ("bunzip2: fix off-by-one check").
> 
>
> ...
>
> -			if (dbufCount+t >= dbufSize)
> +			if (dbufCount+t > dbufSize)

Sashiko thinks there's also a potential overflow here which could be
addressed in this patch.

	https://sashiko.dev/#/patchset/20260912-b4-bunzip2-blocksize-fix-v1-1-c7384bbfc954@gmail.com

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-14  2:07 ` Andrew Morton
@ 2026-09-14  2:16   ` Matt Turner
  2026-09-14  2:46     ` Andrew Morton
  0 siblings, 1 reply; 8+ messages in thread
From: Matt Turner @ 2026-09-14  2:16 UTC (permalink / raw)
  To: Andrew Morton; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sun, Sep 13, 2026 at 10:07 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:
>
> > The run-length path rejects a block when dbufCount+t equals dbufSize,
> > but the loop that follows writes exactly t bytes starting at dbufCount,
> > so a block that fills the buffer exactly is legal. bzip2 allows it too:
> > its decompressor bounds a block at 100000 * blockSize100k and checks
> > that limit per byte appended. Use > instead of >=.
> >
> > bzip2's encoder stops filling a block 19 bytes early, so nothing it
> > produces ever reaches the limit and the bug stays hidden. Compressors
> > that use the full block size do reach it: an lbzip2 -9 image whose block
> > ends on a run fails to decode, and a self-extracting kernel built that
> > way does not boot.
> >
> > This code came from busybox, which fixed the same line in 2013 in commit
> > 932e233a491b ("bunzip2: fix off-by-one check").
>
> Thanks.
>
> > Fixes: bc22c17e12c1 ("bzip2/lzma: library support for gzip, bzip2 and lzma decompression")
> > Cc: stable@vger.kernel.org
>
> Why is a backport proposed?  Hopefully because downstream users need
> this change, but the changelog doesn't tell anyone the reasons why.

I ran into this because my system uses lbzip2 as /bin/bzip2 -- a
common thing on Gentoo I believe. As far as I can tell, anyone using
lbzip2 as their system bzip2 would run into this and it's only because
it's very uncommon these days to compress a kernel with bzip2 that no
one has noticed. I only noticed because I was adding support for
various compression formats on alpha.

No strong preference for a backport from me. Just thought it would be desirable.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-14  2:09 ` Andrew Morton
@ 2026-09-14  2:18   ` Matt Turner
  2026-09-14  2:49     ` Andrew Morton
  0 siblings, 1 reply; 8+ messages in thread
From: Matt Turner @ 2026-09-14  2:18 UTC (permalink / raw)
  To: Andrew Morton; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sun, Sep 13, 2026 at 10:09 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:
>
> > The run-length path rejects a block when dbufCount+t equals dbufSize,
> > but the loop that follows writes exactly t bytes starting at dbufCount,
> > so a block that fills the buffer exactly is legal. bzip2 allows it too:
> > its decompressor bounds a block at 100000 * blockSize100k and checks
> > that limit per byte appended. Use > instead of >=.
> >
> > bzip2's encoder stops filling a block 19 bytes early, so nothing it
> > produces ever reaches the limit and the bug stays hidden. Compressors
> > that use the full block size do reach it: an lbzip2 -9 image whose block
> > ends on a run fails to decode, and a self-extracting kernel built that
> > way does not boot.
> >
> > This code came from busybox, which fixed the same line in 2013 in commit
> > 932e233a491b ("bunzip2: fix off-by-one check").
> >
> >
> > ...
> >
> > -                     if (dbufCount+t >= dbufSize)
> > +                     if (dbufCount+t > dbufSize)
>
> Sashiko thinks there's also a potential overflow here which could be
> addressed in this patch.
>
>         https://sashiko.dev/#/patchset/20260912-b4-bunzip2-blocksize-fix-v1-1-c7384bbfc954@gmail.com

What's the appropriate flow to/from busybox (from which this code
originates)? Apparently we don't sync from busybox, but do we want to
avoid divergences?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-14  2:16   ` Matt Turner
@ 2026-09-14  2:46     ` Andrew Morton
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Morton @ 2026-09-14  2:46 UTC (permalink / raw)
  To: Matt Turner; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sun, 13 Sep 2026 22:16:16 -0400 Matt Turner <mattst88@gmail.com> wrote:

> On Sun, Sep 13, 2026 at 10:07 PM Andrew Morton
> <akpm@linux-foundation.org> wrote:
> >
> > On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:
> >
> > > The run-length path rejects a block when dbufCount+t equals dbufSize,
> > > but the loop that follows writes exactly t bytes starting at dbufCount,
> > > so a block that fills the buffer exactly is legal. bzip2 allows it too:
> > > its decompressor bounds a block at 100000 * blockSize100k and checks
> > > that limit per byte appended. Use > instead of >=.
> > >
> > > bzip2's encoder stops filling a block 19 bytes early, so nothing it
> > > produces ever reaches the limit and the bug stays hidden. Compressors
> > > that use the full block size do reach it: an lbzip2 -9 image whose block
> > > ends on a run fails to decode, and a self-extracting kernel built that
> > > way does not boot.
> > >
> > > This code came from busybox, which fixed the same line in 2013 in commit
> > > 932e233a491b ("bunzip2: fix off-by-one check").
> >
> > Thanks.
> >
> > > Fixes: bc22c17e12c1 ("bzip2/lzma: library support for gzip, bzip2 and lzma decompression")
> > > Cc: stable@vger.kernel.org
> >
> > Why is a backport proposed?  Hopefully because downstream users need
> > this change, but the changelog doesn't tell anyone the reasons why.
> 
> I ran into this because my system uses lbzip2 as /bin/bzip2 -- a
> common thing on Gentoo I believe. As far as I can tell, anyone using
> lbzip2 as their system bzip2 would run into this and it's only because
> it's very uncommon these days to compress a kernel with bzip2 that no
> one has noticed. I only noticed because I was adding support for
> various compression formats on alpha.

OK, thanks.  A real-world i-hit-this is compelling.

> No strong preference for a backport from me. Just thought it would be desirable.

np, I;ll retain the cc:stable.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-14  2:18   ` Matt Turner
@ 2026-09-14  2:49     ` Andrew Morton
  2026-09-14  3:04       ` Matt Turner
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Morton @ 2026-09-14  2:49 UTC (permalink / raw)
  To: Matt Turner; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sun, 13 Sep 2026 22:18:07 -0400 Matt Turner <mattst88@gmail.com> wrote:

> On Sun, Sep 13, 2026 at 10:09 PM Andrew Morton
> <akpm@linux-foundation.org> wrote:
> >
> > On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:
> >
> > > The run-length path rejects a block when dbufCount+t equals dbufSize,
> > > but the loop that follows writes exactly t bytes starting at dbufCount,
> > > so a block that fills the buffer exactly is legal. bzip2 allows it too:
> > > its decompressor bounds a block at 100000 * blockSize100k and checks
> > > that limit per byte appended. Use > instead of >=.
> > >
> > > bzip2's encoder stops filling a block 19 bytes early, so nothing it
> > > produces ever reaches the limit and the bug stays hidden. Compressors
> > > that use the full block size do reach it: an lbzip2 -9 image whose block
> > > ends on a run fails to decode, and a self-extracting kernel built that
> > > way does not boot.
> > >
> > > This code came from busybox, which fixed the same line in 2013 in commit
> > > 932e233a491b ("bunzip2: fix off-by-one check").
> > >
> > >
> > > ...
> > >
> > > -                     if (dbufCount+t >= dbufSize)
> > > +                     if (dbufCount+t > dbufSize)
> >
> > Sashiko thinks there's also a potential overflow here which could be
> > addressed in this patch.
> >
> >         https://sashiko.dev/#/patchset/20260912-b4-bunzip2-blocksize-fix-v1-1-c7384bbfc954@gmail.com
> 
> What's the appropriate flow to/from busybox (from which this code
> originates)? Apparently we don't sync from busybox, but do we want to
> avoid divergences?

I'm not aware of any such process for any of the lib/ material which
mirrors some userspace project.  So I guess it's an ad-hoc "send them
an email" thing.  There's clearly risk that if we accept a new drop
from upstream, such kernel-first fixes will get lost :(




^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check
  2026-09-14  2:49     ` Andrew Morton
@ 2026-09-14  3:04       ` Matt Turner
  0 siblings, 0 replies; 8+ messages in thread
From: Matt Turner @ 2026-09-14  3:04 UTC (permalink / raw)
  To: Andrew Morton; +Cc: H. Peter Anvin, Alain Knaff, linux-kernel, stable

On Sun, Sep 13, 2026 at 10:49 PM Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> On Sun, 13 Sep 2026 22:18:07 -0400 Matt Turner <mattst88@gmail.com> wrote:
>
> > On Sun, Sep 13, 2026 at 10:09 PM Andrew Morton
> > <akpm@linux-foundation.org> wrote:
> > >
> > > On Sat, 12 Sep 2026 14:17:35 -0400 Matt Turner <mattst88@gmail.com> wrote:
> > >
> > > > The run-length path rejects a block when dbufCount+t equals dbufSize,
> > > > but the loop that follows writes exactly t bytes starting at dbufCount,
> > > > so a block that fills the buffer exactly is legal. bzip2 allows it too:
> > > > its decompressor bounds a block at 100000 * blockSize100k and checks
> > > > that limit per byte appended. Use > instead of >=.
> > > >
> > > > bzip2's encoder stops filling a block 19 bytes early, so nothing it
> > > > produces ever reaches the limit and the bug stays hidden. Compressors
> > > > that use the full block size do reach it: an lbzip2 -9 image whose block
> > > > ends on a run fails to decode, and a self-extracting kernel built that
> > > > way does not boot.
> > > >
> > > > This code came from busybox, which fixed the same line in 2013 in commit
> > > > 932e233a491b ("bunzip2: fix off-by-one check").
> > > >
> > > >
> > > > ...
> > > >
> > > > -                     if (dbufCount+t >= dbufSize)
> > > > +                     if (dbufCount+t > dbufSize)
> > >
> > > Sashiko thinks there's also a potential overflow here which could be
> > > addressed in this patch.
> > >
> > >         https://sashiko.dev/#/patchset/20260912-b4-bunzip2-blocksize-fix-v1-1-c7384bbfc954@gmail.com
> >
> > What's the appropriate flow to/from busybox (from which this code
> > originates)? Apparently we don't sync from busybox, but do we want to
> > avoid divergences?
>
> I'm not aware of any such process for any of the lib/ material which
> mirrors some userspace project.  So I guess it's an ad-hoc "send them
> an email" thing.  There's clearly risk that if we accept a new drop
> from upstream, such kernel-first fixes will get lost :(

Okay, thanks. I'll investigate the Sashiko report and follow up with
busybox upstream and then circle back to LKML if there's something we
need to address here.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-14  3:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-12 18:17 [PATCH] lib/decompress_bunzip2: fix off-by-one in run-length bounds check Matt Turner
2026-09-14  2:07 ` Andrew Morton
2026-09-14  2:16   ` Matt Turner
2026-09-14  2:46     ` Andrew Morton
2026-09-14  2:09 ` Andrew Morton
2026-09-14  2:18   ` Matt Turner
2026-09-14  2:49     ` Andrew Morton
2026-09-14  3:04       ` Matt Turner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®