Bug 2689

Summary: unlzma loops
Product: Busybox Reporter: dborca <dborca>
Component: OtherAssignee: unassigned
Status: RESOLVED FIXED    
Severity: minor CC: busybox-cvs, dborca
Priority: P5    
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: Linux   
Host: Target:
Build:
Attachments: unlzma buffer_pos fix

Description dborca 2010-10-08 21:06:36 UTC
Hi,

I just noticed that there's a wierd condition in a nested loop inside decompress_unlzma.c.  To be more precise, the lzma main decoding looks like this:

while (global_pos + buffer_pos < header.dst_size) {
  ...
  do {
    ...
  } while (len != 0 && buffer_pos < header.dst_size);
}

I'm interested in the do { } while condition (busybox-1.17.2.tar.bz2,
archival/libunarchive/decompress_unlzma.c, around line 449).  That code
is in busybox since the beginning of time.

buffer_pos is guaranteed to be inside [0 .. header.dict_size) interval
at all times, and its comparison against header.dst_size seems meaningless.
The total number of written bytes is given by global_pos + buffer_pos;
(see main while loop).
Comment 1 Denys Vlasenko 2010-10-09 11:38:02 UTC
> buffer_pos is guaranteed to be inside [0 .. header.dict_size) interval

Maybe. It's not exactly obvious from the code.

More to the point: what do you want to be done with this code?
Comment 2 dborca 2010-10-09 12:28:20 UTC
Created attachment 2593 [details]
unlzma buffer_pos fix
Comment 3 dborca 2010-10-09 12:30:04 UTC
From what I've seen, buffer_pos only changes this way:

buffer[buffer_pos++] = previous_byte;
if (buffer_pos == header.dict_size) {
  buffer_pos = 0;
  /* flush dictionary */
}

and it looks like header.dst_size should be compared against (global_pos + buffer_pos), not against buffer_pos (see attached patch).

Or maybe I am just missing something.
Comment 4 Denys Vlasenko 2014-02-28 14:44:14 UTC
Added comments to address this easier - if it's a real bug:

commit 81071e6872eeb9e47b938d5d6fd82056aaebdd2e
Author: Denys Vlasenko <vda.linux@googlemail.com>
Date:   Fri Feb 28 15:42:10 2014 +0100

    unlzma: add comments about possible bug from BZ 2689