liblzma: mt dec: Don't modify thr->in_size in the worker thread

Don't set thr->in_size = 0 when returning the thread to the stack of
available threads. Not only is it useless, but the main thread may
read the value in SEQ_BLOCK_THR_RUN. With valid inputs, it made
no difference if the main thread saw the original value or 0. With
invalid inputs (when worker thread stops early), thr->in_size was
no longer modified after the previous commit with the security fix
("Don't free the input buffer too early").

So while the bug appears harmless now, it's important to fix it because
the variable was being modified without proper locking. It's trivial
to fix because there is no need to change the value. Only main thread
needs to set the value in (in SEQ_BLOCK_THR_INIT) when starting a new
Block before the worker thread is activated.

Fixes: 4cce3e27f529 ("liblzma: Add threaded .xz decompressor.")
Reviewed-by: Sebastian Andrzej Siewior <sebastian@breakpoint.cc>
Thanks-to: Sam James <sam@gentoo.org>

diff --git a/src/liblzma/common/stream_decoder_mt.c b/src/liblzma/common/stream_decoder_mt.c
index 98aabcff2afefa6f173c4d801d9400528ff7c7d4..1fa92220568dcc3bb1a1f58229a6bd4589e26003 100644 (file)
--- a/src/liblzma/common/stream_decoder_mt.c
+++ b/src/liblzma/common/stream_decoder_mt.c
@@ -491,8 +491,6 @@ next_loop_unlocked:
                if (ret == LZMA_STREAM_END) {
                        // Update memory usage counters.
                        thr->coder->mem_in_use -= thr->in_size;
-                       thr->in_size = 0; // thr->in was freed above.
-
                        thr->coder->mem_in_use -= thr->mem_filters;
                        thr->coder->mem_cached += thr->mem_filters;
 
@@ -1554,6 +1552,10 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
                }
 
                // Return if the input didn't contain the whole Block.
+               //
+               // NOTE: When we updated coder->thr->in_filled a few lines
+               // above, the worker thread might by now have finished its
+               // work and returned itself back to the stack of free threads.
                if (coder->thr->in_filled < coder->thr->in_size) {
                        assert(*in_pos == in_size);
                        return LZMA_OK;