[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1028250: debian-installer: broken cryptsetup support



Hi kibi,

On Sat, 01 Apr 2023 at 01:34:54 +0200, Guilhem Moulin wrote:
> Ah right, reopened the upstream issue but forgot to follow-up here :-(
> https://gitlab.com/cryptsetup/cryptsetup/-/issues/802#note_1328592911

AFAICT the issue is now fully fixed upstream: on systems without swap
the memory cost won't exceed half the amount of free memory during PBKDF
benchmarking.  I can think of the following next steps (list probably
not exhaustive):

 * Don't do anything: ship 2:2.6.1-3~deb12u1 in bookworm and leave the
   DI errata in place.  The downside is that the PBKDF needs roughly
   half of the physical memory, so the OOM killer might trigger if the
   rest of the system uses close to the other half.  Moreover this not
   future proof, as memory requirement increases along releases.  (That
   said the issue has been present since 2 releases and there is nothing
   we can do about existing volumes.  Concretely, that means low-memory
   Bookworm rescue systems will likely OOM when trying to luksOpen an
   existing LUKS2 volume in graphical mode.)

 * Wait for upstream to release 2.6.2 with fixes for #-1 as well as
   other bugfixes and upload it, either via t-p-u during the hard freeze
   or later via s-p-u.  In upstream's own words “[the minor release]
   will take few week because of translation loop etc.”  The downside
   being of course more review work for the release team, as the diff is
   already rather large:
   https://gitlab.com/cryptsetup/cryptsetup/-/compare/v2.6.1...main?from_project_id=195655&straight=false

 * Backport upstream MR !498, let it mature in sid for a few
   weeks then upload 2:2.6.1-4~deb12u1 via t-p-u.  There are only 2
   upstream commits to cherry-pick and neither is large nor intrusive;
   moreover like the commits previously cherry-picked they are no-op on
   “normal” systems (only systems without swap are affected).  For
   convenience I attach a debdiff for 2:2.6.1-3~deb12u2 and you'll also
   find binary packages for amd64 at
   https://people.debian.org/~guilhem/tmp/cryptsetup_2.6.1-3~deb12u2/
   Tested: autopkgtests (incl. full upstream test suite), d-i in both
   graphical and text install on VMs with 1024M RAM (now memory cost
   won't exceed ~250M resp. ~300M thus leaving plenty of headroom for
   the rest).

Your call :-)
-- 
Guilhem.
diffstat for cryptsetup-2.6.1 cryptsetup-2.6.1

 changelog                                                               |    8 +
 patches/Check-for-physical-memory-available-also-in-PBKDF-benchma.patch |   74 ++++++++++
 patches/Use-only-half-of-detected-free-memory-on-systems-without-.patch |   43 +++++
 patches/series                                                          |    2 
 4 files changed, 127 insertions(+)

diff -Nru cryptsetup-2.6.1/debian/changelog cryptsetup-2.6.1/debian/changelog
--- cryptsetup-2.6.1/debian/changelog	2023-03-26 19:18:59.000000000 +0200
+++ cryptsetup-2.6.1/debian/changelog	2023-04-20 13:19:06.000000000 +0200
@@ -1,3 +1,11 @@
+cryptsetup (2:2.6.1-3~deb12u2) UNRELEASED; urgency=medium
+
+  * Backport upstream MR !498, see #1028250:
+    + 7893c33d: Check for physical memory available also in PBKDF benchmark.
+    + 6721d3a8: Use only half of detected free memory on systems without swap.
+
+ -- Guilhem Moulin <guilhem@debian.org>  Thu, 20 Apr 2023 13:19:06 +0200
+
 cryptsetup (2:2.6.1-3~deb12u1) bookworm; urgency=medium
 
   * Rebuild for Bookworm.
diff -Nru cryptsetup-2.6.1/debian/patches/Check-for-physical-memory-available-also-in-PBKDF-benchma.patch cryptsetup-2.6.1/debian/patches/Check-for-physical-memory-available-also-in-PBKDF-benchma.patch
--- cryptsetup-2.6.1/debian/patches/Check-for-physical-memory-available-also-in-PBKDF-benchma.patch	1970-01-01 01:00:00.000000000 +0100
+++ cryptsetup-2.6.1/debian/patches/Check-for-physical-memory-available-also-in-PBKDF-benchma.patch	2023-04-20 13:19:06.000000000 +0200
@@ -0,0 +1,74 @@
+From: Milan Broz <gmazyland@gmail.com>
+Date: Mon, 3 Apr 2023 13:31:16 +0200
+Subject: Check for physical memory available also in PBKDF benchmark.
+
+Origin: https://gitlab.com/cryptsetup/cryptsetup/-/commit/7893c33d71cde09e240234c484c6c468f22c2fe7
+Bug: https://gitlab.com/cryptsetup/cryptsetup/-/issues/802#note_1328592911
+Bug-Debian: https://bugs.debian.org/1028250
+---
+ lib/internal.h        | 1 +
+ lib/utils_benchmark.c | 9 +++++++++
+ lib/utils_pbkdf.c     | 4 ++--
+ 3 files changed, 12 insertions(+), 2 deletions(-)
+
+diff --git a/lib/internal.h b/lib/internal.h
+index 98095fa..f261cae 100644
+--- a/lib/internal.h
++++ b/lib/internal.h
+@@ -89,6 +89,7 @@ int crypt_benchmark_pbkdf_internal(struct crypt_device *cd,
+ 				   struct crypt_pbkdf_type *pbkdf,
+ 				   size_t volume_key_size);
+ const char *crypt_get_cipher_spec(struct crypt_device *cd);
++uint32_t pbkdf_adjusted_phys_memory_kb(void);
+ 
+ /* Device backend */
+ struct device;
+diff --git a/lib/utils_benchmark.c b/lib/utils_benchmark.c
+index 728e4df..a0326ce 100644
+--- a/lib/utils_benchmark.c
++++ b/lib/utils_benchmark.c
+@@ -101,6 +101,7 @@ int crypt_benchmark_pbkdf(struct crypt_device *cd,
+ {
+ 	int r, priority;
+ 	const char *kdf_opt;
++	uint32_t memory_kb;
+ 
+ 	if (!pbkdf || (!password && password_size))
+ 		return -EINVAL;
+@@ -113,6 +114,14 @@ int crypt_benchmark_pbkdf(struct crypt_device *cd,
+ 
+ 	log_dbg(cd, "Running %s(%s) benchmark.", pbkdf->type, kdf_opt);
+ 
++	memory_kb = pbkdf_adjusted_phys_memory_kb();
++	if (memory_kb < pbkdf->max_memory_kb) {
++		log_dbg(cd, "Not enough physical memory detected, "
++			"PBKDF max memory decreased from %dkB to %dkB.",
++			pbkdf->max_memory_kb, memory_kb);
++		pbkdf->max_memory_kb = memory_kb;
++	}
++
+ 	crypt_process_priority(cd, &priority, true);
+ 	r = crypt_pbkdf_perf(pbkdf->type, pbkdf->hash, password, password_size,
+ 			     salt, salt_size, volume_key_size, pbkdf->time_ms,
+diff --git a/lib/utils_pbkdf.c b/lib/utils_pbkdf.c
+index d8f41c7..b2d4fa0 100644
+--- a/lib/utils_pbkdf.c
++++ b/lib/utils_pbkdf.c
+@@ -61,7 +61,7 @@ const struct crypt_pbkdf_type *crypt_get_pbkdf_type_params(const char *pbkdf_typ
+ 	return NULL;
+ }
+ 
+-static uint32_t adjusted_phys_memory(void)
++uint32_t pbkdf_adjusted_phys_memory_kb(void)
+ {
+ 	uint64_t free_kb, memory_kb = crypt_getphysmemory_kb();
+ 
+@@ -258,7 +258,7 @@ int init_pbkdf_type(struct crypt_device *cd,
+ 	}
+ 
+ 	if (cd_pbkdf->max_memory_kb) {
+-		memory_kb = adjusted_phys_memory();
++		memory_kb = pbkdf_adjusted_phys_memory_kb();
+ 		if (cd_pbkdf->max_memory_kb > memory_kb) {
+ 			log_dbg(cd, "Not enough physical memory detected, "
+ 				"PBKDF max memory decreased from %dkB to %dkB.",
diff -Nru cryptsetup-2.6.1/debian/patches/series cryptsetup-2.6.1/debian/patches/series
--- cryptsetup-2.6.1/debian/patches/series	2023-03-26 19:18:59.000000000 +0200
+++ cryptsetup-2.6.1/debian/patches/series	2023-04-20 13:19:06.000000000 +0200
@@ -1,2 +1,4 @@
 Try-to-avoid-OOM-killer-on-low-memory-systems-without-swa.patch
 Print-warning-when-keyslot-requires-more-memory-than-avai.patch
+Check-for-physical-memory-available-also-in-PBKDF-benchma.patch
+Use-only-half-of-detected-free-memory-on-systems-without-.patch
diff -Nru cryptsetup-2.6.1/debian/patches/Use-only-half-of-detected-free-memory-on-systems-without-.patch cryptsetup-2.6.1/debian/patches/Use-only-half-of-detected-free-memory-on-systems-without-.patch
--- cryptsetup-2.6.1/debian/patches/Use-only-half-of-detected-free-memory-on-systems-without-.patch	1970-01-01 01:00:00.000000000 +0100
+++ cryptsetup-2.6.1/debian/patches/Use-only-half-of-detected-free-memory-on-systems-without-.patch	2023-04-20 13:19:06.000000000 +0200
@@ -0,0 +1,43 @@
+From: Milan Broz <gmazyland@gmail.com>
+Date: Mon, 17 Apr 2023 13:41:17 +0200
+Subject: Use only half of detected free memory on systems without swap.
+
+As tests shows, limiting used Argon2 memory to free memory on
+systems without swap is still not enough.
+Use just half of it, this should bring needed margin while
+still use Argon2.
+
+Note, for very-low memory constrained systems user should
+avoid memory-hard PBKDF (IOW manually select PBKDF2), we
+do not do this automatically.
+
+Origin: https://gitlab.com/cryptsetup/cryptsetup/-/commit/6721d3a8b29b13fe88aeeaefe09d457e99d1c6fa
+Bug: https://gitlab.com/cryptsetup/cryptsetup/-/issues/802#note_1328592911
+Bug-Debian: https://bugs.debian.org/1028250
+---
+ lib/utils_pbkdf.c | 9 ++++++++-
+ 1 file changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/lib/utils_pbkdf.c b/lib/utils_pbkdf.c
+index b2d4fa0..7399bd2 100644
+--- a/lib/utils_pbkdf.c
++++ b/lib/utils_pbkdf.c
+@@ -76,10 +76,17 @@ uint32_t pbkdf_adjusted_phys_memory_kb(void)
+ 	memory_kb /= 2;
+ 
+ 	/*
+-	 * Never use more that available free space on system without swap.
++	 * Never use more that half of available free memory on system without swap.
+ 	 */
+ 	if (!crypt_swapavailable()) {
+ 		free_kb = crypt_getphysmemoryfree_kb();
++
++		/*
++		 * Using exactly free memory causes OOM too, use only half of the value.
++		 * Ignore small values (< 64MB), user should use PBKDF2 in such environment.
++		 */
++		free_kb /= 2;
++
+ 		if (free_kb > (64 * 1024) && free_kb < memory_kb)
+ 			return free_kb;
+ 	}

Attachment: signature.asc
Description: PGP signature


Reply to: