Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

lib/xz: Avoid overlapping memcpy() with invalid input with in-place decompression

With valid files, the safety margin described in lib/decompress_unxz.c
ensures that these buffers cannot overlap. But if the uncompressed size
of the input is larger than the caller thought, which is possible when
the input file is invalid/corrupt, the buffers can overlap. Obviously
the result will then be garbage (and usually the decoder will return
an error too) but no other harm will happen when such an over-run occurs.

This change only affects uncompressed LZMA2 chunks and so this
should have no effect on performance.

Link: https://lore.kernel.org/r/20211010213145.17462-2-xiang@kernel.org
Signed-off-by: Lasse Collin <lasse.collin@tukaani.org>
Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>

authored by

Lasse Collin and committed by
Gao Xiang
83d3c4f2 38629291

+20 -3
+1 -1
lib/decompress_unxz.c
··· 167 167 * memeq and memzero are not used much and any remotely sane implementation 168 168 * is fast enough. memcpy/memmove speed matters in multi-call mode, but 169 169 * the kernel image is decompressed in single-call mode, in which only 170 - * memcpy speed can matter and only if there is a lot of uncompressible data 170 + * memmove speed can matter and only if there is a lot of uncompressible data 171 171 * (LZMA2 stores uncompressible chunks in uncompressed form). Thus, the 172 172 * functions below should just be kept small; it's probably not worth 173 173 * optimizing for speed.
+19 -2
lib/xz/xz_dec_lzma2.c
··· 387 387 388 388 *left -= copy_size; 389 389 390 - memcpy(dict->buf + dict->pos, b->in + b->in_pos, copy_size); 390 + /* 391 + * If doing in-place decompression in single-call mode and the 392 + * uncompressed size of the file is larger than the caller 393 + * thought (i.e. it is invalid input!), the buffers below may 394 + * overlap and cause undefined behavior with memcpy(). 395 + * With valid inputs memcpy() would be fine here. 396 + */ 397 + memmove(dict->buf + dict->pos, b->in + b->in_pos, copy_size); 391 398 dict->pos += copy_size; 392 399 393 400 if (dict->full < dict->pos) ··· 404 397 if (dict->pos == dict->end) 405 398 dict->pos = 0; 406 399 407 - memcpy(b->out + b->out_pos, b->in + b->in_pos, 400 + /* 401 + * Like above but for multi-call mode: use memmove() 402 + * to avoid undefined behavior with invalid input. 403 + */ 404 + memmove(b->out + b->out_pos, b->in + b->in_pos, 408 405 copy_size); 409 406 } 410 407 ··· 432 421 if (dict->pos == dict->end) 433 422 dict->pos = 0; 434 423 424 + /* 425 + * These buffers cannot overlap even if doing in-place 426 + * decompression because in multi-call mode dict->buf 427 + * has been allocated by us in this file; it's not 428 + * provided by the caller like in single-call mode. 429 + */ 435 430 memcpy(b->out + b->out_pos, dict->buf + dict->start, 436 431 copy_size); 437 432 }