From mboxrd@z Thu Jan 1 00:00:00 1970 From: Stefano Brivio To: passt-dev@passt.top Subject: [PATCH 04/16] treewide: Unchecked return value from library, CWE-252 Date: Tue, 05 Apr 2022 19:05:02 +0200 Message-ID: <20220405170514.2963773-5-sbrivio@redhat.com> In-Reply-To: <20220405170514.2963773-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============3657046349050472554==" --===============3657046349050472554== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 v= oid *addr, if (!ih) return 1; =20 + if (ih->type !=3D ICMP_ECHO && ih->type !=3D ICMP_ECHOREPLY) + return 1; + sa.sin_port =3D ih->un.echo.id; =20 iref.icmp.id =3D id =3D ntohs(ih->un.echo.id); @@ -179,8 +182,9 @@ int icmp_tap_handler(const struct ctx *c, int af, const v= oid *addr, bitmap_set(icmp_act[V4], id); =20 sa.sin_addr =3D *(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 =3D=3D AF_INET6) { union icmp_epoll_ref iref =3D { .icmp.v6 =3D 1 }; struct sockaddr_in6 sa =3D { @@ -216,8 +220,9 @@ int icmp_tap_handler(const struct ctx *c, int af, const v= oid *addr, bitmap_set(icmp_act[V6], id); =20 sa.sin6_addr =3D *(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"); } =20 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; =20 #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 =20 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; =20 if (*v4 =3D=3D IP_VERSION_PROBE) { @@ -168,7 +170,9 @@ v6: return 0; } =20 - n =3D nl_req(0, buf, &req, sizeof(req)); + if ((n =3D nl_req(0, buf, &req, sizeof(req))) < 0) + return 0; + nh =3D (struct nlmsghdr *)buf; =20 for ( ; NLMSG_OK(nh, n); nh =3D NLMSG_NEXT(nh, n)) { @@ -289,7 +293,8 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, v= oid *gw) struct rtattr *rta; struct rtmsg *rtm; char buf[BUFSIZ]; - size_t n, na; + ssize_t n; + size_t na; =20 if (set) { if (af =3D=3D AF_INET6) { @@ -323,8 +328,7 @@ void nl_route(int ns, unsigned int ifi, sa_family_t af, v= oid *gw) req.nlh.nlmsg_flags |=3D NLM_F_DUMP; } =20 - n =3D nl_req(ns, buf, &req, req.nlh.nlmsg_len); - if (set) + if (set || (n =3D nl_req(ns, buf, &req, req.nlh.nlmsg_len)) < 0) return; =20 nh =3D (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; =20 if (set) { if (af =3D=3D AF_INET6) { @@ -430,8 +435,7 @@ void nl_addr(int ns, unsigned int ifi, sa_family_t af, req.nlh.nlmsg_flags |=3D NLM_F_DUMP; } =20 - n =3D nl_req(ns, buf, &req, req.nlh.nlmsg_len); - if (set) + if (set || (n =3D nl_req(ns, buf, &req, req.nlh.nlmsg_len)) < 0) return; =20 nh =3D (struct nlmsghdr *)buf; @@ -504,14 +508,17 @@ void nl_link(int ns, unsigned int ifi, void *mac, int u= p, int mtu) struct nlmsghdr *nh; struct rtattr *rta; char buf[BUFSIZ]; - size_t n, na; + ssize_t n; + size_t na; =20 if (!MAC_IS_ZERO(mac)) { req.nlh.nlmsg_len =3D sizeof(req); memcpy(req.set.mac, mac, ETH_ALEN); req.rta.rta_type =3D IFLA_ADDRESS; req.rta.rta_len =3D 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 =3D 0; } =20 @@ -520,17 +527,20 @@ void nl_link(int ns, unsigned int ifi, void *mac, int u= p, int mtu) req.set.mtu.mtu =3D mtu; req.rta.rta_type =3D IFLA_MTU; req.rta.rta_len =3D 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 =3D 0; } =20 - if (up) - nl_req(ns, buf, &req, req.nlh.nlmsg_len); + if (up && nl_req(ns, buf, &req, req.nlh.nlmsg_len) < 0) + return; =20 if (change) return; =20 - n =3D nl_req(ns, buf, &req, req.nlh.nlmsg_len); + if ((n =3D nl_req(ns, buf, &req, req.nlh.nlmsg_len)) < 0) + return; =20 nh =3D (struct nlmsghdr *)buf; for ( ; NLMSG_OK(nh, n); nh =3D 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 =3D 1; i < UNIX_SOCK_MAX; i++) { s =3D 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"); =20 if (s < 0) { perror("socket"); @@ -263,8 +265,11 @@ valid_args: } =20 tv.tv_usec =3D 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); =20 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 =3D htonl(len); =20 - send(c->fd_tap, &vnet_len, 4, flags); + if (send(c->fd_tap, &vnet_len, 4, flags) < 0) + return -1; } =20 return send(c->fd_tap, data, len, flags); @@ -150,7 +151,8 @@ void tap_ip_send(const struct ctx *c, const struct in6_ad= dr *src, uint8_t proto, ih->checksum =3D csum_unaligned(ih, len, 0); } =20 - 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 =3D (struct ipv6hdr *)(eh + 1); char *data =3D (char *)(ip6h + 1); @@ -201,7 +203,8 @@ void tap_ip_send(const struct ctx *c, const struct in6_ad= dr *src, uint8_t proto, ip6h->flow_lbl[2] =3D (flow >> 0) & 0xff; } =20 - 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); } } =20 @@ -862,11 +865,13 @@ static void tap_sock_unix_new(struct ctx *c) =20 c->fd_tap =3D accept4(c->fd_tap_listen, NULL, NULL, 0); =20 - 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); =20 - 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); =20 ev.data.fd =3D c->fd_tap; ev.events =3D 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 =3D=3D -1) return; =20 - 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); =20 - 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); } =20 /** @@ -1547,7 +1547,8 @@ static void tcp_l2_buf_flush_part(const struct ctx *c, =20 missing =3D end - sent; p =3D (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); } =20 /** @@ -2010,6 +2011,7 @@ static void tcp_clamp_window(const struct ctx *c, struc= t tcp_conn *conn, unsigned wnd) { uint32_t prev_scaled =3D conn->wnd_from_tap << conn->ws_from_tap; + int s =3D conn->sock; =20 wnd <<=3D conn->ws_from_tap; wnd =3D MIN(MAX_WINDOW, wnd); @@ -2025,7 +2027,9 @@ static void tcp_clamp_window(const struct ctx *c, struc= t tcp_conn *conn, } =20 conn->wnd_from_tap =3D 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); } =20 @@ -2209,7 +2213,8 @@ static void tcp_conn_from_tap(struct ctx *c, int af, co= nst void *addr, conn->wnd_to_tap =3D WINDOW_DEFAULT; =20 mss =3D 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); =20 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; } =20 - 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); + } } =20 if (!(conn->events & ESTABLISHED)) @@ -428,7 +435,11 @@ static int tcp_splice_connect(const struct ctx *c, struc= t tcp_splice_conn *conn, if (s < 0) tcp_sock_set_bufsize(c, conn->b); =20 - 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); + } =20 if (CONN_V6(conn)) { sa =3D (struct sockaddr *)&addr6; @@ -565,8 +576,11 @@ void tcp_sock_handler_splice(struct ctx *c, union epoll_= ref ref, if ((s =3D accept4(ref.r.s, NULL, NULL, SOCK_NONBLOCK)) < 0) return; =20 - 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); + } =20 conn =3D CONN(c->tcp.splice_conn_count++); conn->a =3D s; @@ -817,10 +831,17 @@ static void tcp_splice_pipe_refill(const struct ctx *c) continue; } =20 - 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); + } } } =20 @@ -850,15 +871,21 @@ void tcp_splice_timer(struct ctx *c) =20 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); } =20 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); } =20 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_re= f ref, uint32_t events, =20 last_mh->msg_iov =3D &last_mh->msg_iov[i]; =20 - 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); =20 *iov_base -=3D first_offset; break; diff --git a/util.c b/util.c index 81cf744..a4d2cd1 100644 --- a/util.c +++ b/util.c @@ -154,7 +154,8 @@ void passt_vsyslog(int pri, const char *format, va_list a= p) if (log_opt & LOG_PERROR) fprintf(stderr, "%s", buf + sizeof("<0>")); =20 - send(log_sock, buf, n, 0); + if (send(log_sock, buf, n, 0)) + fprintf(stderr, "Failed to send %i bytes to syslog\n", n); } =20 #define IPV6_NH_OPT(nh) \ @@ -239,7 +240,7 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto, u= int16_t port, }; const struct sockaddr *sa; struct epoll_event ev; - int fd, sl, one =3D 1; + int fd, sl, y =3D 1; =20 if (proto !=3D IPPROTO_TCP && proto !=3D IPPROTO_UDP && proto !=3D IPPROTO_ICMP && proto !=3D IPPROTO_ICMPV6) @@ -287,10 +288,12 @@ int sock_l4(const struct ctx *c, int af, uint8_t proto,= uint16_t port, sa =3D (const struct sockaddr *)&addr6; sl =3D sizeof(addr6); =20 - 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); } =20 - 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); =20 if (bind(fd, sa, sl) < 0) { /* We'll fail to bind to low ports if we don't have enough --=20 2.35.1 --===============3657046349050472554==--