OpenVPN / OpenVPN/openvpn

Destructor for generic/top-level client in plugin not called on conditional restart

Open
#674 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug non-trivial change
Dominant language
C
Stars
14.6k
Forks
3.4k
PR merge metrics
No merged PRs in 30d

Description

Describe the bug

When performing a conditional restart (SIGUSR1) of an OpenVPN server with a plugin, openvpn_plugin_client_constructor_v1 is called to create a new generic/top-level client instance without deconstructing the previous one with openvpn_plugin_client_destructor_v1. When the server is stopped after one or more conditional restarts, only the last instance is destructed.

This not only causes a memory leak, it also creates a logical issue when trying to distinguish the allocation of a generic/top-level client and a real client early on by counting allocations/deallocations.

To reproduce

  1. Write a simple OpenVPN plugin (see below for a minimal example).
    • Make an allocation in openvpn_plugin_client_constructor_v1 and return a pointer to it.
    • Free the allocation in openvpn_plugin_client_destructor_v1.
  2. Start an OpenVPN server with the plugin loaded.
  3. Trigger a conditional restart of the OpenVPN server via SIGUSR1 to observe the issue.

Expected behavior

On a conditional restart, either openvpn_plugin_client_constructor_v1 should not be called, or openvpn_plugin_client_destructor_v1 should be called beforehand, so that there is no memory leak.

Version information

  • OS: Debian Bookworm
  • OpenVPN version: 2.6.13 (also 2.5.9)

Additional context

Minimal example:

#include <openvpn/openvpn-plugin.h>

#define PLUGIN_NAME         "Issue Demo"

OPENVPN_EXPORT int openvpn_plugin_open_v3(
    int version,
    const struct openvpn_plugin_args_open_in *args,
    struct openvpn_plugin_args_open_return *ret)
{
    ret->handle = args->callbacks;
    return OPENVPN_PLUGIN_FUNC_SUCCESS;
}

OPENVPN_EXPORT int openvpn_plugin_func_v3(
    int version,
    const struct openvpn_plugin_args_func_in *args,
    struct openvpn_plugin_args_func_return *ret)
{
    return OPENVPN_PLUGIN_FUNC_ERROR;
}

OPENVPN_EXPORT void openvpn_plugin_close_v1(openvpn_plugin_handle_t handle)
{
}

OPENVPN_EXPORT void *openvpn_plugin_client_constructor_v1(openvpn_plugin_handle_t handle)
{
    struct openvpn_plugin_callbacks *callbacks = handle;

    void *per_client_context = calloc(1, sizeof(int));
    callbacks->plugin_log(PLOG_ERR, PLUGIN_NAME,
        "### openvpn_plugin_client_constructor_v1: %p ###", per_client_context);

    return per_client_context;
}

OPENVPN_EXPORT void openvpn_plugin_client_destructor_v1(
    openvpn_plugin_handle_t handle,
    void *per_client_context)
{
    struct openvpn_plugin_callbacks *callbacks = handle;

    callbacks->plugin_log(PLOG_ERR, PLUGIN_NAME,
        "### openvpn_plugin_client_destructor_v1: %p ###", per_client_context);
    free(per_client_context);
}

OPENVPN_EXPORT int openvpn_plugin_min_version_required_v1()
{
    return 3;
}

Log:

2025-01-23 16:47:15 us=692534 OpenVPN 2.6.13 x86_64-pc-linux-gnu [SSL (OpenSSL)] [LZO] [LZ4] [EPOLL] [PKCS11] [MH/PKTINFO] [AEAD] [DCO]
...
2025-01-23 16:47:15 us=692829 PLUGIN Issue Demo: ### openvpn_plugin_client_constructor_v1: 0x5776fb579170 ###
...
2025-01-23 16:47:15 us=696927 Initialization Sequence Completed
...
2025-01-23 16:47:27 us=366162 SIGUSR1[hard,] received, process restarting
2025-01-23 16:47:27 us=366207 Restart pause, 1 second(s)
...
2025-01-23 16:47:28 us=367471 PLUGIN Issue Demo: ### openvpn_plugin_client_constructor_v1: 0x5776fb596700 ###
...
2025-01-23 16:47:36 us=391133 SIGUSR1[hard,] received, process restarting
2025-01-23 16:47:36 us=391172 Restart pause, 1 second(s)
...
2025-01-23 16:47:37 us=392536 PLUGIN Issue Demo: ### openvpn_plugin_client_constructor_v1: 0x5776fb585640 ###
...
2025-01-23 16:47:37 us=399453 Initialization Sequence Completed
...
^C
...
2025-01-23 16:47:41 us=730149 PLUGIN Issue Demo: ### openvpn_plugin_client_destructor_v1: 0x5776fb585640 ###
...
2025-01-23 16:47:41 us=730235 SIGINT[hard,] received, process exiting

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

Reproduce the conditional restart sequence with the minimal plugin, then trace the lifecycle around openvpn_plugin_client_constructor_v1 and openvpn_plugin_client_destructor_v1. Done means each generic/top-level client instance is destructed before it is replaced, with no allocation left leaked after repeated SIGUSR1 restarts.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.