[Openvpn-devel,v2] Require at least 100MB of mlock()-able memory if --mlock is used.

Message ID 20210308071023.18269-1-gert@greenie.muc.de
State Superseded
Headers show
Series [Openvpn-devel,v2] Require at least 100MB of mlock()-able memory if --mlock is used. | expand

Commit Message

Gert Doering March 7, 2021, 8:10 p.m. UTC
If --mlock is used, the amount of memory OpenVPN can use is guarded
by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this
is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
as soon as the first TLS handshake comes it, OpenVPN will crash due
to "ouf of memory", and might even end up in a crash loop.

Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
many concurrent clients) of memory.  TLS renegotiation with EC keys
requires up to 90 MB of transient memory.

So: with this patch, we check if getrlimit() is available, and if yes,
log the amount of mlock'able memory.  If the amount is below 100 MB,
which is an arbitrary value "large enough for most smaller deployments",
we try to increase the limits to 100 MB, and abort if this fails.

v2:
  change arbitrary number to 100 MB, introduce #define for it
  not only check but also increase with setrlimit()
  uncrustify fixes

Trac: #1390

Signed-off-by: Gert Doering <gert@greenie.muc.de>
---
 configure.ac                         |  2 +-
 doc/man-sections/generic-options.rst |  7 +++++++
 src/openvpn/platform.c               | 29 ++++++++++++++++++++++++++++
 src/openvpn/platform.h               |  4 ++++
 4 files changed, 41 insertions(+), 1 deletion(-)

Comments

Selva Nair March 9, 2021, 10:54 a.m. UTC | #1
Hi,

On Mon, Mar 8, 2021 at 2:11 AM Gert Doering <gert@greenie.muc.de> wrote:

> If --mlock is used, the amount of memory OpenVPN can use is guarded
> by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this
> is usually 64 Kbyte, which is enough for OpenVPN to initialize, but
> as soon as the first TLS handshake comes it, OpenVPN will crash due
> to "ouf of memory", and might even end up in a crash loop.
>
> Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with
> many concurrent clients) of memory.  TLS renegotiation with EC keys
> requires up to 90 MB of transient memory.
>
> So: with this patch, we check if getrlimit() is available, and if yes,
> log the amount of mlock'able memory.  If the amount is below 100 MB,
> which is an arbitrary value "large enough for most smaller deployments",
> we try to increase the limits to 100 MB, and abort if this fails.
>
> v2:
>   change arbitrary number to 100 MB, introduce #define for it
>   not only check but also increase with setrlimit()
>   uncrustify fixes
>
> Trac: #1390
>
> Signed-off-by: Gert Doering <gert@greenie.muc.de>
> ---
>  configure.ac                         |  2 +-
>  doc/man-sections/generic-options.rst |  7 +++++++
>  src/openvpn/platform.c               | 29 ++++++++++++++++++++++++++++
>  src/openvpn/platform.h               |  4 ++++
>  4 files changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/configure.ac b/configure.ac
> index 1ab8fe59..c65df3e2 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -645,7 +645,7 @@ AC_FUNC_FORK
>
>  AC_CHECK_FUNCS([ \
>         daemon chroot getpwnam setuid nice system getpid dup dup2 \
> -       getpass syslog openlog mlockall getgrnam setgid \
> +       getpass syslog openlog mlockall getrlimit getgrnam setgid \
>         setgroups stat flock readv writev time gettimeofday \
>         ctime memset vsnprintf strdup \
>         setsid chdir putenv getpeername unlink \
> diff --git a/doc/man-sections/generic-options.rst
> b/doc/man-sections/generic-options.rst
> index d5f08839..c026529e 100644
> --- a/doc/man-sections/generic-options.rst
> +++ b/doc/man-sections/generic-options.rst
> @@ -237,6 +237,13 @@ which mode OpenVPN is configured as.
>    likely fail. The limit can be increased using ulimit or systemd
>    directives depending on how OpenVPN is started.
>
> +  If the platform has the getrlimit(2) system call, OpenVPN will check
> +  for the amount of mlock-able memory before calling mlockall(2), and
> +  tries to increase the limit to 100 MB if less than this is configured.
> +  100 Mb is somewhat arbitrary - it is enough for a moderately-sized
> +  OpenVPN deployment, but the memory usage might go beyond that if the
> +  number of concurrent clients is high.
> +
>  --nice n
>    Change process priority after initialization (``n`` greater than 0 is
>    lower priority, ``n`` less than zero is higher priority).
> diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
> index ef688c23..dfbf50c0 100644
> --- a/src/openvpn/platform.c
> +++ b/src/openvpn/platform.c
> @@ -193,6 +193,35 @@ void
>  platform_mlockall(bool print_msg)
>  {
>  #ifdef HAVE_MLOCKALL
> +
> +#ifdef HAVE_GETRLIMIT
> +#define MIN_LOCKED_MEM_MB 100
> +    struct rlimit rl;
> +    if (getrlimit(RLIMIT_MEMLOCK, &rl) < 0)
> +    {
> +        msg(M_WARN | M_ERRNO, "WARNING: getrlimit(RLIMIT_MEMLOCK)
> failed");
> +    }
> +    else
> +    {
> +        msg(M_INFO, "mlock: MEMLOCK limit: soft=%ld KB, hard=%ld KB",
> +            ((long int) rl.rlim_cur) / 1024, ((long int) rl.rlim_max) /
> 1024);
> +        if (rl.rlim_cur < MIN_LOCKED_MEM_MB*1024*1024)
> +        {
> +            msg(M_INFO, "mlock: RLIMIT_MEMLOCK < %d MB, increase limit",
> +                MIN_LOCKED_MEM_MB);
> +            rl.rlim_cur = MIN_LOCKED_MEM_MB*1024*1024;
> +            if (rl.rlim_max < rl.rlim_cur)
> +            {
> +                rl.rlim_max = rl.rlim_cur;
> +            }
> +            if (setrlimit(RLIMIT_MEMLOCK, &rl) < 0)
> +            {
> +                msg(M_FATAL | M_ERRNO, "ERROR: setrlimit() failed");
> +            }
> +        }
> +    }
> +#endif
> +
>      if (mlockall(MCL_CURRENT | MCL_FUTURE))
>      {
>          msg(M_WARN | M_ERRNO, "WARNING: mlockall call failed");
> diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h
> index 01f3200c..02c23e38 100644
> --- a/src/openvpn/platform.h
> +++ b/src/openvpn/platform.h
> @@ -48,6 +48,10 @@
>  #include <stdio.h>
>  #endif
>
> +#ifdef HAVE_GETRLIMIT
> +#include <sys/resource.h>
> +#endif
> +
>  #include "basic.h"
>  #include "buffer.h"
>
> --
> 2.26.2
>

Looks good to me. We call mlockall (and now setrlimit) early before
dropping privileges, so this should take care of at least the naive user
error of running with "--mlock --user foo" and the default low memlock
limit.

Acked-by: selva.nair@gmail.com

Selva
<div dir="ltr"><div dir="ltr">Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Mar 8, 2021 at 2:11 AM Gert Doering &lt;<a href="mailto:gert@greenie.muc.de">gert@greenie.muc.de</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">If --mlock is used, the amount of memory OpenVPN can use is guarded<br>
by the RLIMIT_MEMLOCK value (see mlockall(2)).  The OS default for this<br>
is usually 64 Kbyte, which is enough for OpenVPN to initialize, but<br>
as soon as the first TLS handshake comes it, OpenVPN will crash due<br>
to &quot;ouf of memory&quot;, and might even end up in a crash loop.<br>
<br>
Steady-state OpenVPN requires between 8 MB and 30-50 MB (servers with<br>
many concurrent clients) of memory.  TLS renegotiation with EC keys<br>
requires up to 90 MB of transient memory.<br>
<br>
So: with this patch, we check if getrlimit() is available, and if yes,<br>
log the amount of mlock&#39;able memory.  If the amount is below 100 MB,<br>
which is an arbitrary value &quot;large enough for most smaller deployments&quot;,<br>
we try to increase the limits to 100 MB, and abort if this fails.<br>
<br>
v2:<br>
  change arbitrary number to 100 MB, introduce #define for it<br>
  not only check but also increase with setrlimit()<br>
  uncrustify fixes<br>
<br>
Trac: #1390<br>
<br>
Signed-off-by: Gert Doering &lt;<a href="mailto:gert@greenie.muc.de" target="_blank">gert@greenie.muc.de</a>&gt;<br>
---<br>
 <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>                         |  2 +-<br>
 doc/man-sections/generic-options.rst |  7 +++++++<br>
 src/openvpn/platform.c               | 29 ++++++++++++++++++++++++++++<br>
 src/openvpn/platform.h               |  4 ++++<br>
 4 files changed, 41 insertions(+), 1 deletion(-)<br>
<br>
diff --git a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a> b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
index 1ab8fe59..c65df3e2 100644<br>
--- a/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
+++ b/<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a><br>
@@ -645,7 +645,7 @@ AC_FUNC_FORK<br>
<br>
 AC_CHECK_FUNCS([ \<br>
        daemon chroot getpwnam setuid nice system getpid dup dup2 \<br>
-       getpass syslog openlog mlockall getgrnam setgid \<br>
+       getpass syslog openlog mlockall getrlimit getgrnam setgid \<br>
        setgroups stat flock readv writev time gettimeofday \<br>
        ctime memset vsnprintf strdup \<br>
        setsid chdir putenv getpeername unlink \<br>
diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst<br>
index d5f08839..c026529e 100644<br>
--- a/doc/man-sections/generic-options.rst<br>
+++ b/doc/man-sections/generic-options.rst<br>
@@ -237,6 +237,13 @@ which mode OpenVPN is configured as.<br>
   likely fail. The limit can be increased using ulimit or systemd<br>
   directives depending on how OpenVPN is started.<br>
<br>
+  If the platform has the getrlimit(2) system call, OpenVPN will check<br>
+  for the amount of mlock-able memory before calling mlockall(2), and<br>
+  tries to increase the limit to 100 MB if less than this is configured.<br>
+  100 Mb is somewhat arbitrary - it is enough for a moderately-sized<br>
+  OpenVPN deployment, but the memory usage might go beyond that if the<br>
+  number of concurrent clients is high.<br>
+<br>
 --nice n<br>
   Change process priority after initialization (``n`` greater than 0 is<br>
   lower priority, ``n`` less than zero is higher priority).<br>
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c<br>
index ef688c23..dfbf50c0 100644<br>
--- a/src/openvpn/platform.c<br>
+++ b/src/openvpn/platform.c<br>
@@ -193,6 +193,35 @@ void<br>
 platform_mlockall(bool print_msg)<br>
 {<br>
 #ifdef HAVE_MLOCKALL<br>
+<br>
+#ifdef HAVE_GETRLIMIT<br>
+#define MIN_LOCKED_MEM_MB 100<br>
+    struct rlimit rl;<br>
+    if (getrlimit(RLIMIT_MEMLOCK, &amp;rl) &lt; 0)<br>
+    {<br>
+        msg(M_WARN | M_ERRNO, &quot;WARNING: getrlimit(RLIMIT_MEMLOCK) failed&quot;);<br>
+    }<br>
+    else<br>
+    {<br>
+        msg(M_INFO, &quot;mlock: MEMLOCK limit: soft=%ld KB, hard=%ld KB&quot;,<br>
+            ((long int) rl.rlim_cur) / 1024, ((long int) rl.rlim_max) / 1024);<br>
+        if (rl.rlim_cur &lt; MIN_LOCKED_MEM_MB*1024*1024)<br>
+        {<br>
+            msg(M_INFO, &quot;mlock: RLIMIT_MEMLOCK &lt; %d MB, increase limit&quot;,<br>
+                MIN_LOCKED_MEM_MB);<br>
+            rl.rlim_cur = MIN_LOCKED_MEM_MB*1024*1024;<br>
+            if (rl.rlim_max &lt; rl.rlim_cur)<br>
+            {<br>
+                rl.rlim_max = rl.rlim_cur;<br>
+            }<br>
+            if (setrlimit(RLIMIT_MEMLOCK, &amp;rl) &lt; 0)<br>
+            {<br>
+                msg(M_FATAL | M_ERRNO, &quot;ERROR: setrlimit() failed&quot;);<br>
+            }<br>
+        }<br>
+    }<br>
+#endif<br>
+<br>
     if (mlockall(MCL_CURRENT | MCL_FUTURE))<br>
     {<br>
         msg(M_WARN | M_ERRNO, &quot;WARNING: mlockall call failed&quot;);<br>
diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h<br>
index 01f3200c..02c23e38 100644<br>
--- a/src/openvpn/platform.h<br>
+++ b/src/openvpn/platform.h<br>
@@ -48,6 +48,10 @@<br>
 #include &lt;stdio.h&gt;<br>
 #endif<br>
<br>
+#ifdef HAVE_GETRLIMIT<br>
+#include &lt;sys/resource.h&gt;<br>
+#endif<br>
+<br>
 #include &quot;basic.h&quot;<br>
 #include &quot;buffer.h&quot;<br>
<br>
-- <br>
2.26.2<br></blockquote><div><br></div><div>Looks good to me. We call mlockall (and now setrlimit) early before dropping privileges, so this should take care of at least the naive user error of running with &quot;--mlock --user foo&quot; and the default low memlock limit.</div><div><br></div><div>Acked-by: <a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a></div><div><br></div><div>Selva</div></div></div>

Patch

diff --git a/configure.ac b/configure.ac
index 1ab8fe59..c65df3e2 100644
--- a/configure.ac
+++ b/configure.ac
@@ -645,7 +645,7 @@  AC_FUNC_FORK
 
 AC_CHECK_FUNCS([ \
 	daemon chroot getpwnam setuid nice system getpid dup dup2 \
-	getpass syslog openlog mlockall getgrnam setgid \
+	getpass syslog openlog mlockall getrlimit getgrnam setgid \
 	setgroups stat flock readv writev time gettimeofday \
 	ctime memset vsnprintf strdup \
 	setsid chdir putenv getpeername unlink \
diff --git a/doc/man-sections/generic-options.rst b/doc/man-sections/generic-options.rst
index d5f08839..c026529e 100644
--- a/doc/man-sections/generic-options.rst
+++ b/doc/man-sections/generic-options.rst
@@ -237,6 +237,13 @@  which mode OpenVPN is configured as.
   likely fail. The limit can be increased using ulimit or systemd
   directives depending on how OpenVPN is started.
 
+  If the platform has the getrlimit(2) system call, OpenVPN will check
+  for the amount of mlock-able memory before calling mlockall(2), and
+  tries to increase the limit to 100 MB if less than this is configured.
+  100 Mb is somewhat arbitrary - it is enough for a moderately-sized
+  OpenVPN deployment, but the memory usage might go beyond that if the
+  number of concurrent clients is high.
+
 --nice n
   Change process priority after initialization (``n`` greater than 0 is
   lower priority, ``n`` less than zero is higher priority).
diff --git a/src/openvpn/platform.c b/src/openvpn/platform.c
index ef688c23..dfbf50c0 100644
--- a/src/openvpn/platform.c
+++ b/src/openvpn/platform.c
@@ -193,6 +193,35 @@  void
 platform_mlockall(bool print_msg)
 {
 #ifdef HAVE_MLOCKALL
+
+#ifdef HAVE_GETRLIMIT
+#define MIN_LOCKED_MEM_MB 100
+    struct rlimit rl;
+    if (getrlimit(RLIMIT_MEMLOCK, &rl) < 0)
+    {
+        msg(M_WARN | M_ERRNO, "WARNING: getrlimit(RLIMIT_MEMLOCK) failed");
+    }
+    else
+    {
+        msg(M_INFO, "mlock: MEMLOCK limit: soft=%ld KB, hard=%ld KB",
+            ((long int) rl.rlim_cur) / 1024, ((long int) rl.rlim_max) / 1024);
+        if (rl.rlim_cur < MIN_LOCKED_MEM_MB*1024*1024)
+        {
+            msg(M_INFO, "mlock: RLIMIT_MEMLOCK < %d MB, increase limit",
+                MIN_LOCKED_MEM_MB);
+            rl.rlim_cur = MIN_LOCKED_MEM_MB*1024*1024;
+            if (rl.rlim_max < rl.rlim_cur)
+            {
+                rl.rlim_max = rl.rlim_cur;
+            }
+            if (setrlimit(RLIMIT_MEMLOCK, &rl) < 0)
+            {
+                msg(M_FATAL | M_ERRNO, "ERROR: setrlimit() failed");
+            }
+        }
+    }
+#endif
+
     if (mlockall(MCL_CURRENT | MCL_FUTURE))
     {
         msg(M_WARN | M_ERRNO, "WARNING: mlockall call failed");
diff --git a/src/openvpn/platform.h b/src/openvpn/platform.h
index 01f3200c..02c23e38 100644
--- a/src/openvpn/platform.h
+++ b/src/openvpn/platform.h
@@ -48,6 +48,10 @@ 
 #include <stdio.h>
 #endif
 
+#ifdef HAVE_GETRLIMIT
+#include <sys/resource.h>
+#endif
+
 #include "basic.h"
 #include "buffer.h"