From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Gao Xiang <hsiangkao@linux.alibaba.com>
Cc: stable@vger.kernel.org, Yann Collet <yann.collet.73@gmail.com>,
Miao Xie <miaoxie@huawei.com>, Chao Yu <yuchao0@huawei.com>,
Li Guifu <bluce.liguifu@huawei.com>,
Guo Xuenan <guoxuenan@huawei.com>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [PATCH for-5.10.y] lib/lz4: explicitly support in-place decompression
Date: Tue, 8 Jun 2021 18:35:10 +0200 [thread overview]
Message-ID: <YL+cPrR7ARC1WdnA@kroah.com> (raw)
In-Reply-To: <1622562405-63431-2-git-send-email-hsiangkao@linux.alibaba.com>
On Tue, Jun 01, 2021 at 11:46:45PM +0800, Gao Xiang wrote:
> commit 89b158635ad79574bde8e94d45dad33f8cf09549 upstream.
>
> LZ4 final literal copy could be overlapped when doing
> in-place decompression, so it's unsafe to just use memcpy()
> on an optimized memcpy approach but memmove() instead.
>
> Upstream LZ4 has updated this years ago [1] (and the impact
> is non-sensible [2] plus only a few bytes remain), this commit
> just synchronizes LZ4 upstream code to the kernel side as well.
>
> It can be observed as EROFS in-place decompression failure
> on specific files when X86_FEATURE_ERMS is unsupported,
> memcpy() optimization of commit 59daa706fbec ("x86, mem:
> Optimize memcpy by avoiding memory false dependece") will
> be enabled then.
>
> Currently most modern x86-CPUs support ERMS, these CPUs just
> use "rep movsb" approach so no problem at all. However, it can
> still be verified with forcely disabling ERMS feature...
>
> arch/x86/lib/memcpy_64.S:
> ALTERNATIVE_2 "jmp memcpy_orig", "", X86_FEATURE_REP_GOOD, \
> - "jmp memcpy_erms", X86_FEATURE_ERMS
> + "jmp memcpy_orig", X86_FEATURE_ERMS
>
> We didn't observe any strange on arm64/arm/x86 platform before
> since most memcpy() would behave in an increasing address order
> ("copy upwards" [3]) and it's the correct order of in-place
> decompression but it really needs an update to memmove() for sure
> considering it's an undefined behavior according to the standard
> and some unique optimization already exists in the kernel.
>
> [1] https://github.com/lz4/lz4/commit/33cb8518ac385835cc17be9a770b27b40cd0e15b
> [2] https://github.com/lz4/lz4/pull/717#issuecomment-497818921
> [3] https://sourceware.org/bugzilla/show_bug.cgi?id=12518
>
> Link: https://lkml.kernel.org/r/20201122030749.2698994-1-hsiangkao@redhat.com
> Reviewed-by: Nick Terrell <terrelln@fb.com>
> Cc: Yann Collet <yann.collet.73@gmail.com>
> Cc: Miao Xie <miaoxie@huawei.com>
> Cc: Chao Yu <yuchao0@huawei.com>
> Cc: Li Guifu <bluce.liguifu@huawei.com>
> Cc: Guo Xuenan <guoxuenan@huawei.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Gao Xiang <hsiangkao@linux.alibaba.com>
> ---
> Hi,
>
> Please kindly consider these two backports to 5.4.y and 5.10.y LTS
> kernels, and the reason shown as above (it could cause lz4 in-place
> decompression (mainly EROFS) failure due to the different designed
> memcpy overlapped behavior on x86 if ERMS is unsupported.) The lz4
> upstream commit itself has been merged for 2 years. And the linux
> upstream commit is also merged for months without any other
> regression.
>
> And in principle, it won't have any real impact at all, so I think
> it's now safe to backport this to LTS kernels for unsupported ERMS
> x86s.
Both now queued up, thanks!
greg k-h
prev parent reply other threads:[~2021-06-08 16:35 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-01 15:46 [PATCH for-5.4.y] lib/lz4: explicitly support in-place decompression Gao Xiang
2021-06-01 15:46 ` [PATCH for-5.10.y] " Gao Xiang
2021-06-08 16:35 ` Greg Kroah-Hartman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YL+cPrR7ARC1WdnA@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=bluce.liguifu@huawei.com \
--cc=guoxuenan@huawei.com \
--cc=hsiangkao@linux.alibaba.com \
--cc=miaoxie@huawei.com \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=yann.collet.73@gmail.com \
--cc=yuchao0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.