diff mbox series

[Openvpn-devel,1/4] mtcp: Handle multi_create_instance() returning NULL

Message ID 20220516185621.6182-2-kprovost@netgate.com
State Accepted
Delegated to: Antonio Quartulli
Headers show
Series [Openvpn-devel,1/4] mtcp: Handle multi_create_instance() returning NULL | expand

Commit Message

Kristof Provost via Openvpn-devel May 16, 2022, 8:56 a.m. UTC 
From: Kristof Provost <kp@FreeBSD.org>

multi_create_instance() can fail (i.e. return NULL).
multi_create_instance_tcp() is ready for this, but called
multi_assign_peer_id() without first checking if mi was non-NULL.
multi_assign_peer_id() assumed that mi is non-NULL, dereferencing it and
causing a crash.

Move the call to multi_assign_peer_id() after the mi NULL check.

Signed-off-by: Kristof Provost <kprovost@netgate.com>
---
 src/openvpn/mtcp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Arne Schwabe May 16, 2022, 9:32 a.m. UTC | #1 
Am 16.05.2022 um 20:56 schrieb Kristof Provost via Openvpn-devel:
> From: Kristof Provost <kp@FreeBSD.org>
>
> multi_create_instance() can fail (i.e. return NULL).
> multi_create_instance_tcp() is ready for this, but called
> multi_assign_peer_id() without first checking if mi was non-NULL.
> multi_assign_peer_id() assumed that mi is non-NULL, dereferencing it and
> causing a crash.
>
> Move the call to multi_assign_peer_id() after the mi NULL check.
>
I did add multi_assign_peer there iirc but Kristof is right.

Acked-By: Arne Schwabe <arne@rfc2549.org
Antonio Quartulli May 17, 2022, 1:51 a.m. UTC | #2 
Hi all,

On 16/05/2022 21:32, Arne Schwabe wrote:
> 
> Am 16.05.2022 um 20:56 schrieb Kristof Provost via Openvpn-devel:
>> From: Kristof Provost <kp@FreeBSD.org>
>>
>> multi_create_instance() can fail (i.e. return NULL).
>> multi_create_instance_tcp() is ready for this, but called
>> multi_assign_peer_id() without first checking if mi was non-NULL.
>> multi_assign_peer_id() assumed that mi is non-NULL, dereferencing it and
>> causing a crash.
>>
>> Move the call to multi_assign_peer_id() after the mi NULL check.
>>
> I did add multi_assign_peer there iirc but Kristof is right.
> 
> Acked-By: Arne Schwabe <arne@rfc2549.org

Thank you both.

I am pulling this patch in the dco branch.

Regards,
diff mbox series

Patch

diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c
index b4445dbe..414a5676 100644
--- a/src/openvpn/mtcp.c
+++ b/src/openvpn/mtcp.c
@@ -124,7 +124,6 @@  multi_create_instance_tcp(struct multi_context *m)
     struct hash *hash = m->hash;
 
     mi = multi_create_instance(m, NULL);
-    multi_assign_peer_id(m, mi);
 
     if (mi)
     {
@@ -132,6 +131,8 @@  multi_create_instance_tcp(struct multi_context *m)
         const uint32_t hv = hash_value(hash, &mi->real);
         struct hash_bucket *bucket = hash_bucket(hash, hv);
 
+        multi_assign_peer_id(m, mi);
+
         he = hash_lookup_fast(hash, bucket, &mi->real, hv);
 
         if (he)