diff mbox series

[Openvpn-devel] mss/mtu: make all size calculations use size_t

Message ID 20231008104022.20200-1-frank@lichtenheld.com
State Accepted
Headers show
Series [Openvpn-devel] mss/mtu: make all size calculations use size_t | expand

Commit Message

Frank Lichtenheld Oct. 8, 2023, 10:40 a.m. UTC 
Half of them used unsigned int, the other half size_t.
Standardize on one. Could've also standardized on the
other, both are much too big for the expected numbers
anyway.

Add a new utility function clamp_size_to_int for
cases we need to change from size_t to int (there
are a lot of those all over our codebase).

Resolves some -Wconversion warnings.

Change-Id: Ic996eca227d9e68279a454db93fcbc86a7bd0380
Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
---

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/+/269
This mail reflects revision 3 of this Change.
Acked-by according to Gerrit (reflected above):
Arne Schwabe <arne-openvpn@rfc2549.org>

Comments

Gert Doering Oct. 19, 2023, 4:58 p.m. UTC | #1 
Not a huge fan of size_t (8 bytes instead of 4 bytes for an unsigned int,
on normal LP64 platforms), but this is all non-critical code, not stored
and not in the packet forwarding path.  So not NAKing an already-ACKed
patch again...

Smoke-tested locally and on GHA.

Your patch has been applied to the master branch.

commit 00685421aefcc294581d6e74371e744acdce6bbf
Author: Frank Lichtenheld
Date:   Sun Oct 8 12:40:22 2023 +0200

     mss/mtu: make all size calculations use size_t

     Signed-off-by: Frank Lichtenheld <frank@lichtenheld.com>
     Acked-by: Arne Schwabe <arne-openvpn@rfc2549.org>
     Message-Id: <20231008104022.20200-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/search?l=mid&q=20231008104022.20200-1-frank@lichtenheld.com
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/integer.h b/src/openvpn/integer.h
index 30b9ecf..a0e421d 100644
--- a/src/openvpn/integer.h
+++ b/src/openvpn/integer.h
@@ -36,6 +36,13 @@ 
                    ((uint64_t)ntohl((uint32_t)((x) & 0xFFFFFFFF)) << 32) | ntohl((uint32_t)((x) >> 32)))
 #endif
 
+static inline int
+clamp_size_to_int(size_t size)
+{
+    ASSERT(size <= INT_MAX);
+    return (int)size;
+}
+
 /*
  * min/max functions
  */
@@ -201,8 +208,8 @@ 
 /**
  * Rounds down num to the nearest multiple of multiple
  */
-static inline unsigned int
-round_down_uint(unsigned int num, unsigned int multiple)
+static inline size_t
+round_down_size(size_t num, size_t multiple)
 {
     return (num / multiple) * multiple;
 }
diff --git a/src/openvpn/mss.c b/src/openvpn/mss.c
index d7ee4c2..108b370 100644
--- a/src/openvpn/mss.c
+++ b/src/openvpn/mss.c
@@ -207,8 +207,8 @@ 
     }
 }
 
-static inline unsigned int
-adjust_payload_max_cbc(const struct key_type *kt, unsigned int target)
+static inline size_t
+adjust_payload_max_cbc(const struct key_type *kt, size_t target)
 {
     if (!cipher_kt_mode_cbc(kt->cipher))
     {
@@ -221,13 +221,13 @@ 
         /* With CBC we need at least one extra byte for padding and then need
          * to ensure that the resulting CBC ciphertext length, which is always
          * a multiple of the block size, is not larger than the target value */
-        unsigned int block_size = cipher_kt_block_size(kt->cipher);
-        target = round_down_uint(target, block_size);
+        size_t block_size = cipher_kt_block_size(kt->cipher);
+        target = round_down_size(target, block_size);
         return target - 1;
     }
 }
 
-static unsigned int
+static size_t
 get_ip_encap_overhead(const struct options *options,
                       const struct link_socket_info *lsi)
 {
@@ -258,7 +258,7 @@ 
                          struct link_socket_info *lsi)
 {
 #if defined(ENABLE_FRAGMENT)
-    unsigned int overhead;
+    size_t overhead;
 
     overhead = frame_calculate_protocol_header_size(kt, options, false);
 
@@ -267,12 +267,12 @@ 
         overhead += get_ip_encap_overhead(options, lsi);
     }
 
-    unsigned int target = options->ce.fragment - overhead;
+    size_t target = options->ce.fragment - overhead;
     /* The 4 bytes of header that fragment adds itself. The other extra payload
      * bytes (Ethernet header/compression) are handled by the fragment code
      * just as part of the payload and therefore automatically taken into
      * account if the packet needs to fragmented */
-    frame->max_fragment_size = adjust_payload_max_cbc(kt, target) - 4;
+    frame->max_fragment_size = clamp_size_to_int(adjust_payload_max_cbc(kt, target)) - 4;
 
     if (cipher_kt_mode_cbc(kt->cipher))
     {
@@ -296,7 +296,7 @@ 
         return;
     }
 
-    unsigned int overhead, payload_overhead;
+    size_t overhead, payload_overhead;
 
     overhead = frame_calculate_protocol_header_size(kt, options, false);
 
@@ -325,7 +325,7 @@ 
      * by ce.mssfix */
 
     /* This is the target value our payload needs to be smaller */
-    unsigned int target = options->ce.mssfix - overhead;
+    size_t target = options->ce.mssfix - overhead;
     frame->mss_fix = (uint16_t)(adjust_payload_max_cbc(kt, target) - payload_overhead);
 
 
diff --git a/src/openvpn/mtu.c b/src/openvpn/mtu.c
index 389d140..df6ba13 100644
--- a/src/openvpn/mtu.c
+++ b/src/openvpn/mtu.c
@@ -174,7 +174,7 @@ 
      */
     const char *ciphername = o->ciphername;
 
-    unsigned int overhead = 0;
+    size_t overhead = 0;
 
     if (strcmp(o->ciphername, "BF-CBC") == 0)
     {
@@ -192,7 +192,7 @@ 
      * the ciphers are actually valid for non tls in occ calucation */
     init_key_type(&occ_kt, ciphername, o->authname, true, false);
 
-    unsigned int payload = frame_calculate_payload_size(frame, o, &occ_kt);
+    size_t payload = frame_calculate_payload_size(frame, o, &occ_kt);
     overhead += frame_calculate_protocol_header_size(&occ_kt, o, true);
 
     return payload + overhead;