From 22ed4467a413961816f317c641e436ca627d06d2 Mon Sep 17 00:00:00 2001 From: Stefano Brivio Date: Tue, 5 Apr 2022 07:10:30 +0200 Subject: treewide: Unchecked return value from library, CWE-252 All instances were harmless, but it might be useful to have some debug messages here and there. Reported by Coverity. Signed-off-by: Stefano Brivio --- icmp.c | 13 +++++++++---- netlink.c | 40 +++++++++++++++++++++++++--------------- qrap.c | 13 +++++++++---- tap.c | 19 ++++++++++++------- tcp.c | 19 ++++++++++++------- tcp_splice.c | 53 ++++++++++++++++++++++++++++++++++++++++------------- udp.c | 3 ++- util.c | 11 +++++++---- 8 files changed, 116 insertions(+), 55 deletions(-) diff --git a/icmp.c b/icmp.c index 0eb5bfe..8abc94b 100644 --- a/icmp.c +++ b/icmp.c @@ -160,6 +160,9 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, if (!ih) return 1; + if (ih->type != ICMP_ECHO && ih->type != ICMP_ECHOREPLY) + return 1; + sa.sin_port = ih->un.echo.id; iref.icmp.id = id = ntohs(ih->un.echo.id); @@ -179,8 +182,9 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, bitmap_set(icmp_act[V4], id); sa.sin_addr = *(struct in_addr *)addr; - sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL, - (struct sockaddr *)&sa, sizeof(sa)); + if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL, + (struct sockaddr *)&sa, sizeof(sa)) < 0) + debug("ICMP: failed to relay request to socket"); } else if (af == AF_INET6) { union icmp_epoll_ref iref = { .icmp.v6 = 1 }; struct sockaddr_in6 sa = { @@ -216,8 +220,9 @@ int icmp_tap_handler(const struct ctx *c, int af, const void *addr, bitmap_set(icmp_act[V6], id); sa.sin6_addr = *(struct in6_addr *)addr; - sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL, - (struct sockaddr *)&sa, sizeof(sa)); + if (sendto(s, ih, sizeof(*ih) + plen, MSG_NOSIGNAL, + (struct sockaddr *)&sa, sizeof(sa)) < 1) + debug("ICMPv6: failed to relay request to socket"); } return 1; diff --git a/netlink.c b/netlink.c index 5902dc4..78c1186 100644 --- a/netlink.c +++ b/netlink.c @@ -60,7 +60,8 @@ ns: return 0; #ifdef NETLINK_GET_STRICT_CHK - setsockopt(*s, SOL_NETLINK, NETLINK_GET_STRICT_CHK, &y, sizeof(y)); + if (setsockopt(*s, SOL_NETLINK, NETLINK_GET_STRICT_CHK, &y, sizeof(y))) + debug("netlink: cannot set NETLINK_GET_STRICT_CHK on %i", *s); #endif ns_enter((struct ctx *)arg); @@ -152,7 +153,8 @@ unsigned int nl_get_ext_if(int *v4, int *v6) char buf[BUFSIZ]; long *word, tmp; uint8_t *vmap; - size_t n, na; + ssize_t n; + size_t na; int *v; if (*v4 == IP_VERSION_PROBE) { @@ -168,7 +170,9 @@ v6: return 0; } - n = nl_req(0, buf, &req, sizeof(req)); + if ((n = nl_req(0, buf, &req, sizeof(req))) < 0) + return 0; + nh = (struct nlmsghdr *)buf; for ( ; NLMSG_OK(nh, n); nh = NLMSG_NEXT(nh, n)) { @@ -289,7 +293,8 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw) struct rtattr *rta; struct rtmsg *rtm; char buf[BUFSIZ]; - size_t n, na; + ssize_t n; + size_t na; if (set) { if (af == AF_INET6) { @@ -323,8 +328,7 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, void *gw) req.nlh.nlmsg_flags |= NLM_F_DUMP; } - n = nl_req(ns, buf, &req, req.nlh.nlmsg_len); - if (set) + if (set || (n = nl_req(ns, buf, &req, req.nlh.nlmsg_len)) < 0) return; nh = (struct nlmsghdr *)buf; @@ -398,7 +402,8 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af, struct nlmsghdr *nh; struct rtattr *rta; char buf[BUFSIZ]; - size_t n, na; + ssize_t n; + size_t na; if (set) { if (af == AF_INET6) { @@ -430,8 +435,7 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af, req.nlh.nlmsg_flags |= NLM_F_DUMP; } - n = nl_req(ns, buf, &req, req.nlh.nlmsg_len); - if (set) + if (set || (n = nl_req(ns, buf, &req, req.nlh.nlmsg_len)) < 0) return; nh = (struct nlmsghdr *)buf; @@ -504,14 +508,17 @@ void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu) struct nlmsghdr *nh; struct rtattr *rta; char buf[BUFSIZ]; - size_t n, na; + ssize_t n; + size_t na; if (!MAC_IS_ZERO(mac)) { req.nlh.nlmsg_len = sizeof(req); memcpy(req.set.mac, mac, ETH_ALEN); req.rta.rta_type = IFLA_ADDRESS; req.rta.rta_len = RTA_LENGTH(ETH_ALEN); - nl_req(ns, buf, &req, req.nlh.nlmsg_len); + if (nl_req(ns, buf, &req, req.nlh.nlmsg_len) < 0) + return; + up = 0; } @@ -520,17 +527,20 @@ void nl_link(int ns, unsigned int ifi, void *mac, int up, int mtu) req.set.mtu.mtu = mtu; req.rta.rta_type = IFLA_MTU; req.rta.rta_len = RTA_LENGTH(sizeof(unsigned int)); - nl_req(ns, buf, &req, req.nlh.nlmsg_len); + if (nl_req(ns, buf, &req, req.nlh.nlmsg_len) < 0) + return; + up = 0; } - if (up) - nl_req(ns, buf, &req, req.nlh.nlmsg_len); + if (up && nl_req(ns, buf, &req, req.nlh.nlmsg_len) < 0) + return; if (change) return; - n = nl_req(ns, buf, &req, req.nlh.nlmsg_len); + if ((n = nl_req(ns, buf, &req, req.nlh.nlmsg_len)) < 0) + return; nh = (struct nlmsghdr *)buf; for ( ; NLMSG_OK(nh, n); nh = NLMSG_NEXT(nh, n)) { diff --git a/qrap.c b/qrap.c index 3ee73a7..50eea89 100644 --- a/qrap.c +++ b/qrap.c @@ -234,8 +234,10 @@ int main(int argc, char **argv) valid_args: for (i = 1; i < UNIX_SOCK_MAX; i++) { s = socket(AF_UNIX, SOCK_STREAM, 0); - setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); - setsockopt(s, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); + if (setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv))) + perror("setsockopt SO_RCVTIMEO"); + if (setsockopt(s, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv))) + perror("setsockopt SO_SNDTIMEO"); if (s < 0) { perror("socket"); @@ -263,8 +265,11 @@ valid_args: } tv.tv_usec = 0; - setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv)); - setsockopt(s, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv)); + if (setsockopt(s, SOL_SOCKET, SO_RCVTIMEO, &tv, sizeof(tv))) + perror("setsockopt, SO_RCVTIMEO reset"); + if (setsockopt(s, SOL_SOCKET, SO_SNDTIMEO, &tv, sizeof(tv))) + perror("setsockopt, SO_SNDTIMEO reset"); + fprintf(stderr, "Connected to %s\n", addr.sun_path); if (dup2(s, (int)fd) < 0) { diff --git a/tap.c b/tap.c index f8222a2..e4dd804 100644 --- a/tap.c +++ b/tap.c @@ -84,7 +84,8 @@ int tap_send(const struct ctx *c, const void *data, size_t len, int vnet_pre) } else { uint32_t vnet_len = htonl(len); - send(c->fd_tap, &vnet_len, 4, flags); + if (send(c->fd_tap, &vnet_len, 4, flags) < 0) + return -1; } return send(c->fd_tap, data, len, flags); @@ -150,7 +151,8 @@ void tap_ip_send(const struct ctx *c, const struct in6_addr *src, uint8_t proto, ih->checksum = csum_unaligned(ih, len, 0); } - tap_send(c, buf, len + sizeof(*iph) + sizeof(*eh), 1); + if (tap_send(c, buf, len + sizeof(*iph) + sizeof(*eh), 1) < 0) + debug("tap: failed to send %lu bytes (IPv4)", len); } else { struct ipv6hdr *ip6h = (struct ipv6hdr *)(eh + 1); char *data = (char *)(ip6h + 1); @@ -201,7 +203,8 @@ void tap_ip_send(const struct ctx *c, const struct in6_addr *src, uint8_t proto, ip6h->flow_lbl[2] = (flow >> 0) & 0xff; } - tap_send(c, buf, len + sizeof(*ip6h) + sizeof(*eh), 1); + if (tap_send(c, buf, len + sizeof(*ip6h) + sizeof(*eh), 1) < 1) + debug("tap: failed to send %lu bytes (IPv6)", len); } } @@ -862,11 +865,13 @@ static void tap_sock_unix_new(struct ctx *c) c->fd_tap = accept4(c->fd_tap_listen, NULL, NULL, 0); - if (!c->low_rmem) - setsockopt(c->fd_tap, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v)); + if (!c->low_rmem && + setsockopt(c->fd_tap, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v))) + trace("tap: failed to set SO_RCVBUF to %i", v); - if (!c->low_wmem) - setsockopt(c->fd_tap, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v)); + if (!c->low_wmem && + setsockopt(c->fd_tap, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v))) + trace("tap: failed to set SO_SNDBUF to %i", v); ev.data.fd = c->fd_tap; ev.events = EPOLLIN | EPOLLET | EPOLLRDHUP; diff --git a/tcp.c b/tcp.c index 858eb41..92cefab 100644 --- a/tcp.c +++ b/tcp.c @@ -1053,11 +1053,11 @@ void tcp_sock_set_bufsize(const struct ctx *c, int s) if (s == -1) return; - if (!c->low_rmem) - setsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v)); + if (!c->low_rmem && setsockopt(s, SOL_SOCKET, SO_RCVBUF, &v, sizeof(v))) + trace("TCP: failed to set SO_RCVBUF to %i", v); - if (!c->low_wmem) - setsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v)); + if (!c->low_wmem && setsockopt(s, SOL_SOCKET, SO_SNDBUF, &v, sizeof(v))) + trace("TCP: failed to set SO_SNDBUF to %i", v); } /** @@ -1547,7 +1547,8 @@ static void tcp_l2_buf_flush_part(const struct ctx *c, missing = end - sent; p = (char *)iov->iov_base + iov->iov_len - missing; - send(c->fd_tap, p, missing, MSG_NOSIGNAL); + if (send(c->fd_tap, p, missing, MSG_NOSIGNAL)) + debug("TCP: failed to flush %lu missing bytes to tap", missing); } /** @@ -2010,6 +2011,7 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_conn *conn, unsigned wnd) { uint32_t prev_scaled = conn->wnd_from_tap << conn->ws_from_tap; + int s = conn->sock; wnd <<= conn->ws_from_tap; wnd = MIN(MAX_WINDOW, wnd); @@ -2025,7 +2027,9 @@ static void tcp_clamp_window(const struct ctx *c, struct tcp_conn *conn, } conn->wnd_from_tap = MIN(wnd >> conn->ws_from_tap, USHRT_MAX); - setsockopt(conn->sock, SOL_TCP, TCP_WINDOW_CLAMP, &wnd, sizeof(wnd)); + if (setsockopt(s, SOL_TCP, TCP_WINDOW_CLAMP, &wnd, sizeof(wnd))) + trace("TCP: failed to set TCP_WINDOW_CLAMP on socket %i", s); + conn_flag(c, conn, WND_CLAMPED); } @@ -2209,7 +2213,8 @@ static void tcp_conn_from_tap(struct ctx *c, int af, const void *addr, conn->wnd_to_tap = WINDOW_DEFAULT; mss = tcp_conn_tap_mss(c, conn, opts, optlen); - setsockopt(s, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss)); + if (setsockopt(s, SOL_TCP, TCP_MAXSEG, &mss, sizeof(mss))) + trace("TCP: failed to set TCP_MAXSEG on socket %i", s); MSS_SET(conn, mss); tcp_get_tap_ws(conn, opts, optlen); diff --git a/tcp_splice.c b/tcp_splice.c index 24a3b4b..84df8ed 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -376,8 +376,15 @@ static int tcp_splice_connect_finish(const struct ctx *c, return -EIO; } - fcntl(conn->pipe_a_b[0], F_SETPIPE_SZ, c->tcp.pipe_size); - fcntl(conn->pipe_b_a[0], F_SETPIPE_SZ, c->tcp.pipe_size); + if (fcntl(conn->pipe_a_b[0], F_SETPIPE_SZ, c->tcp.pipe_size)) { + trace("TCP (spliced): cannot set a->b pipe size to %lu", + c->tcp.pipe_size); + } + + if (fcntl(conn->pipe_b_a[0], F_SETPIPE_SZ, c->tcp.pipe_size)) { + trace("TCP (spliced): cannot set b->a pipe size to %lu", + c->tcp.pipe_size); + } } if (!(conn->events & ESTABLISHED)) @@ -428,7 +435,11 @@ static int tcp_splice_connect(const struct ctx *c, struct tcp_splice_conn *conn, if (s < 0) tcp_sock_set_bufsize(c, conn->b); - setsockopt(conn->b, SOL_TCP, TCP_QUICKACK, &((int){ 1 }), sizeof(int)); + if (setsockopt(conn->b, SOL_TCP, TCP_QUICKACK, + &((int){ 1 }), sizeof(int))) { + trace("TCP (spliced): failed to set TCP_QUICKACK on socket %i", + conn->b); + } if (CONN_V6(conn)) { sa = (struct sockaddr *)&addr6; @@ -565,8 +576,11 @@ void tcp_sock_handler_splice(struct ctx *c, union epoll_ref ref, if ((s = accept4(ref.r.s, NULL, NULL, SOCK_NONBLOCK)) < 0) return; - setsockopt(s, SOL_TCP, TCP_QUICKACK, &((int){ 1 }), - sizeof(int)); + if (setsockopt(s, SOL_TCP, TCP_QUICKACK, &((int){ 1 }), + sizeof(int))) { + trace("TCP (spliced): failed to set TCP_QUICKACK on %i", + s); + } conn = CONN(c->tcp.splice_conn_count++); conn->a = s; @@ -817,10 +831,17 @@ static void tcp_splice_pipe_refill(const struct ctx *c) continue; } - fcntl(splice_pipe_pool[i][0][0], F_SETPIPE_SZ, - c->tcp.pipe_size); - fcntl(splice_pipe_pool[i][1][0], F_SETPIPE_SZ, - c->tcp.pipe_size); + if (fcntl(splice_pipe_pool[i][0][0], F_SETPIPE_SZ, + c->tcp.pipe_size)) { + trace("TCP (spliced): cannot set a->b pipe size to %lu", + c->tcp.pipe_size); + } + + if (fcntl(splice_pipe_pool[i][1][0], F_SETPIPE_SZ, + c->tcp.pipe_size)) { + trace("TCP (spliced): cannot set b->a pipe size to %lu", + c->tcp.pipe_size); + } } } @@ -850,15 +871,21 @@ void tcp_splice_timer(struct ctx *c) if ( (conn->flags & RCVLOWAT_SET_A) && !(conn->flags & RCVLOWAT_ACT_A)) { - setsockopt(conn->a, SOL_SOCKET, SO_RCVLOWAT, - &((int){ 1 }), sizeof(int)); + if (setsockopt(conn->a, SOL_SOCKET, SO_RCVLOWAT, + &((int){ 1 }), sizeof(int))) { + trace("TCP (spliced): can't set SO_RCVLOWAT on " + "%i", conn->a); + } conn_flag(c, conn, ~RCVLOWAT_SET_A); } if ( (conn->flags & RCVLOWAT_SET_B) && !(conn->flags & RCVLOWAT_ACT_B)) { - setsockopt(conn->b, SOL_SOCKET, SO_RCVLOWAT, - &((int){ 1 }), sizeof(int)); + if (setsockopt(conn->b, SOL_SOCKET, SO_RCVLOWAT, + &((int){ 1 }), sizeof(int))) { + trace("TCP (spliced): can't set SO_RCVLOWAT on " + "%i", conn->b); + } conn_flag(c, conn, ~RCVLOWAT_SET_B); } diff --git a/udp.c b/udp.c index 1c0fdc6..cbd3ac8 100644 --- a/udp.c +++ b/udp.c @@ -938,7 +938,8 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, uint32_t events, last_mh->msg_iov = &last_mh->msg_iov[i]; - sendmsg(c->fd_tap, last_mh, MSG_NOSIGNAL); + if (sendmsg(c->fd_tap, last_mh, MSG_NOSIGNAL) < 0) + debug("UDP: %li bytes to tap missing", missing); *iov_base -= first_offset; break; diff --git a/util.c b/util.c index 81cf744..c3475d6 100644 --- a/util.c +++ b/util.c @@ -154,7 +154,8 @@ void passt_vsyslog(int pri, const char *format, va_list ap) if (log_opt & LOG_PERROR) fprintf(stderr, "%s", buf + sizeof("<0>")); - send(log_sock, buf, n, 0); + if (send(log_sock, buf, n, 0) != n) + fprintf(stderr, "Failed to send %i bytes to syslog\n", n); } #define IPV6_NH_OPT(nh) \ @@ -239,7 +240,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto, uint16_t port, }; const struct sockaddr *sa; struct epoll_event ev; - int fd, sl, one = 1; + int fd, sl, y = 1; if (proto != IPPROTO_TCP && proto != IPPROTO_UDP && proto != IPPROTO_ICMP && proto != IPPROTO_ICMPV6) @@ -287,10 +288,12 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto, uint16_t port, sa = (const struct sockaddr *)&addr6; sl = sizeof(addr6); - setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &one, sizeof(one)); + if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &y, sizeof(y))) + debug("Failed to set IPV6_V6ONLY on socket %i", fd); } - setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)); + if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) + debug("Failed to set IPV6_V6ONLY on socket %i", fd); if (bind(fd, sa, sl) < 0) { /* We'll fail to bind to low ports if we don't have enough -- cgit v1.2.3