| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Dec 2023 14:52:03 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Hongchen Zhang <zhanghongchen@...ngson.cn>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>,
Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
Bojan Smojver <bojan@...ursive.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, loongson-kernel@...ts.loongnix.cn,
stable@...r.kernel.org, Weihao Li <liweihao@...ngson.cn>
Subject: Re: [PATCH v3] PM: hibernate: use acquire/release ordering when
compress/decompress image
On Wed, Dec 13, 2023 at 2:11 AM Hongchen Zhang
<zhanghongchen@...ngson.cn> wrote:
>
> When we test S4(suspend to disk) on LoongArch 3A6000 platform, the
> test case sometimes fails. The dmesg log shows the following error:
> Invalid LZO compressed length
> After we dig into the code, we find out that:
> When compress/decompress the image, the synchronization operation
> between the control thread and the compress/decompress/crc thread
> uses relaxed ordering interface, which is unreliable, and the
> following situation may occur:
> CPU 0 CPU 1
> save_image_lzo lzo_compress_threadfn
> atomic_set(&d->stop, 1);
> atomic_read(&data[thr].stop)
> data[thr].cmp = data[thr].cmp_len;
> WRITE data[thr].cmp_len
> Then CPU0 get a old cmp_len and write to disk. When cpu resume from S4,
> wrong cmp_len is loaded.
>
> To maintain data consistency between two threads, we should use the
> acquire/release ordering interface. So we change atomic_read/atomic_set
> to atomic_read_acquire/atomic_set_release.
>
> Fixes: 081a9d043c98 ("PM / Hibernate: Improve performance of LZO/plain hibernation, checksum image")
> Cc: stable@...r.kernel.org
> Co-developed-by: Weihao Li <liweihao@...ngson.cn>
I gather that the tag above is the only difference between this
version of the patch and the previous one.
It is always better to list the changes made between consecutive
versions of a patch.
> Signed-off-by: Weihao Li <liweihao@...ngson.cn>
> Signed-off-by: Hongchen Zhang <zhanghongchen@...ngson.cn>
> ---
> kernel/power/swap.c | 38 +++++++++++++++++++-------------------
> 1 file changed, 19 insertions(+), 19 deletions(-)
>
> diff --git a/kernel/power/swap.c b/kernel/power/swap.c
> index a2cb0babb5ec..d44f5937f1e5 100644
> --- a/kernel/power/swap.c
> +++ b/kernel/power/swap.c
> @@ -606,11 +606,11 @@ static int crc32_threadfn(void *data)
> unsigned i;
>
> while (1) {
> - wait_event(d->go, atomic_read(&d->ready) ||
> + wait_event(d->go, atomic_read_acquire(&d->ready) ||
> kthread_should_stop());
> if (kthread_should_stop()) {
> d->thr = NULL;
> - atomic_set(&d->stop, 1);
> + atomic_set_release(&d->stop, 1);
> wake_up(&d->done);
> break;
> }
> @@ -619,7 +619,7 @@ static int crc32_threadfn(void *data)
> for (i = 0; i < d->run_threads; i++)
> *d->crc32 = crc32_le(*d->crc32,
> d->unc[i], *d->unc_len[i]);
> - atomic_set(&d->stop, 1);
> + atomic_set_release(&d->stop, 1);
> wake_up(&d->done);
> }
> return 0;
> @@ -649,12 +649,12 @@ static int lzo_compress_threadfn(void *data)
> struct cmp_data *d = data;
>
> while (1) {
> - wait_event(d->go, atomic_read(&d->ready) ||
> + wait_event(d->go, atomic_read_acquire(&d->ready) ||
> kthread_should_stop());
> if (kthread_should_stop()) {
> d->thr = NULL;
> d->ret = -1;
> - atomic_set(&d->stop, 1);
> + atomic_set_release(&d->stop, 1);
> wake_up(&d->done);
> break;
> }
> @@ -663,7 +663,7 @@ static int lzo_compress_threadfn(void *data)
> d->ret = lzo1x_1_compress(d->unc, d->unc_len,
> d->cmp + LZO_HEADER, &d->cmp_len,
> d->wrk);
> - atomic_set(&d->stop, 1);
> + atomic_set_release(&d->stop, 1);
> wake_up(&d->done);
> }
> return 0;
> @@ -798,7 +798,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
>
> data[thr].unc_len = off;
>
> - atomic_set(&data[thr].ready, 1);
> + atomic_set_release(&data[thr].ready, 1);
> wake_up(&data[thr].go);
> }
>
> @@ -806,12 +806,12 @@ static int save_image_lzo(struct swap_map_handle *handle,
> break;
>
> crc->run_threads = thr;
> - atomic_set(&crc->ready, 1);
> + atomic_set_release(&crc->ready, 1);
> wake_up(&crc->go);
>
> for (run_threads = thr, thr = 0; thr < run_threads; thr++) {
> wait_event(data[thr].done,
> - atomic_read(&data[thr].stop));
> + atomic_read_acquire(&data[thr].stop));
> atomic_set(&data[thr].stop, 0);
>
> ret = data[thr].ret;
> @@ -850,7 +850,7 @@ static int save_image_lzo(struct swap_map_handle *handle,
> }
> }
>
> - wait_event(crc->done, atomic_read(&crc->stop));
> + wait_event(crc->done, atomic_read_acquire(&crc->stop));
> atomic_set(&crc->stop, 0);
> }
>
> @@ -1132,12 +1132,12 @@ static int lzo_decompress_threadfn(void *data)
> struct dec_data *d = data;
>
> while (1) {
> - wait_event(d->go, atomic_read(&d->ready) ||
> + wait_event(d->go, atomic_read_acquire(&d->ready) ||
> kthread_should_stop());
> if (kthread_should_stop()) {
> d->thr = NULL;
> d->ret = -1;
> - atomic_set(&d->stop, 1);
> + atomic_set_release(&d->stop, 1);
> wake_up(&d->done);
> break;
> }
> @@ -1150,7 +1150,7 @@ static int lzo_decompress_threadfn(void *data)
> flush_icache_range((unsigned long)d->unc,
> (unsigned long)d->unc + d->unc_len);
>
> - atomic_set(&d->stop, 1);
> + atomic_set_release(&d->stop, 1);
> wake_up(&d->done);
> }
> return 0;
> @@ -1335,7 +1335,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
> }
>
> if (crc->run_threads) {
> - wait_event(crc->done, atomic_read(&crc->stop));
> + wait_event(crc->done, atomic_read_acquire(&crc->stop));
> atomic_set(&crc->stop, 0);
> crc->run_threads = 0;
> }
> @@ -1371,7 +1371,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
> pg = 0;
> }
>
> - atomic_set(&data[thr].ready, 1);
> + atomic_set_release(&data[thr].ready, 1);
> wake_up(&data[thr].go);
> }
>
> @@ -1390,7 +1390,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
>
> for (run_threads = thr, thr = 0; thr < run_threads; thr++) {
> wait_event(data[thr].done,
> - atomic_read(&data[thr].stop));
> + atomic_read_acquire(&data[thr].stop));
> atomic_set(&data[thr].stop, 0);
>
> ret = data[thr].ret;
> @@ -1421,7 +1421,7 @@ static int load_image_lzo(struct swap_map_handle *handle,
> ret = snapshot_write_next(snapshot);
> if (ret <= 0) {
> crc->run_threads = thr + 1;
> - atomic_set(&crc->ready, 1);
> + atomic_set_release(&crc->ready, 1);
> wake_up(&crc->go);
> goto out_finish;
> }
> @@ -1429,13 +1429,13 @@ static int load_image_lzo(struct swap_map_handle *handle,
> }
>
> crc->run_threads = thr;
> - atomic_set(&crc->ready, 1);
> + atomic_set_release(&crc->ready, 1);
> wake_up(&crc->go);
> }
>
> out_finish:
> if (crc->run_threads) {
> - wait_event(crc->done, atomic_read(&crc->stop));
> + wait_event(crc->done, atomic_read_acquire(&crc->stop));
> atomic_set(&crc->stop, 0);
> }
> stop = ktime_get();
> --
Applied as 6.8 material with some edits in the subject and changelog.
Thanks!
Powered by blists - more mailing lists