Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1476?usp=email
to look at the new patch set (#5).
Change subject: socket: Fix various conversion warnings
......................................................................
socket: Fix various conversion warnings
- We assume that addrinfo->ai_family always fits
into a sa_family_t, so we do not check those.
- Reduce places where we explicitly pass ai_family
and prefer sock->info.af.
- Make sure that --bind-dev has a suitable length.
- Treat PROTO_* as uint8_t where possible (however,
much code uses proto = -1 for various error handling).
Change-Id: I7be7427480d3540d43dd818eddb6eb5860956459
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/options.c
M src/openvpn/socket.c
M src/openvpn/socket_util.c
3 files changed, 30 insertions(+), 36 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/76/1476/5
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index fdbc678..e1c62bd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6492,7 +6492,15 @@
else if (streq(p[0], "bind-dev") && p[1])
{
VERIFY_PERMISSION(OPT_P_SOCKFLAGS);
- options->bind_dev = p[1];
+ if (strlen(p[1]) <= IFNAMSIZ)
+ {
+ options->bind_dev = p[1];
+ }
+ else
+ {
+ msg(msglevel, "argument to --bind-dev is longer than allowed %u",
IFNAMSIZ);
+ goto err;
+ }
}
#endif
else if (streq(p[0], "txqueuelen") && p[1] && !p[2])
@@ -6578,11 +6586,9 @@
}
else if (streq(p[0], "proto") && p[1] && !p[2])
{
- int proto;
- sa_family_t af;
VERIFY_PERMISSION(OPT_P_GENERAL | OPT_P_CONNECTION);
- proto = ascii2proto(p[1]);
- af = ascii2af(p[1]);
+ int proto = ascii2proto(p[1]);
+ sa_family_t af = ascii2af(p[1]);
if (proto < 0)
{
msg(msglevel, "Bad protocol: '%s'. Allowed protocols with --proto
option: %s", p[1],
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index b5f000a..d2f6bbe 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -649,28 +649,23 @@
}
static void
-bind_local(struct link_socket *sock, const sa_family_t ai_family)
+bind_local(struct link_socket *sock)
{
/* bind to local address/port */
if (sock->bind_local)
{
if (sock->socks_proxy && sock->info.proto == PROTO_UDP)
{
- socket_bind(sock->ctrl_sd, sock->info.lsa->bind_local, ai_family,
"SOCKS", false);
+ socket_bind(sock->ctrl_sd, sock->info.lsa->bind_local,
sock->info.af, "SOCKS", false);
}
else
{
- socket_bind(sock->sd, sock->info.lsa->bind_local, ai_family,
"TCP/UDP",
+ socket_bind(sock->sd, sock->info.lsa->bind_local, sock->info.af,
"TCP/UDP",
sock->info.bind_ipv6_only);
}
}
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
static void
create_socket(struct link_socket *sock, struct addrinfo *addr)
{
@@ -702,7 +697,7 @@
}
/* Set af field of sock->info, so it always reflects the address family
* of the created socket */
- sock->info.af = addr->ai_family;
+ sock->info.af = (sa_family_t)addr->ai_family;
/* set socket buffers based on --sndbuf and --rcvbuf options */
socket_set_buffers(sock->sd, &sock->socket_buffer_sizes, true);
@@ -714,8 +709,9 @@
if (sock->bind_dev)
{
msg(M_INFO, "Using bind-dev %s", sock->bind_dev);
+ /* Note: We verify strlen of bind_dev in options parsing */
if (setsockopt(sock->sd, SOL_SOCKET, SO_BINDTODEVICE, sock->bind_dev,
- strlen(sock->bind_dev) + 1)
+ (socklen_t)(strlen(sock->bind_dev) + 1))
!= 0)
{
msg(M_WARN | M_ERRNO, "WARN: setsockopt SO_BINDTODEVICE=%s
failed", sock->bind_dev);
@@ -723,13 +719,9 @@
}
#endif
- bind_local(sock, addr->ai_family);
+ bind_local(sock);
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
#ifdef TARGET_ANDROID
static void
protect_fd_nonlocal(int fd, const struct sockaddr *addr)
@@ -1161,13 +1153,8 @@
}
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#endif
-
static void
-resolve_bind_local(struct link_socket *sock, const sa_family_t af)
+resolve_bind_local(struct link_socket *sock)
{
struct gc_arena gc = gc_new();
@@ -1183,12 +1170,12 @@
}
/* will return AF_{INET|INET6}from local_host */
- status = get_cached_dns_entry(sock->dns_cache, sock->local_host,
sock->local_port, af,
+ status = get_cached_dns_entry(sock->dns_cache, sock->local_host,
sock->local_port, sock->info.af,
flags, &sock->info.lsa->bind_local);
if (status)
{
- status = openvpn_getaddrinfo(flags, sock->local_host,
sock->local_port, 0, NULL, af,
+ status = openvpn_getaddrinfo(flags, sock->local_host,
sock->local_port, 0, NULL, sock->info.af,
&sock->info.lsa->bind_local);
}
@@ -1209,7 +1196,7 @@
/* the resolved 'local entry' might have a different family than
* what was globally configured
*/
- sock->info.af = sock->info.lsa->bind_local->ai_family;
+ sock->info.af = (sa_family_t)sock->info.lsa->bind_local->ai_family;
}
}
@@ -1405,7 +1392,8 @@
sock->mark = o->mark;
sock->bind_dev = o->bind_dev;
- sock->info.proto = proto;
+ ASSERT(proto >= 0 && proto < PROTO_N);
+ sock->info.proto = (uint8_t)proto;
sock->info.af = o->ce.af;
sock->info.remote_float = o->ce.remote_float;
sock->info.lsa = &c->c1.link_socket_addrs[sock_index];
@@ -1471,7 +1459,7 @@
{
if (sock->bind_local)
{
- resolve_bind_local(sock, sock->info.af);
+ resolve_bind_local(sock);
}
resolve_remote(sock, 1, NULL);
}
@@ -1732,9 +1720,9 @@
* and we should not connect a remote */
if (sock->info.af == AF_UNSPEC)
{
+ sock->info.af =
(sa_family_t)sock->info.lsa->bind_local->ai_family;
msg(M_WARN, "Could not determine IPv4/IPv6 protocol. Using %s",
- addr_family_name(sock->info.lsa->bind_local->ai_family));
- sock->info.af = sock->info.lsa->bind_local->ai_family;
+ addr_family_name(sock->info.af));
}
create_socket(sock, sock->info.lsa->bind_local);
}
@@ -2374,8 +2362,8 @@
else if (cmsg != NULL)
{
msg(M_WARN,
- "CMSG received that cannot be parsed (cmsg_level=%d, cmsg_type=%d,
cmsg=len=%d)",
- (int)cmsg->cmsg_level, (int)cmsg->cmsg_type, (int)cmsg->cmsg_len);
+ "CMSG received that cannot be parsed (cmsg_level=%d, cmsg_type=%d,
cmsg=len=%zu)",
+ cmsg->cmsg_level, cmsg->cmsg_type, (size_t)cmsg->cmsg_len);
}
return buf->len;
diff --git a/src/openvpn/socket_util.c b/src/openvpn/socket_util.c
index 0194f38..913ac69 100644
--- a/src/openvpn/socket_util.c
+++ b/src/openvpn/socket_util.c
@@ -356,7 +356,7 @@
const char *short_form;
const char *display_form;
sa_family_t proto_af;
- int proto;
+ uint8_t proto;
};
/* Indexed by PROTO_x */
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1476?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7be7427480d3540d43dd818eddb6eb5860956459
Gerrit-Change-Number: 1476
Gerrit-PatchSet: 5
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
