Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1153?usp=email
to look at the new patch set (#5).
Change subject: options: Review use of positive_atoi vs atoi_constrained
......................................................................
options: Review use of positive_atoi vs atoi_constrained
Replace where it is useful.
While here also add a missing cast in atoi_constrained.
Change-Id: Id440917f433aab1a7db608ba04fa95ba47c2ddde
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/options.c
M src/openvpn/options_util.c
2 files changed, 22 insertions(+), 40 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/53/1153/5
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index f801743..5192127 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -7019,21 +7019,15 @@
else if (streq(p[0], "tun-mtu-max") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
- int max_mtu = positive_atoi(p[1], msglevel);
- if (max_mtu < 68 || max_mtu > 65536)
- {
- msg(msglevel, "--tun-mtu-max value '%s' is invalid", p[1]);
- }
- else
- {
- options->ce.tun_mtu_max = max_mtu;
- }
+ atoi_constrained(p[1], &options->ce.tun_mtu_max, p[0],
TUN_MTU_MAX_MIN, 65536, msglevel);
}
else if (streq(p[0], "tun-mtu-extra") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
- options->ce.tun_mtu_extra = positive_atoi(p[1], msglevel);
- options->ce.tun_mtu_extra_defined = true;
+ if (atoi_constrained(p[1], &options->ce.tun_mtu_extra, p[0], 0, 65536,
msglevel))
+ {
+ options->ce.tun_mtu_extra_defined = true;
+ }
}
else if (streq(p[0], "max-packet-size") && p[1] && !p[2])
{
@@ -7065,11 +7059,8 @@
else if (streq(p[0], "fragment") && p[1] && !p[3])
{
VERIFY_PERMISSION(OPT_P_MTU | OPT_P_CONNECTION);
- options->ce.fragment = positive_atoi(p[1], msglevel);
-
- if (options->ce.fragment < 68)
+ if (!atoi_constrained(p[1], &options->ce.fragment, p[0], 68, INT_MAX,
msglevel))
{
- msg(msglevel, "--fragment needs to be at least 68");
goto err;
}
@@ -7743,12 +7734,14 @@
VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_CONNECTION);
if (p[1])
{
- int mssfix = positive_atoi(p[1], msglevel);
- /* can be 0, but otherwise it needs to be high enough so we can
- * substract room for headers. */
- if (mssfix != 0 && (mssfix < TLS_CHANNEL_MTU_MIN || mssfix >
UINT16_MAX))
+ int mssfix;
+ if (!atoi_constrained(p[1], &mssfix, p[0], 0, UINT16_MAX,
msglevel))
{
- msg(msglevel, "--mssfix value '%s' is invalid", p[1]);
+ goto err;
+ }
+ if (mssfix != 0 && mssfix < TLS_CHANNEL_MTU_MIN)
+ {
+ msg(msglevel, "mssfix needs to be >= %d, not %d",
TLS_CHANNEL_MTU_MIN, mssfix);
goto err;
}
@@ -8001,7 +7994,7 @@
else if (streq(p[0], "max-routes-per-client") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_INHERIT);
- options->max_routes_per_client = max_int(positive_atoi(p[1],
msglevel), 1);
+ atoi_constrained(p[1], &options->max_routes_per_client, p[0], 1,
INT_MAX, msglevel);
}
else if (streq(p[0], "client-cert-not-required") && !p[1])
{
@@ -9808,8 +9801,6 @@
}
else if (streq(p[0], "keying-material-exporter") && p[1] && p[2])
{
- int ekm_length = positive_atoi(p[2], msglevel);
-
VERIFY_PERMISSION(OPT_P_GENERAL);
if (strncmp(p[1], "EXPORTER", 8))
@@ -9823,14 +9814,14 @@
msg(msglevel,
"Keying material exporter label must not be '"
EXPORT_KEY_DATA_LABEL "'.");
}
- if (ekm_length < 16 || ekm_length > 4095)
+
+ if (!atoi_constrained(p[2], &options->keying_material_exporter_length,
+ p[0], 16, 4095, msglevel))
{
- msg(msglevel, "Invalid keying material exporter length");
goto err;
}
options->keying_material_exporter_label = p[1];
- options->keying_material_exporter_length = ekm_length;
}
else if (streq(p[0], "allow-recursive-routing") && !p[1])
{
@@ -9865,15 +9856,14 @@
}
else if (streq(p[0], "vlan-pvid") && p[1] && !p[2])
{
+ int vlan_pvid;
VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_INSTANCE);
- options->vlan_pvid = positive_atoi(p[1], msglevel);
- if (options->vlan_pvid < OPENVPN_8021Q_MIN_VID
- || options->vlan_pvid > OPENVPN_8021Q_MAX_VID)
+ if (!atoi_constrained(p[1], &vlan_pvid, p[0],
+ OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID,
msglevel))
{
- msg(msglevel, "the parameter of --vlan-pvid parameters must be >=
%u and <= %u",
- OPENVPN_8021Q_MIN_VID, OPENVPN_8021Q_MAX_VID);
goto err;
}
+ options->vlan_pvid = (uint16_t)vlan_pvid;
}
else
{
diff --git a/src/openvpn/options_util.c b/src/openvpn/options_util.c
index 8a1c083..2eeecb8 100644
--- a/src/openvpn/options_util.c
+++ b/src/openvpn/options_util.c
@@ -162,11 +162,6 @@
return (int)i;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
bool
atoi_constrained(const char *str, int *value, const char *name, int min, int
max, msglvl_t msglevel)
{
@@ -194,13 +189,10 @@
return false;
}
- *value = i;
+ *value = (int)i;
return true;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
static const char *updatable_options[] = { "block-ipv6", "block-outside-dns",
"dhcp-option", "dns",
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1153?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id440917f433aab1a7db608ba04fa95ba47c2ddde
Gerrit-Change-Number: 1153
Gerrit-PatchSet: 5
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-MessageType: newpatchset
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
