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 (#4).
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).
- Some of the usual "recv* returns ssize_t not int"
adaptions.
Change-Id: I7be7427480d3540d43dd818eddb6eb5860956459
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/options.c
M src/openvpn/socket.c
M src/openvpn/socket.h
M src/openvpn/socket_util.c
4 files changed, 50 insertions(+), 55 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/76/1476/4
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 85669e0..0f2bd03 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6515,7 +6515,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])
@@ -6601,11 +6609,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 331249d..a214093 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -661,28 +661,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)
{
@@ -714,7 +709,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);
@@ -726,8 +721,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);
@@ -735,13 +731,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)
@@ -1144,7 +1136,7 @@
static void stream_buf_close(struct stream_buf *sb);
-static bool stream_buf_added(struct stream_buf *sb, int length_added);
+static bool stream_buf_added(struct stream_buf *sb, ssize_t length_added);
/* For stream protocols, allocate a buffer to build up packet.
* Called after frame has been finalized. */
@@ -1173,13 +1165,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();
@@ -1195,12 +1182,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);
}
@@ -1221,7 +1208,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;
}
}
@@ -1417,7 +1404,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];
@@ -1483,7 +1471,7 @@
{
if (sock->bind_local)
{
- resolve_bind_local(sock, sock->info.af);
+ resolve_bind_local(sock);
}
resolve_remote(sock, 1, NULL);
}
@@ -1744,9 +1732,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);
}
@@ -2142,12 +2130,13 @@
}
static bool
-stream_buf_added(struct stream_buf *sb, int length_added)
+stream_buf_added(struct stream_buf *sb, ssize_t length_added)
{
- dmsg(D_STREAM_DEBUG, "STREAM: ADD length_added=%d", length_added);
+ dmsg(D_STREAM_DEBUG, "STREAM: ADD length_added=%zd", length_added);
if (length_added > 0)
{
- sb->buf.len += length_added;
+ ASSERT(sb->buf.len + length_added <= INT_MAX);
+ sb->buf.len += (int)length_added;
}
/* if length unknown, see if we can get the length prefix from
@@ -2253,7 +2242,7 @@
int
link_socket_read_tcp(struct link_socket *sock, struct buffer *buf)
{
- int len = 0;
+ ssize_t len = 0;
if (!sock->stream_buf.residual_fully_formed)
{
@@ -2284,7 +2273,7 @@
}
if (len <= 0)
{
- return buf->len = len;
+ return buf->len = (int)len;
}
}
@@ -2334,13 +2323,15 @@
mesg.msg_name = &from->dest.addr;
mesg.msg_namelen = fromlen;
mesg.msg_control = pktinfo_buf;
- mesg.msg_controllen = sizeof pktinfo_buf;
- buf->len = recvmsg(sock->sd, &mesg, 0);
- if (buf->len >= 0)
+ mesg.msg_controllen = (socklen_t)sizeof(pktinfo_buf);
+ ssize_t len = recvmsg(sock->sd, &mesg, 0);
+ ASSERT(len >= INT_MIN && len <= INT_MAX);
+ /* Note that we communicate recvmsg failure to caller here */
+ buf->len = (int)len;
+ if (len >= 0)
{
- struct cmsghdr *cmsg;
fromlen = mesg.msg_namelen;
- cmsg = CMSG_FIRSTHDR(&mesg);
+ struct cmsghdr *cmsg = CMSG_FIRSTHDR(&mesg);
if (cmsg != NULL && CMSG_NXTHDR(&mesg, cmsg) == NULL
#if defined(HAVE_IN_PKTINFO) && defined(HAVE_IPI_SPEC_DST)
&& cmsg->cmsg_level == SOL_IP && cmsg->cmsg_type == IP_PKTINFO
@@ -2375,8 +2366,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);
}
}
@@ -2403,8 +2394,8 @@
else
#endif
{
- buf->len = recvfrom(sock->sd, BPTR(buf), buf_forward_capacity(buf), 0,
&from->dest.addr.sa,
- &fromlen);
+ buf->len = (int)recvfrom(sock->sd, BPTR(buf),
buf_forward_capacity(buf),
+ 0, &from->dest.addr.sa, &fromlen);
}
/* FIXME: won't do anything when sock->info.af == AF_UNSPEC */
if (buf->len >= 0 && expectedlen && fromlen != expectedlen)
@@ -2423,8 +2414,9 @@
ssize_t
link_socket_write_tcp(struct link_socket *sock, struct buffer *buf, struct
link_socket_actual *to)
{
- packet_size_type len = BLEN(buf);
- dmsg(D_STREAM_DEBUG, "STREAM: WRITE %d offset=%d", (int)len, buf->offset);
+ ASSERT(BLEN(buf) >= 0 && BLEN(buf) <= PACKET_SIZE_MAX);
+ packet_size_type len = (packet_size_type)BLEN(buf);
+ dmsg(D_STREAM_DEBUG, "STREAM: WRITE %u offset=%d", len, buf->offset);
ASSERT(len <= sock->stream_buf.maxlen);
len = htonps(len);
ASSERT(buf_write_prepend(buf, &len, sizeof(len)));
@@ -2435,10 +2427,6 @@
#endif
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
#if ENABLE_IP_PKTINFO
ssize_t
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index 850064d..192a03c 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -54,6 +54,7 @@
*/
typedef uint16_t packet_size_type;
+#define PACKET_SIZE_MAX UINT16_MAX
/* convert a packet_size_type from host to network order */
#define htonps(x) htons(x)
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: 4
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
