[Openvpn-devel] Handle exceeding 'max-clients'

Message ID 20220808143424.65924-2-kprovost@netgate.com
State Not Applicable
Headers show
Series [Openvpn-devel] Handle exceeding 'max-clients' | expand

Commit Message

Kristof Provost via Openvpn-devel Aug. 8, 2022, 4:34 a.m. UTC
From: Kristof Provost <kp@FreeBSD.org>

If 'max-clients' is set multi_create_instance() can return NULL (for any
client that would take us over the client limit).

If mi is NULL we don't add it to the hash map, but we do potentially
dereference it to increment the session count.
Do not attempt to do so if 'mi == NULL'.

Signed-off-by: Kristof Provost <kprovost@netgate.com>
---
 src/openvpn/mudp.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

Comments

Kristof Provost via Openvpn-devel Aug. 8, 2022, 4:35 a.m. UTC | #1
Apologies,

This patch isn’t part of the series (and has in fact already landed), but got sent because of a stray patch file in my working tree.

Kristof

On 8 Aug 2022, at 16:34, Kristof Provost wrote:
> From: Kristof Provost <kp@FreeBSD.org>
>
> If 'max-clients' is set multi_create_instance() can return NULL (for any
> client that would take us over the client limit).
>
> If mi is NULL we don't add it to the hash map, but we do potentially
> dereference it to increment the session count.
> Do not attempt to do so if 'mi == NULL'.
>
> Signed-off-by: Kristof Provost <kprovost@netgate.com>
> ---
>  src/openvpn/mudp.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
> index 0810fada..0cbca1a9 100644
> --- a/src/openvpn/mudp.c
> +++ b/src/openvpn/mudp.c
> @@ -241,15 +241,16 @@ multi_get_create_instance_udp(struct multi_context *m, bool *floated)
>                          hash_add_fast(hash, bucket, &mi->real, hv, mi);
>                          mi->did_real_hash = true;
>                          multi_assign_peer_id(m, mi);
> -                    }
> -                    /* If we have a session id already, ensure that the
> -                     * state is using the same */
> -                    if (session_id_defined(&state.server_session_id)
> -                        && session_id_defined((&state.peer_session_id)))
> -                    {
> -                        mi->context.c2.tls_multi->n_sessions++;
> -                        struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
> -                        session_skip_to_pre_start(session, &state, &m->top.c2.from);
> +
> +                        /* If we have a session id already, ensure that the
> +                         * state is using the same */
> +                        if (session_id_defined(&state.server_session_id)
> +                            && session_id_defined((&state.peer_session_id)))
> +                        {
> +                            mi->context.c2.tls_multi->n_sessions++;
> +                            struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
> +                            session_skip_to_pre_start(session, &state, &m->top.c2.from);
> +                        }
>                      }
>                  }
>                  else
> -- 
> 2.37.0
Gert Doering Aug. 8, 2022, 5:25 a.m. UTC | #2
Hi,

On Mon, Aug 08, 2022 at 04:35:22PM +0200, Kristof Provost via Openvpn-devel wrote:
> Apologies,
> 
> This patch isn???t part of the series (and has in fact already landed), but got sent because of a stray patch file in my working tree.

Thanks for clarifying.  I was a bit puzzled and was about to figure
out why it seemed like I had seen it before :-)

Closing this as "not applicable" in patchwork.

gert

Patch

diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index 0810fada..0cbca1a9 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -241,15 +241,16 @@  multi_get_create_instance_udp(struct multi_context *m, bool *floated)
                         hash_add_fast(hash, bucket, &mi->real, hv, mi);
                         mi->did_real_hash = true;
                         multi_assign_peer_id(m, mi);
-                    }
-                    /* If we have a session id already, ensure that the
-                     * state is using the same */
-                    if (session_id_defined(&state.server_session_id)
-                        && session_id_defined((&state.peer_session_id)))
-                    {
-                        mi->context.c2.tls_multi->n_sessions++;
-                        struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
-                        session_skip_to_pre_start(session, &state, &m->top.c2.from);
+
+                        /* If we have a session id already, ensure that the
+                         * state is using the same */
+                        if (session_id_defined(&state.server_session_id)
+                            && session_id_defined((&state.peer_session_id)))
+                        {
+                            mi->context.c2.tls_multi->n_sessions++;
+                            struct tls_session *session = &mi->context.c2.tls_multi->session[TM_ACTIVE];
+                            session_skip_to_pre_start(session, &state, &m->top.c2.from);
+                        }
                     }
                 }
                 else