| From 3cf78d0ef72f5cafda0c8774cce1dcd468f24e5f Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 11 Oct 2021 05:31:39 +0800 |
| Subject: lib/xz: Avoid overlapping memcpy() with invalid input with in-place |
| decompression |
| |
| From: Lasse Collin <lasse.collin@tukaani.org> |
| |
| [ Upstream commit 83d3c4f22a36d005b55f44628f46cc0d319a75e8 ] |
| |
| 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> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| lib/decompress_unxz.c | 2 +- |
| lib/xz/xz_dec_lzma2.c | 21 +++++++++++++++++++-- |
| 2 files changed, 20 insertions(+), 3 deletions(-) |
| |
| diff --git a/lib/decompress_unxz.c b/lib/decompress_unxz.c |
| index 25d59a95bd668..abea25310ac73 100644 |
| --- a/lib/decompress_unxz.c |
| +++ b/lib/decompress_unxz.c |
| @@ -167,7 +167,7 @@ |
| * memeq and memzero are not used much and any remotely sane implementation |
| * is fast enough. memcpy/memmove speed matters in multi-call mode, but |
| * the kernel image is decompressed in single-call mode, in which only |
| - * memcpy speed can matter and only if there is a lot of uncompressible data |
| + * memmove speed can matter and only if there is a lot of uncompressible data |
| * (LZMA2 stores uncompressible chunks in uncompressed form). Thus, the |
| * functions below should just be kept small; it's probably not worth |
| * optimizing for speed. |
| diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c |
| index 156f26fdc4c91..dd80989ca5a6b 100644 |
| --- a/lib/xz/xz_dec_lzma2.c |
| +++ b/lib/xz/xz_dec_lzma2.c |
| @@ -387,7 +387,14 @@ static void dict_uncompressed(struct dictionary *dict, struct xz_buf *b, |
| |
| *left -= copy_size; |
| |
| - memcpy(dict->buf + dict->pos, b->in + b->in_pos, copy_size); |
| + /* |
| + * If doing in-place decompression in single-call mode and the |
| + * uncompressed size of the file is larger than the caller |
| + * thought (i.e. it is invalid input!), the buffers below may |
| + * overlap and cause undefined behavior with memcpy(). |
| + * With valid inputs memcpy() would be fine here. |
| + */ |
| + memmove(dict->buf + dict->pos, b->in + b->in_pos, copy_size); |
| dict->pos += copy_size; |
| |
| if (dict->full < dict->pos) |
| @@ -397,7 +404,11 @@ static void dict_uncompressed(struct dictionary *dict, struct xz_buf *b, |
| if (dict->pos == dict->end) |
| dict->pos = 0; |
| |
| - memcpy(b->out + b->out_pos, b->in + b->in_pos, |
| + /* |
| + * Like above but for multi-call mode: use memmove() |
| + * to avoid undefined behavior with invalid input. |
| + */ |
| + memmove(b->out + b->out_pos, b->in + b->in_pos, |
| copy_size); |
| } |
| |
| @@ -421,6 +432,12 @@ static uint32_t dict_flush(struct dictionary *dict, struct xz_buf *b) |
| if (dict->pos == dict->end) |
| dict->pos = 0; |
| |
| + /* |
| + * These buffers cannot overlap even if doing in-place |
| + * decompression because in multi-call mode dict->buf |
| + * has been allocated by us in this file; it's not |
| + * provided by the caller like in single-call mode. |
| + */ |
| memcpy(b->out + b->out_pos, dict->buf + dict->start, |
| copy_size); |
| } |
| -- |
| 2.33.0 |
| |