lzma_raw_coder_init() always calls lzma_next_end() if an error occurs
in filter initialization, and no other code path can call
lzma_simple_coder_init(), so this change doesn't fix any bug.
This is just to reduce likelyhood of future bugs.
Filter coders are initialized using lzma_next_filter_init(). If filter
chain initialization fails, the entire filter chain should be cleaned up
with lzma_next_end(). lzma_raw_encoder_init() and lzma_raw_decoder_init()
have always done this via lzma_raw_coder_init() in filter_common.c.
(Separate cleanup is weird, but it must have made sense to the young me.)
The following decoders initialize LZMA1 filter directly without using the
raw filter chain API. This avoids needlessly pulling in all other filters
when a program is linked against static liblzma. These functions didn't
call lzma_next_end() after an initialization error, which left the state
available for later reinitialization.
- lzma_alone_decoder()
- lzma_lzip_decoder()
- lzma_auto_decoder() [*]
- lzma_microlzma_decoder()
[*] lzma_auto_decoder() is only indirectly affected due to the first two
functions. lzma_auto_decoder() itself doesn't need a fix.
There are also encoder functions that initialize the LZMA1 encoder
directly. They don't have this issue because the whole lzma_stream
is cleaned up when encoder initialization fails.
Reported-by: GitHub user christos-cantina-security (christos-spearbit)
If memory allocation fails and the resulting coder state is reused,
ensure that memory allocation is attempted again. However, coders that
are initialized via lzma_next_filter_init() shouldn't be reinitialized
after failure; they should be cleaned up with lzma_next_end().
Reported-by: GitHub user christos-cantina-security (christos-spearbit)
Worker threads update coder->progress_in while holding coder->mutex, but
the main thread updated the same field without the mutex while decoding
the next Block Header. ThreadSanitizer reported the concurrent writes and
lzma_get_progress() under-reported input progress by one Block.
All other coder->progress_in/progress_out accesses in stream_decode_mt()
are done when the worker threads aren't active, so only the access in
SEQ_BLOCK_HEADER needs to lock coder->mutex.
Co-authored-by: Lasse Collin <lasse.collin@tukaani.org>
Fixes: 4cce3e27f529 ("liblzma: Add threaded .xz decompressor.")
Closes: https://github.com/tukaani-project/xz/pull/243
It wasn't really a bug, but it's good to be more strict.
Side effects ("downsides" would be a too strong word):
- Now the LZMA decoder has to use the slow path at the end of every
LZMA2 chunk even when the read position isn't close to &in[in_size].
This doesn't matter in practice.
- If a chunk header was corrupt exactly so that the compressed size is
too small, now fewer bytes of uncompressed data can be recovered.
Reported-by: GitHub user christos-spearbit
If at least three files are passed to xz --list, the totals might exceed
the range of uint64_t. Previously wrong totals were displayed in this
case. Now an error is shown as soon as an overflow would occur, and the
remaining files aren't processed. In most other cases an error doesn't
prevent xz from processing the remaining files, but in this case it
feels OK because (1) the error should be very rare, (2) it keeps the
code slightly simpler, and (3) with xz --robot --list the caller will
likely ignore the output anyway when xz exits with a non-zero status.
Co-authored-by: Lasse Collin <lasse.collin@tukaani.org>
Fixes: https://github.com/tukaani-project/xz/pull/240
lzma_index_cat() is used by lzma_file_info_decoder() and by a few apps
that were written before lzma_file_info_decoder() existed (it was added
in 5.3.1alpha). The most common use case for these functions is xz --list.
The other use case is random access decoding.
When decoding Index fields from multi-Stream .xz files, lzma_index_cat()
is used to concatenate the decoded lzma_index structures. Such decoding
starts from the last Stream in the file and stops at the first Stream in
the file, so lzma_index_cat() is called with the most recently decoded
Stream as the destination argument while the source argument contains all
other Streams that have been decoded so far. That is, the typical use case
is lzma_index_cat(one_stream, many_streams, NULL).
With the old code, lzma_index_cat() appended Streams from src into dest
one at a time and updated their absolute offsets at the same time.
When a .xz file contained many Streams, the number of Streams in
the source argument grew for each call, which created a performance
problem if the file contained an unusually large number of Streams
(over 10,000). (A large number of Blocks isn't a problem.)
Make lzma_index_cat() prepend the index_streams from dest to src
instead of appending from src to dest. Use the _bias members to adjust
the offsets of the existing index_streams in src.
Change the numbering in index_stream.number from 1, 2, 3, ... to
...., 2, 1, 0. That is, internally the last index_stream is now 0.
Now lzma_index_cat() needs to update all Streams from dest instead from
src, which keeps the normal use case fast. Obviously the performance
problem still exists, but now it's in the opposite situation (repeated
calls of lzma_index_cat(many_streams, one_stream, NULL)). This should
be acceptable because there should be no use cases where doing it would
be useful. A full fix would require a more complex patch.
As an alternative, I tried a rewrite of index.c that replaces the trees
with plain arrays and realloc(). It's smaller and simpler, but it's
still somewhat complex because the API is too flexible compared to what
functionality is actually needed. I didn't merge that version because
a rewrite has a higher risk of regressions, and in some corner cases
the peak memory usage was considerably higher than with the old code
(if a custom lzma_allocator is used, realloc() has to be emulated with
alloc + memcpy + free, and native realloc() might need to do that too),
and large preallocations make it unclear what lzma_index_memusage() and
lzma_index_memused() should return.
Below is an example where xz --list became too slow before this commit.
Pass 18 or 20 instead of 16 to seq, and xz effectively hangs.
true | xz > big.xz
for I in $(seq 16)
do
cat big.xz big.xz > tmp.xz
mv tmp.xz big.xz
echo ====
echo $I
time xz -l big.xz
done
Reported-by: Omkhar Arasaratnam
This can matter on Windows:
- MSVC puts the binaries into subdirectories based on build type
(like "Debug" or "Release"), and then the test executables won't
find liblzma.dll because it's not in the working directory from
which the tests are run.
- With mingw-w64, liblzma.dll was in the working directory so the test
executables worked. However, the if the system happened to have
liblzma.dll in, for example, the Windows directory, then the DLL
search order might pick that instead.
Putting the shared library and test executables in the same directory
avoids these problems.
Reported-by: Frank <mail@franku.nl>
Fixes: https://github.com/tukaani-project/xz/pull/239
Previously, if an Index had more than PREALLOC_MAX Records, then
allocations were done in PREALLOC_DEFAULT chunks (512 Records).
That makes little sense in practice because it's likely that one
would run out of address space later. Return LZMA_MEM_ERROR error
pre-emptively if PREALLOC_MAX would be exceeded.
lzma_index_dup() copied the per-Stream flags but not the cached bitmask
for all Streams except the last one. As a result, lzma_index_checks() on
a duplicated multi-Stream Index reported only the Check type of its
final Stream.
Fixes: https://github.com/tukaani-project/xz/pull/238
If *in_pos > 0, the condition became more relaxed than it needed to be.
Fixes: 5005914f1acf ("liblzma: Index decoder: Reject an obviously-bad Number of>
Fixes: 0352dc7929b1 ("liblzma: Silence a compiler warning")
