From 3154d7b5e03949cc6c29d99b510285b1c474259f Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 03:03:37 +0200 Subject: [PATCH 1/2] Unix: lock key buffers in memory and exclude them from core dumps SecureBuffer only erased its memory when freed. The FUSE service that holds the master keys had no locked memory and could be core dumped, and a core file of it contained both halves of the master key. Give each SecureBuffer whole pages of its own, lock them with mlock and exclude them from core dumps (MADV_DONTDUMP, or MADV_NOCORE on FreeBSD). Separate pages let a freed buffer be unlocked without affecting another one. Memory locks are not inherited by fork, so the FUSE service locks the live buffers again once FUSE has started it in the background. If RLIMIT_MEMLOCK does not allow locking, the buffers are used unlocked as before. Key material can also be copied outside SecureBuffer, so the core service and the FUSE service additionally disable core dumps: they set RLIMIT_CORE to 0 and, on Linux, clear the dumpable flag, which a core_pattern pipe does not ignore. Volume::WriteSectors encrypts each write in a buffer that is encrypted in place right after the copy. Allocating it as a SecureBuffer would lock and unlock its pages on every write and cut write throughput by about 20 to 60 percent depending on the write size, so it becomes a Buffer that is still erased when freed. Fixes #1958 --- src/Core/Unix/CoreService.cpp | 2 + src/Driver/Fuse/FuseService.cpp | 6 ++ src/Platform/Buffer.cpp | 108 +++++++++++++++++++++++++++++++- src/Platform/Buffer.h | 10 ++- src/Platform/Unix/Process.cpp | 16 +++++ src/Platform/Unix/Process.h | 1 + src/Volume/Volume.cpp | 5 +- 7 files changed, 144 insertions(+), 4 deletions(-) diff --git a/src/Core/Unix/CoreService.cpp b/src/Core/Unix/CoreService.cpp index 66fbf20959..3c63b024c3 100644 --- a/src/Core/Unix/CoreService.cpp +++ b/src/Core/Unix/CoreService.cpp @@ -1136,6 +1136,8 @@ namespace VeraCrypt { try { + Process::DisableCoreDumps(); + if (forkProcess) RedirectStandardErrorToDevNull (); diff --git a/src/Driver/Fuse/FuseService.cpp b/src/Driver/Fuse/FuseService.cpp index 055eb1de0a..5b6a0527a9 100644 --- a/src/Driver/Fuse/FuseService.cpp +++ b/src/Driver/Fuse/FuseService.cpp @@ -664,6 +664,9 @@ namespace VeraCrypt { try { + // FUSE may have forked to run in the background + SecureBuffer::LockAllAfterFork(); + // Termination signals are handled by a separate process to allow clean dismount on shutdown struct sigaction action; Memory::Zero (&action, sizeof (action)); @@ -1749,6 +1752,9 @@ namespace VeraCrypt void FuseService::ExecFunctor::operator() (int argc, char *argv[]) { + // The service holds the master keys until the volume is dismounted + Process::DisableCoreDumps(); + #if defined(TC_MACOSX) && defined(VC_MACOSX_FUSET) close (StartupPeerFd); #endif diff --git a/src/Platform/Buffer.cpp b/src/Platform/Buffer.cpp index ac55c7bb93..db63bbaf90 100644 --- a/src/Platform/Buffer.cpp +++ b/src/Platform/Buffer.cpp @@ -12,9 +12,51 @@ #include "Buffer.h" #include "Exception.h" +#ifdef TC_UNIX +#include +#include +#include +#include "ForEach.h" +#include "Mutex.h" +#endif namespace VeraCrypt { +#ifdef TC_UNIX + // Secure buffers occupy whole pages of their own, which are locked in memory + // and excluded from core dumps. Unlocking or including the pages again when a + // buffer is freed cannot then affect another allocation. + struct SecureBufferList + { + Mutex ListMutex; + set Buffers; + }; + + static SecureBufferList &GetSecureBufferList () + { + // Never destroyed: buffers may be freed after static destructors have run + static SecureBufferList *list = new SecureBufferList; + return *list; + } + + static size_t GetPageSize () + { + long pageSize = sysconf (_SC_PAGESIZE); + return pageSize > 0 ? (size_t) pageSize : 4096; + } + + static void ExcludeFromCoreDump (void *memory, size_t size, bool exclude) + { +#if defined (MADV_DONTDUMP) + madvise (memory, size, exclude ? MADV_DONTDUMP : MADV_DODUMP); +#elif defined (MADV_NOCORE) + madvise (memory, size, exclude ? MADV_NOCORE : MADV_CORE); +#else + (void) memory; (void) size; (void) exclude; +#endif + } +#endif + Buffer::Buffer () : DataPtr (nullptr), DataSize (0), DataAlignment (0) { } @@ -108,7 +150,7 @@ namespace VeraCrypt Memory::Zero (DataPtr, DataSize); } - SecureBuffer::SecureBuffer (size_t size, size_t alignment) + SecureBuffer::SecureBuffer (size_t size, size_t alignment) : AllocatedSize (0), Locked (false) { Allocate (size, alignment); } @@ -121,7 +163,40 @@ namespace VeraCrypt void SecureBuffer::Allocate (size_t size, size_t alignment) { +#ifdef TC_UNIX + if (size < 1) + throw ParameterIncorrect (SRC_POS); + + if (DataPtr != nullptr) + { + if ((DataSize == size) && (DataAlignment == alignment)) + return; + Free(); + } + + const size_t pageSize = GetPageSize(); + if (size > ((size_t) -1) - pageSize) + throw ParameterTooLarge (SRC_POS); + + const size_t allocatedSize = (size + pageSize - 1) / pageSize * pageSize; + // Alignments are powers of two: the larger one satisfies both + uint8 *memory = static_cast (Memory::AllocateAligned (allocatedSize, alignment > pageSize ? alignment : pageSize)); + + // Locking fails if RLIMIT_MEMLOCK is exceeded; the buffer is still usable + Locked = mlock (memory, allocatedSize) == 0; + ExcludeFromCoreDump (memory, allocatedSize, true); + + DataPtr = memory; + DataSize = size; + DataAlignment = alignment; + AllocatedSize = allocatedSize; + + SecureBufferList &list = GetSecureBufferList(); + ScopeLock lock (list.ListMutex); + list.Buffers.insert (this); +#else Buffer::Allocate (size, alignment); +#endif } void SecureBuffer::Free () @@ -129,8 +204,39 @@ namespace VeraCrypt if (DataPtr == nullptr) throw NotInitialized (SRC_POS); +#ifdef TC_UNIX + { + SecureBufferList &list = GetSecureBufferList(); + ScopeLock lock (list.ListMutex); + list.Buffers.erase (this); + } + + burn (DataPtr, AllocatedSize); + ExcludeFromCoreDump (DataPtr, AllocatedSize, false); + if (Locked) + munlock (DataPtr, AllocatedSize); + + Memory::FreeAligned (DataPtr); + DataPtr = nullptr; + DataSize = 0; + DataAlignment = 0; + AllocatedSize = 0; + Locked = false; +#else Erase (); Buffer::Free (); +#endif + } + + void SecureBuffer::LockAllAfterFork () + { +#ifdef TC_UNIX + SecureBufferList &list = GetSecureBufferList(); + ScopeLock lock (list.ListMutex); + + foreach (SecureBuffer *buffer, list.Buffers) + buffer->Locked = mlock (buffer->DataPtr, buffer->AllocatedSize) == 0; +#endif } void BufferPtr::CopyFrom (const ConstBufferPtr &bufferPtr) const diff --git a/src/Platform/Buffer.h b/src/Platform/Buffer.h index 5d14d6a6b8..05e4cc4d91 100644 --- a/src/Platform/Buffer.h +++ b/src/Platform/Buffer.h @@ -107,15 +107,21 @@ namespace VeraCrypt class SecureBuffer : public Buffer { public: - SecureBuffer () { } + SecureBuffer () : AllocatedSize (0), Locked (false) { } SecureBuffer (size_t size, size_t alignment = 0); - SecureBuffer (const ConstBufferPtr &bufferPtr) { CopyFrom (bufferPtr); } + SecureBuffer (const ConstBufferPtr &bufferPtr) : AllocatedSize (0), Locked (false) { CopyFrom (bufferPtr); } virtual ~SecureBuffer (); virtual void Allocate (size_t size, size_t alignment = 0); virtual void Free (); + // Memory locks are not inherited by a forked process + static void LockAllAfterFork (); + private: + size_t AllocatedSize; + bool Locked; + SecureBuffer (const SecureBuffer &); SecureBuffer &operator= (const SecureBuffer &); }; diff --git a/src/Platform/Unix/Process.cpp b/src/Platform/Unix/Process.cpp index 51f0882308..6b7a0eee09 100644 --- a/src/Platform/Unix/Process.cpp +++ b/src/Platform/Unix/Process.cpp @@ -18,9 +18,11 @@ #include #include #include +#include #include #include #ifdef TC_LINUX +#include #include #endif #include "Process.h" @@ -209,6 +211,20 @@ namespace VeraCrypt } #endif + void Process::DisableCoreDumps () + { + // For processes that hold key material. A core_pattern pipe ignores + // RLIMIT_CORE on Linux, but not the dumpable flag, which also keeps + // other processes of the same user from attaching. + struct rlimit limit; + limit.rlim_cur = 0; + limit.rlim_max = 0; + setrlimit (RLIMIT_CORE, &limit); +#ifdef TC_LINUX + prctl (PR_SET_DUMPABLE, 0, 0, 0, 0); +#endif + } + bool Process::IsExecutable(const std::string& path) { struct stat sb; if (stat(path.c_str(), &sb) == 0) { diff --git a/src/Platform/Unix/Process.h b/src/Platform/Unix/Process.h index 3e3a61503a..cf4999e0c5 100644 --- a/src/Platform/Unix/Process.h +++ b/src/Platform/Unix/Process.h @@ -31,6 +31,7 @@ namespace VeraCrypt Process (); virtual ~Process (); + static void DisableCoreDumps (); static bool IsExecutable(const std::string& path); static std::string FindSystemBinary(const char* name, std::string& errorMsg); static string Execute (const string &processName, const list &arguments, int timeOut = -1, ProcessExecFunctor *execFunctor = nullptr, const Buffer *inputData = nullptr); diff --git a/src/Volume/Volume.cpp b/src/Volume/Volume.cpp index 0cee6344c6..9536a5b059 100644 --- a/src/Volume/Volume.cpp +++ b/src/Volume/Volume.cpp @@ -400,7 +400,10 @@ namespace VeraCrypt if (Protection == VolumeProtection::HiddenVolumeReadOnly) CheckProtectedRange (hostOffset, length); - SecureBuffer encBuf (buffer.Size()); + // Encrypted in place right after the copy. A SecureBuffer would lock the + // pages of every write in memory, which slows down writes considerably. + Buffer encBuf (buffer.Size()); + finally_do_arg (Buffer *, &encBuf, { finally_arg->Erase(); }); encBuf.CopyFrom (buffer); EA->EncryptSectors (encBuf, hostOffset / SectorSize, length / SectorSize, SectorSize); From 49f4b39a4ea26baccd9af8a0267c4b9bd01632af Mon Sep 17 00:00:00 2001 From: Ville Takio Date: Tue, 27 Oct 2026 13:40:42 +0200 Subject: [PATCH 2/2] Tests: add a regression test for #1958 Tests/test_linux_fuse_service_memory.py checks locked memory and no core file for the FUSE service against the installed package in the Linux workflow. --- .github/workflows/build-linux.yml | 3 + Tests/test_linux_fuse_service_memory.py | 174 ++++++++++++++++++++++++ 2 files changed, 177 insertions(+) create mode 100644 Tests/test_linux_fuse_service_memory.py diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index 6379b82ce8..8de7e6ef15 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -28,6 +28,7 @@ on: - 'Tests/test_stdin_password.py' - 'Tests/test_mount_helper.py' - 'Tests/test_linux_mount_point_protection.py' + - 'Tests/test_linux_fuse_service_memory.py' - 'Tests/test.*.hc' - 'Tests/test_volume_read.py' - 'Tests/volume_read_test.cpp' @@ -64,6 +65,7 @@ on: - 'Tests/test_stdin_password.py' - 'Tests/test_mount_helper.py' - 'Tests/test_linux_mount_point_protection.py' + - 'Tests/test_linux_fuse_service_memory.py' - 'Tests/test.*.hc' - 'Tests/test_volume_read.py' - 'Tests/volume_read_test.cpp' @@ -221,6 +223,7 @@ jobs: sudo python3 Tests/test_stdin_password.py --require-locales sudo python3 Tests/test_mount_helper.py --helper /usr/sbin/mount.veracrypt sudo python3 Tests/test_linux_mount_point_protection.py + sudo python3 Tests/test_linux_fuse_service_memory.py sudo apt remove -y veracrypt - name: Install and test VeraCrypt Console .deb packages diff --git a/Tests/test_linux_fuse_service_memory.py b/Tests/test_linux_fuse_service_memory.py new file mode 100644 index 0000000000..9e7055ea9a --- /dev/null +++ b/Tests/test_linux_fuse_service_memory.py @@ -0,0 +1,174 @@ +#!/usr/bin/env python3 +"""Regression test for issue #1958, against the installed veracrypt. + +Usage (as root, after installing a package): + python3 Tests/test_linux_fuse_service_memory.py [--veracrypt PATH] + +Mounts a small volume through FUSE (nokernelcrypto) and checks that the +FUSE service, which holds the master keys, has locked memory and a core +file size limit of zero. +""" + +import argparse +import os +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + +PASSWORD_A = "regression-test-password-a" +# A low PIM keeps the tests fast; it needs passwords of 20 characters or more. +COMMON = ["--pim=1", "--keyfiles="] +MOUNT = COMMON + ["--protect-hidden=no", "--mount-options=nokernelcrypto"] + + +class Checks: + def __init__(self): + self.failures = 0 + + def check(self, desc, cond, detail=""): + print(("ok " if cond else "FAIL ") + desc, flush=True) + if not cond: + self.failures += 1 + if detail: + print(" " + detail.strip().replace("\n", "\n ")) + + +def text(result): + return (result.stdout or b"").decode("utf-8", "replace") + + +class VeraCrypt: + def __init__(self, path, work): + self.path = path + self.binary = os.path.realpath(path) + self.work = work + + def run(self, args, stdin=None, timeout=300): + return subprocess.run([self.path, "--text"] + args, input=stdin, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, cwd=self.work, + timeout=timeout) + + def create(self, path, size, password, filesystem, volume_type="normal"): + return self.run(["--non-interactive", "--create", str(path), + "--size=" + size, "--volume-type=" + volume_type, + "--encryption=AES", "--hash=SHA-512", + "--filesystem=" + filesystem, "-p", password, + "--random-source=" + str(self.work / "random")] + + COMMON) + + def mount(self, volume, password, mount_point=None, extra=()): + args = ["--non-interactive", "-p", password] + MOUNT + list(extra) + args.append(str(volume)) + if mount_point: + args.append(str(mount_point)) + return self.run(args) + + def dismount(self, target): + return self.run(["--non-interactive", "-d", str(target)]) + + def mounted_volumes(self): + """Paths of the mounted volumes; raises if they cannot be listed.""" + r = self.run(["--list"]) + if r.returncode != 0: + if "No volumes mounted" in text(r): + return set() + raise RuntimeError("veracrypt --list failed: " + text(r)) + return {line.split()[1] for line in text(r).splitlines() + if len(line.split()) >= 4 and line.split()[0].endswith(":")} + + def dismount_volumes(self, volumes): + """Dismounts only these volumes; returns whether none remains.""" + paths = {str(volume) for volume in volumes} + for path in paths & self.mounted_volumes(): + self.dismount(path) + return not paths & self.mounted_volumes() + + def service(self, volume): + """PID of the FUSE service that has the volume file open.""" + volume = os.path.realpath(volume) + for entry in os.listdir("/proc"): + if not entry.isdigit(): + continue + try: + if os.readlink("/proc/%s/exe" % entry) != self.binary: + continue + fds = ["/proc/%s/fd/%s" % (entry, fd) + for fd in os.listdir("/proc/%s/fd" % entry)] + targets = {os.readlink(fd) for fd in fds} + except OSError: + continue + if "/dev/fuse" in targets and volume in targets: + return int(entry) + return None + + +def process_status(pid, field): + with open("/proc/%d/status" % pid) as status: + for line in status: + if line.startswith(field + ":"): + return line.split(":", 1)[1].strip() + return "" + + +def core_limit(pid): + with open("/proc/%d/limits" % pid) as limits: + for line in limits: + if line.startswith("Max core file size"): + return line.split()[4:6] + return [] + + +def test_memory(vc, c, volume, mount_point): + r = vc.mount(volume, PASSWORD_A, mount_point) + c.check("#1958: mount", r.returncode == 0, text(r)) + pid = vc.service(volume) + c.check("#1958: find the FUSE service", pid is not None) + if pid: + locked = process_status(pid, "VmLck") + c.check("#1958: the FUSE service has locked memory", + locked.split()[:1] != ["0"], "VmLck: " + locked) + limit = core_limit(pid) + c.check("#1958: the FUSE service cannot write a core file", + limit == ["0", "0"], "core file size limit: %s" % limit) + c.check("#1958: dismount", vc.dismount_volumes([volume])) + + +def main(): + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("--veracrypt", + default=shutil.which("veracrypt") or "veracrypt") + args = parser.parse_args() + if os.geteuid() != 0: + sys.exit("Run as root: the test mounts volumes") + + work = Path(tempfile.mkdtemp(prefix="vc-regression-")) + vc = VeraCrypt(args.veracrypt, work) + c = Checks() + mount_points = [work / name for name in ("a", "b")[:1]] + for mount_point in mount_points: + mount_point.mkdir() + (work / "random").write_bytes(os.urandom(4096)) + try: + volume = work / "a.hc" + r = vc.create(volume, "10M", PASSWORD_A, "ext4") + c.check("create %s" % volume.name, r.returncode == 0, text(r)) + if c.failures: + sys.exit("Cannot create the test volume") + test_memory(vc, c, volume, mount_points[0]) + finally: + # Only the volume of this test: unrelated ones may be mounted + if (not vc.dismount_volumes([work / "a.hc"]) + or any(os.path.ismount(m) for m in mount_points)): + sys.exit("A test volume is still mounted; not removing %s" % work) + shutil.rmtree(work, ignore_errors=True) + + if c.failures: + sys.exit("%d check(s) failed" % c.failures) + print("All checks passed") + + +if __name__ == "__main__": + main()