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
| ||
|
Message-ID: <d85e4c86-6dd0-4d5d-730b-91217d38ecaa@loongson.cn> Date: Thu, 14 Dec 2023 09:44:27 +0800 From: Hongchen Zhang <zhanghongchen@...ngson.cn> To: "Rafael J. Wysocki" <rafael@...nel.org> Cc: 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 Hi Rafael, Thanks for your review. On 2023/12/13 pm 9:52, Rafael J. Wysocki wrote: > 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. > Yes. > It is always better to list the changes made between consecutive > versions of a patch. Sorry for not updating the changelog, I will remember this next time. > >> 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! > -- Best Regards Hongchen Zhang
Powered by blists - more mailing lists