diff mbox series

[Openvpn-devel,v1] buffer: Change limits for array_mult_safe

Message ID 20251212100920.7671-1-gert@greenie.muc.de
State New
Headers show
Series [Openvpn-devel,v1] buffer: Change limits for array_mult_safe | expand

Commit Message

Gert Doering Dec. 12, 2025, 10:09 a.m. UTC 
From: Frank Lichtenheld <frank@lichtenheld.com>

- Lower the limit to 1GB on 32bit systems.
  The limit of 4GB-1 makes no sense on systems that usually
  don't allow a single process to allocate anything near to
  this limit.
- Increate the limit from 4GB-1 to 4GB on other systems. It
  makes no difference in protection but makes it much easier
  to use the limit in other contexts, e.g. if dividing it.

Change-Id: I4f95edd7ce2098180aa620a231727217f333a12d
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Gert Doering <gert@greenie.muc.de>
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1436
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1436
This mail reflects revision 1 of this Change.

Acked-by according to Gerrit (reflected above):
Gert Doering <gert@greenie.muc.de>

Comments

Gert Doering Dec. 12, 2025, 11:57 a.m. UTC | #1 
As discussed on IRC, this is a reasonable change - on 32bit platforms,
having a maximum allocation size of 4G-1 is no good, as that will always
fail (so why bother having that check at all).  1G seems well out of
range of what OpenVPN would ever need, but it *would* catch some program
parts asking for "2Gbyte" and then being surprised that malloc() fails...
(and provide a better FATAL message).

Not tested beyond "BB says it's all green".

Your patch has been applied to the master branch.

commit 2b8149a4fe0d73ea0bedf2e282188d55c8511c45
Author: Frank Lichtenheld
Date:   Fri Dec 12 11:09:14 2025 +0100

     buffer: Change limits for array_mult_safe

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1436
     Message-Id: <20251212100920.7671-1-gert@greenie.muc.de>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg35044.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/buffer.c b/src/openvpn/buffer.c
index 293622f..40baca6 100644
--- a/src/openvpn/buffer.c
+++ b/src/openvpn/buffer.c
@@ -39,7 +39,7 @@ 
 size_t
 array_mult_safe(const size_t m1, const size_t m2, const size_t extra)
 {
-    const size_t limit = 0xFFFFFFFF;
+    const size_t limit = ALLOC_SIZE_MAX;
     unsigned long long res =
         (unsigned long long)m1 * (unsigned long long)m2 + (unsigned long long)extra;
     if (unlikely(m1 > limit) || unlikely(m2 > limit) || unlikely(extra > limit)
diff --git a/src/openvpn/buffer.h b/src/openvpn/buffer.h
index ab2a29d..1dbe0b2 100644
--- a/src/openvpn/buffer.h
+++ b/src/openvpn/buffer.h
@@ -1044,6 +1044,18 @@ 
  * Allocate memory to hold a structure
  */
 
+/* When allocating arrays make sure we do not use a excessive amount
+ * of memory.
+ */
+#if UINTPTR_MAX <= UINT32_MAX
+/* 1 GB on 32bit systems, they usually can only allocate 2 GB for the
+ * whole process.
+ */
+#define ALLOC_SIZE_MAX (1u << 30)
+#else
+#define ALLOC_SIZE_MAX ((size_t)1 << 32) /* 4 GB */
+#endif
+
 #define ALLOC_OBJ(dptr, type)                                       \
     {                                                               \
         check_malloc_return((dptr) = (type *)malloc(sizeof(type))); \