Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian....@packages.debian.org
Usertags: pu
[ Reason ]
There's a bug in pmdk versions 1.9..1.11, that can cause data loss when
power to the CPU is lost (ie, an unclean shutdown of the machine).
It's caused by a clash between a macro named "barrier" vs function pointers
also named "barrier".
buster (1.5) has an ancient version of this code from before it was
reworked, and thus doesn't contain this bug.
buster-bpo (1.9.2) has a full upstream bugfix release (1.9.3) waiting in
BACKPORTS-POLICY.
bullseye (1.10) can be fixed either via the full upstream bugfix release
(1.10.1) or via a single cherry-picked commit; this p-u has just the
single fix.
bookworm (1.11) has already been updated to 1.11.1.
[ Impact ]
With missing barriers, a power loss at an unfortunate moment can cause
data corruption: eg. a pointer to a new version of the data may survive
the crash but the data hasn't been made durable yet, etc.
[ Tests ]
It's hard to test power loss behaviour -- the persistent vs volatile state
isn't distinguishable without an actual power loss. There's a valgrind
fork (pmemcheck) that is supposed to look for this kind of bugs, but it
didn't catch this one.
On the other hand, non-temporal memcpy has same visible effects as regular
(cached) memcpy, and testing whether it actually works is well-covered by
the testsuite, ran at build time.
[ Risks ]
The compiler barriers (the macro) were introduced long after function
pointers thus re-exposing the proper code is well tested. And, a store
barrier too much can't hurt anything but a bit of performance.
[ Checklist ]
[✓] *all* changes are documented in the d/changelog
[✓] I reviewed all changes and I approve them
[✓] attach debdiff against the package in (old)stable
[✓] the issue is verified as fixed in unstable
[ Changes ]
I've cherry-picked commit 55ec1b24ac89371e1dd0544a17662c738075041e from
upstream. The patch renames all uses of the macro, converting it to an
inline function as well.
[ Other info ]
The bug was introduced in 75ba8a54b3e7045dbbdc2cf7324fe71d8d24069a.
diff -Nru pmdk-1.10/debian/changelog pmdk-1.10/debian/changelog
--- pmdk-1.10/debian/changelog 2021-07-02 17:02:37.000000000 +0200
+++ pmdk-1.10/debian/changelog 2021-09-28 17:41:00.000000000 +0200
@@ -1,3 +1,9 @@
+pmdk (1.10-2+deb11u1) bullseye; urgency=high
+
+ * Fix missing barriers after non-temporal memcpy.
+
+ -- Adam Borowski <kilob...@angband.pl> Tue, 28 Sep 2021 17:41:00 +0200
+
pmdk (1.10-2) unstable; urgency=high
* Fix insufficient flushing on ARMv8.2+ (closes: #990573).
diff -Nru
pmdk-1.10/debian/patches/0001-common-fix-missing-sfence-in-non-temporal-memcpy.patch
pmdk-1.10/debian/patches/0001-common-fix-missing-sfence-in-non-temporal-memcpy.patch
---
pmdk-1.10/debian/patches/0001-common-fix-missing-sfence-in-non-temporal-memcpy.patch
1970-01-01 01:00:00.000000000 +0100
+++
pmdk-1.10/debian/patches/0001-common-fix-missing-sfence-in-non-temporal-memcpy.patch
2021-09-28 17:41:00.000000000 +0200
@@ -0,0 +1,188 @@
+From e67ca1ee3089d28e5945bc4a3e33ac525e313b5b Mon Sep 17 00:00:00 2001
+From: Piotr Balcer <piotr.bal...@intel.com>
+Date: Wed, 1 Sep 2021 14:12:49 +0200
+Subject: [PATCH] common: fix missing sfence in non-temporal memcpy
+
+The implementation of hardware fencing for non-temporal
+memcpy variants is done using a function pointer. Some
+of those pointers are called "barrier" which unfortunately
+overlaps with a function-like macro that's used for compiler
+barriers. This meant that a compiler barrier was being used
+instead of a hardware store barrier.
+
+This patch changes the compiler barrier to a static inline
+function called "compiler_barrier" to avoid name conflicts.
+
+Fixes #5292
+Reported-by: @Transpeptidase
+---
+ src/core/util.h | 17 ++++++++++++++---
+ src/libpmem2/x86_64/memcpy/memcpy_nt_avx.c | 4 ++--
+ src/libpmem2/x86_64/memcpy/memcpy_nt_avx512f.c | 4 ++--
+ src/libpmem2/x86_64/memcpy/memcpy_nt_sse2.c | 4 ++--
+ src/libpmem2/x86_64/memset/memset_nt_avx.c | 4 ++--
+ src/libpmem2/x86_64/memset/memset_nt_avx512f.c | 4 ++--
+ src/libpmem2/x86_64/memset/memset_nt_sse2.c | 4 ++--
+ 7 files changed, 26 insertions(+), 15 deletions(-)
+
+diff --git a/src/core/util.h b/src/core/util.h
+index 542047a17..1ce575dfa 100644
+--- a/src/core/util.h
++++ b/src/core/util.h
+@@ -1,5 +1,5 @@
+ /* SPDX-License-Identifier: BSD-3-Clause */
+-/* Copyright 2014-2020, Intel Corporation */
++/* Copyright 2014-2021, Intel Corporation */
+ /*
+ * Copyright (c) 2016-2020, Microsoft Corporation. All rights reserved.
+ *
+@@ -133,13 +133,24 @@ void util_set_alloc_funcs(
+ #ifdef _MSC_VER
+ #define force_inline inline __forceinline
+ #define NORETURN __declspec(noreturn)
+-#define barrier() _ReadWriteBarrier()
+ #else
+ #define force_inline __attribute__((always_inline)) inline
+ #define NORETURN __attribute__((noreturn))
+-#define barrier() asm volatile("" ::: "memory")
+ #endif
+
++/*
++ * compiler_barrier -- issues a compiler barrier
++ */
++static force_inline void
++compiler_barrier(void)
++{
++#ifdef _MSC_VER
++ _ReadWriteBarrier();
++#else
++ asm volatile("" ::: "memory");
++#endif
++}
++
+ #ifdef _MSC_VER
+ typedef UNALIGNED uint64_t ua_uint64_t;
+ typedef UNALIGNED uint32_t ua_uint32_t;
+diff --git a/src/libpmem2/x86_64/memcpy/memcpy_nt_avx.c
b/src/libpmem2/x86_64/memcpy/memcpy_nt_avx.c
+index ff007fb3c..6311bed4f 100644
+--- a/src/libpmem2/x86_64/memcpy/memcpy_nt_avx.c
++++ b/src/libpmem2/x86_64/memcpy/memcpy_nt_avx.c
+@@ -1,5 +1,5 @@
+ // SPDX-License-Identifier: BSD-3-Clause
+-/* Copyright 2017-2020, Intel Corporation */
++/* Copyright 2017-2021, Intel Corporation */
+
+ #include <immintrin.h>
+ #include <stddef.h>
+@@ -22,7 +22,7 @@ static force_inline void
+ mm256_stream_si256(char *dest, unsigned idx, __m256i src)
+ {
+ _mm256_stream_si256((__m256i *)dest + idx, src);
+- barrier();
++ compiler_barrier();
+ }
+
+ static force_inline void
+diff --git a/src/libpmem2/x86_64/memcpy/memcpy_nt_avx512f.c
b/src/libpmem2/x86_64/memcpy/memcpy_nt_avx512f.c
+index fb19504e4..4a60b9cd0 100644
+--- a/src/libpmem2/x86_64/memcpy/memcpy_nt_avx512f.c
++++ b/src/libpmem2/x86_64/memcpy/memcpy_nt_avx512f.c
+@@ -1,5 +1,5 @@
+ // SPDX-License-Identifier: BSD-3-Clause
+-/* Copyright 2017-2020, Intel Corporation */
++/* Copyright 2017-2021, Intel Corporation */
+
+ #include <immintrin.h>
+ #include <stddef.h>
+@@ -22,7 +22,7 @@ static force_inline void
+ mm512_stream_si512(char *dest, unsigned idx, __m512i src)
+ {
+ _mm512_stream_si512((__m512i *)dest + idx, src);
+- barrier();
++ compiler_barrier();
+ }
+
+ static force_inline void
+diff --git a/src/libpmem2/x86_64/memcpy/memcpy_nt_sse2.c
b/src/libpmem2/x86_64/memcpy/memcpy_nt_sse2.c
+index b633be9da..05c5cf9bf 100644
+--- a/src/libpmem2/x86_64/memcpy/memcpy_nt_sse2.c
++++ b/src/libpmem2/x86_64/memcpy/memcpy_nt_sse2.c
+@@ -1,5 +1,5 @@
+ // SPDX-License-Identifier: BSD-3-Clause
+-/* Copyright 2017-2020, Intel Corporation */
++/* Copyright 2017-2021, Intel Corporation */
+
+ #include <immintrin.h>
+ #include <stddef.h>
+@@ -21,7 +21,7 @@ static force_inline void
+ mm_stream_si128(char *dest, unsigned idx, __m128i src)
+ {
+ _mm_stream_si128((__m128i *)dest + idx, src);
+- barrier();
++ compiler_barrier();
+ }
+
+ static force_inline void
+diff --git a/src/libpmem2/x86_64/memset/memset_nt_avx.c
b/src/libpmem2/x86_64/memset/memset_nt_avx.c
+index 4a4d5f6a2..4882b3c58 100644
+--- a/src/libpmem2/x86_64/memset/memset_nt_avx.c
++++ b/src/libpmem2/x86_64/memset/memset_nt_avx.c
+@@ -1,5 +1,5 @@
+ // SPDX-License-Identifier: BSD-3-Clause
+-/* Copyright 2017-2020, Intel Corporation */
++/* Copyright 2017-2021, Intel Corporation */
+
+ #include <immintrin.h>
+ #include <stddef.h>
+@@ -17,7 +17,7 @@ static force_inline void
+ mm256_stream_si256(char *dest, unsigned idx, __m256i src)
+ {
+ _mm256_stream_si256((__m256i *)dest + idx, src);
+- barrier();
++ compiler_barrier();
+ }
+
+ static force_inline void
+diff --git a/src/libpmem2/x86_64/memset/memset_nt_avx512f.c
b/src/libpmem2/x86_64/memset/memset_nt_avx512f.c
+index b29402a93..5db88c5aa 100644
+--- a/src/libpmem2/x86_64/memset/memset_nt_avx512f.c
++++ b/src/libpmem2/x86_64/memset/memset_nt_avx512f.c
+@@ -1,5 +1,5 @@
+ // SPDX-License-Identifier: BSD-3-Clause
+-/* Copyright 2017-2020, Intel Corporation */
++/* Copyright 2017-2021, Intel Corporation */
+
+ #include <immintrin.h>
+ #include <stddef.h>
+@@ -18,7 +18,7 @@ static force_inline void
+ mm512_stream_si512(char *dest, unsigned idx, __m512i src)
+ {
+ _mm512_stream_si512((__m512i *)dest + idx, src);
+- barrier();
++ compiler_barrier();
+ }
+
+ static force_inline void
+diff --git a/src/libpmem2/x86_64/memset/memset_nt_sse2.c
b/src/libpmem2/x86_64/memset/memset_nt_sse2.c
+index 5590a65f8..0793ff5be 100644
+--- a/src/libpmem2/x86_64/memset/memset_nt_sse2.c
++++ b/src/libpmem2/x86_64/memset/memset_nt_sse2.c
+@@ -1,5 +1,5 @@
+ // SPDX-License-Identifier: BSD-3-Clause
+-/* Copyright 2017-2020, Intel Corporation */
++/* Copyright 2017-2021, Intel Corporation */
+
+ #include <immintrin.h>
+ #include <stddef.h>
+@@ -16,7 +16,7 @@ static force_inline void
+ mm_stream_si128(char *dest, unsigned idx, __m128i src)
+ {
+ _mm_stream_si128((__m128i *)dest + idx, src);
+- barrier();
++ compiler_barrier();
+ }
+
+ static force_inline void
+--
+2.33.0
+
diff -Nru pmdk-1.10/debian/patches/series pmdk-1.10/debian/patches/series
--- pmdk-1.10/debian/patches/series 2021-07-02 17:02:37.000000000 +0200
+++ pmdk-1.10/debian/patches/series 2021-09-28 17:41:00.000000000 +0200
@@ -1,2 +1,3 @@
manpage-debug-packages.patch
0001-pmem2-arm64-fix-data-loss-on-ARMv8.2-improper-flushi.patch
+0001-common-fix-missing-sfence-in-non-temporal-memcpy.patch
