OpenVPN / OpenVPN/openvpn

Fix Race Condition / I/O Buffering in status_flush() causing truncated/zero-byte status file reads

Open
#1,054 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
14.6k
Forks
3.4k
PR merge metrics
No merged PRs in 30d

Description

Describe the bug
There is a race condition and I/O buffering issue inside status_flush() in src/openvpn/status.c. When OpenVPN periodically updates the status file (e.g., via status-version 1), it calls ftruncate() to resize/truncate the file descriptor before overwriting it with fresh data.

Because OpenVPN does not utilize any kernel-level file locking (flock) or immediate buffer flushing (fsync) during this exact window, external third-party plugins, daemons, or parsers (such as radiusplugin or custom accounting scripts) that poll the status file frequently can hit a race condition. They randomly read the file exactly when it is truncated (0 bytes) or while data is still half-written/cached in the OS I/O buffers. This causes external plugins to parse incomplete tables, leading to critical accounting bugs (e.g., incorrectly reporting 0 bytes for active users).

To Reproduce
Steps to reproduce the behavior:

  1. Configure OpenVPN to log periodic status updates to a specific file path (e.g., status /run/openvpn/status.log 10).
  2. Have multiple active clients connected transferring data.
  3. Run an aggressive asynchronous external script or plugin that continuously reads and parses /run/openvpn/status.log every few seconds.
  4. Observe that occasionally, the external reader hits a race condition exactly during OpenVPN's status_flush() loop, reading a completely empty or partial file, resulting in missed accounting data (0 bytes processed) even though the user is fully active.

Expected behavior
OpenVPN should implement an atomic or synchronized write/flush mechanism on its status file descriptor. While the file is being truncated and rewritten inside status_flush(), it should temporarily acquire an exclusive advisory lock (flock) and force a buffer sync (fsync) right before releasing the lock. This ensures that any external parser attempting to read the file will be safely blocked for a few microseconds by the OS kernel until a 100% complete file structure is ready.

Version information:

  • OS: Linux (e.g., Ubuntu 24.04 / Debian 13 using tmpfs / ramdisk)
  • OpenVPN version: 2.7.x / 2.6.x (Affects all versions using the traditional status_flush logic in status.c)

Additional context
I have locally patched and successfully tested this issue by modifying the status_flush() function inside src/openvpn/status.c. Implementing an exclusive kernel lock (LOCK_EX) and forcing a buffer commit via fsync entirely resolved the random 0-byte parsing issues without adding any measurable CPU or memory overhead.

Here is the exact diff/proposed fix that eliminates the race condition:

void
status_flush(struct status_output *so)
{
    if (so && so->fd >= 0 && (so->flags & STATUS_OUTPUT_WRITE))
    {
        /* Step 1: Apply an exclusive kernel lock BEFORE truncating the file.
         * This prevents external parsers from opening or reading a half-written/empty file. */
        #ifdef HAVE_SYS_FILE_H
        flock(so->fd, LOCK_EX);
        #endif

#if defined(HAVE_FTRUNCATE)
        {
            const off_t off = lseek(so->fd, (off_t)0, SEEK_CUR);
            if (ftruncate(so->fd, off) != 0)
            {
                msg(M_WARN | M_ERRNO, "Failed to truncate status file");
            }
        }
#elif defined(HAVE_CHSIZE)
        {
            const long off = (long)lseek(so->fd, (off_t)0, SEEK_CUR);
            chsize(so->fd, off);
        }
#else /* if defined(HAVE_FTRUNCATE) */
#warning both ftruncate and chsize functions appear to be missing from this OS
#endif

        /* Step 2: Flush internal stdio buffers to ensure data is physically committed */
        fsync(so->fd);

        /* Step 3: Release the kernel lock. Now external hooks can safely read a 100% complete file. */
        #ifdef HAVE_SYS_FILE_H
        flock(so->fd, LOCK_UN);
        #endif

        /* clear read buffer */
        if (buf_defined(&so->read_buf))
        {
            ASSERT(buf_init(&so->read_buf, 0));
        }
    }
}

By ensuring that flock(so->fd, LOCK_EX) encapsulates the ftruncate block and adding fsync(so->fd), the kernel serializes read/write operations seamlessly. I highly recommend merging a similar thread-safe approach into upstream core to prevent data corruption for downstream management tools.

Disclaimer & Technical Note:
Please note that I am sharing this patch based on my own local troubleshooting and testing, which successfully resolved the issue in my environment. As I do not possess deep architectural knowledge of the OpenVPN codebase, I am not 100% certain if this is the most optimal or standard way upstream handles file synchronization, or if it might impact other platforms (like Windows/macOS).

This is purely a constructive suggestion from my testing experience, offered with the utmost respect to the core maintainers. I would highly appreciate the core developers' expert insight on whether this approach is safe or if there is a more idiomatic way within the OpenVPN architecture to achieve the same synchronized/atomic write behavior.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/openvpn/status.c at status_flush() and review how the status file is truncated and rewritten. Reproduce the issue with a periodic status file and an aggressive external reader, then assess the proposed locking and synchronization behavior across supported platforms. Done means external readers no longer observe empty or partial status data without breaking existing status output.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.