From 2198df2fbd4187ac5e8e8c26f574dbe96e2ebfd0 Mon Sep 17 00:00:00 2001 From: Albert Chu Date: Wed, 7 Dec 2016 11:43:48 -0800 Subject: [PATCH] Remove warnings/re-arch/cleanup to remove warnings in ipmipower IPv6 code and get style closer to FreeIPMI's. Add mem-leak fix. Fix support for get session challenge workaround. --- ipmipower/ipmipower.c | 63 ++++++++++++++++----------- ipmipower/ipmipower.h | 11 ++++- ipmipower/ipmipower_connection.c | 73 +++++++++++++++++++++----------- ipmipower/ipmipower_powercmd.c | 24 ++++------- 4 files changed, 105 insertions(+), 66 deletions(-) diff --git a/ipmipower/ipmipower.c b/ipmipower/ipmipower.c index d30595de2..09719e19a 100644 --- a/ipmipower/ipmipower.c +++ b/ipmipower/ipmipower.c @@ -209,7 +209,7 @@ _eliminate_nodes (void) } static void -_sendto (cbuf_t cbuf, int fd, struct sockaddr_in6 *destaddr) +_sendto (cbuf_t cbuf, int fd, struct sockaddr *destaddr, socklen_t destaddrlen) { int n, rv; uint8_t buf[IPMIPOWER_PACKET_BUFLEN]; @@ -226,15 +226,15 @@ _sendto (cbuf_t cbuf, int fd, struct sockaddr_in6 *destaddr) exit (EXIT_FAILURE); } - do + do { if (cmd_args.common_args.driver_type == IPMI_DEVICE_LAN) rv = ipmi_lan_sendto (fd, buf, n, 0, - (struct sockaddr *)destaddr, - sizeof (struct sockaddr_in6)); + destaddr, + destaddrlen); else { if (ipmi_is_ipmi_1_5_packet (buf, n)) @@ -242,15 +242,15 @@ _sendto (cbuf_t cbuf, int fd, struct sockaddr_in6 *destaddr) buf, n, 0, - (struct sockaddr *)destaddr, - sizeof (struct sockaddr_in6)); + destaddr, + destaddrlen); else rv = ipmi_rmcpplus_sendto (fd, buf, n, 0, - (struct sockaddr *)destaddr, - sizeof (struct sockaddr_in6)); + destaddr, + destaddrlen); } } while (rv < 0 && errno == EINTR); @@ -269,13 +269,13 @@ _sendto (cbuf_t cbuf, int fd, struct sockaddr_in6 *destaddr) } static void -_recvfrom (cbuf_t cbuf, int fd, struct sockaddr_in6 *srcaddr) +_recvfrom (cbuf_t cbuf, int fd, struct sockaddr *srcaddr, socklen_t srcaddrlen) { int n, rv, dropped = 0; uint8_t buf[IPMIPOWER_PACKET_BUFLEN]; - struct sockaddr_in6 from; - struct sockaddr_in *from4; - unsigned int fromlen = sizeof (struct sockaddr_in6); + struct sockaddr_in6 from6; + struct sockaddr *from = (struct sockaddr *)&from6; + socklen_t fromlen = sizeof (struct sockaddr_in6); do { @@ -291,7 +291,7 @@ _recvfrom (cbuf_t cbuf, int fd, struct sockaddr_in6 *srcaddr) buf, IPMIPOWER_PACKET_BUFLEN, 0, - (struct sockaddr *)&from, + from, &fromlen); } while (rv < 0 && errno == EINTR); @@ -344,12 +344,25 @@ _recvfrom (cbuf_t cbuf, int fd, struct sockaddr_in6 *srcaddr) exit (EXIT_FAILURE); } - from4 = (struct sockaddr_in*)&from; - /* Don't store if this packet is strange for some reason */ - if (!(from4->sin_family == AF_INET - && from4->sin_addr.s_addr == ((struct sockaddr_in*)srcaddr)->sin_addr.s_addr) && - !(from.sin6_family == AF_INET6 && memcmp (&from.sin6_addr, &srcaddr->sin6_addr, sizeof(from.sin6_addr)) == 0)) - return; + if (from6.sin6_family == AF_INET6) + { + if (memcmp (&from6.sin6_addr, + &(((struct sockaddr_in6 *)srcaddr)->sin6_addr), + sizeof (from6.sin6_addr))) + return; + } + else + { + /* memcpy hacks to avoid warnings, i.e. + * warning: dereferencing pointer 'X' does break strict-aliasing rules + */ + struct sockaddr_in from4; + + memcpy (&from4, from, fromlen); + + if (from4.sin_addr.s_addr != ((struct sockaddr_in *)srcaddr)->sin_addr.s_addr) + return; + } /* cbuf should be empty, but if it isn't, empty it */ if (!cbuf_is_empty (cbuf)) @@ -496,15 +509,15 @@ _poll_loop (int non_interactive) { IPMIPOWER_DEBUG (("host = %s; IPMI POLLERR", ics[i].hostname)); /* See comments in _ipmi_recvfrom() regarding ECONNRESET/ECONNREFUSED */ - _recvfrom (ics[i].ipmi_in, ics[i].ipmi_fd, &(ics[i].destaddr)); + _recvfrom (ics[i].ipmi_in, ics[i].ipmi_fd, ics[i].destaddr, ics[i].destaddrlen); } else { if (pfds[i*2].revents & POLLIN) - _recvfrom (ics[i].ipmi_in, ics[i].ipmi_fd, &(ics[i].destaddr)); + _recvfrom (ics[i].ipmi_in, ics[i].ipmi_fd, ics[i].destaddr, ics[i].destaddrlen); if (pfds[i*2].revents & POLLOUT) - _sendto (ics[i].ipmi_out, ics[i].ipmi_fd, &(ics[i].destaddr)); + _sendto (ics[i].ipmi_out, ics[i].ipmi_fd, ics[i].destaddr, ics[i].destaddrlen); } if (!cmd_args.ping_interval) @@ -513,15 +526,15 @@ _poll_loop (int non_interactive) if (pfds[i*2+1].revents & POLLERR) { IPMIPOWER_DEBUG (("host = %s; PING_POLLERR", ics[i].hostname)); - _recvfrom (ics[i].ping_in, ics[i].ping_fd, &(ics[i].destaddr)); + _recvfrom (ics[i].ping_in, ics[i].ping_fd, ics[i].destaddr, ics[i].destaddrlen); } else { if (pfds[i*2+1].revents & POLLIN) - _recvfrom (ics[i].ping_in, ics[i].ping_fd, &(ics[i].destaddr)); + _recvfrom (ics[i].ping_in, ics[i].ping_fd, ics[i].destaddr, ics[i].destaddrlen); if (pfds[i*2+1].revents & POLLOUT) - _sendto (ics[i].ping_out, ics[i].ping_fd, &(ics[i].destaddr)); + _sendto (ics[i].ping_out, ics[i].ping_fd, ics[i].destaddr, ics[i].destaddrlen); } } diff --git a/ipmipower/ipmipower.h b/ipmipower/ipmipower.h index 0cc35d4a5..6127cc7fb 100644 --- a/ipmipower/ipmipower.h +++ b/ipmipower/ipmipower.h @@ -61,6 +61,8 @@ #define MAXHOSTNAMELEN 64 #endif /* MAXHOSTNAMELEN */ +#define MAXPORTBUFLEN 16 + #define IPMIPOWER_MIN_TTY_BUF 1024*4 #define IPMIPOWER_MAX_TTY_BUF 1024*32 @@ -467,7 +469,14 @@ struct ipmipower_connection char hostname[MAXHOSTNAMELEN+1]; /* for oem power types ; extra arg passed in via "+extra" at end of hostname */ struct ipmipower_connection_extra_arg *extra_args; - struct sockaddr_in6 destaddr; + struct sockaddr *srcaddr; + socklen_t srcaddrlen; + struct sockaddr_in srcaddr4; + struct sockaddr_in6 srcaddr6; + struct sockaddr *destaddr; + socklen_t destaddrlen; + struct sockaddr_in destaddr4; + struct sockaddr_in6 destaddr6; /* for eliminate option */ int skip; diff --git a/ipmipower/ipmipower_connection.c b/ipmipower/ipmipower_connection.c index b222993df..76702de3e 100644 --- a/ipmipower/ipmipower_connection.c +++ b/ipmipower/ipmipower_connection.c @@ -126,15 +126,13 @@ ipmipower_connection_clear (struct ipmipower_connection *ic) static int _connection_setup (struct ipmipower_connection *ic, const char *hostname) { - struct sockaddr_in6 srcaddr; - struct hostent *result; char *hostname_first_parse_copy = NULL; const char *hostname_first_parse_ptr = NULL; char *hostname_second_parse_copy = NULL; const char *hostname_second_parse_ptr = NULL; uint16_t port = RMCP_PRIMARY_RMCP_PORT; - char port_str[12]; - struct addrinfo ai_hints, *ai_res, *ai; + char port_str[MAXPORTBUFLEN + 1]; + struct addrinfo ai_hints, *ai_res = NULL, *ai = NULL; int rv = -1; assert (ic); @@ -332,22 +330,26 @@ _connection_setup (struct ipmipower_connection *ic, const char *hostname) strncpy (ic->hostname, hostname_second_parse_ptr, MAXHOSTNAMELEN); ic->hostname[MAXHOSTNAMELEN] = '\0'; - snprintf(port_str, sizeof (port_str), "%d", port); - memset(&ai_hints, 0, sizeof (struct addrinfo)); + memset (port_str, '\0', MAXPORTBUFLEN + 1); + snprintf (port_str, MAXPORTBUFLEN, "%d", port); + memset (&ai_hints, 0, sizeof (struct addrinfo)); ai_hints.ai_family = AF_UNSPEC; ai_hints.ai_socktype = SOCK_DGRAM; ai_hints.ai_flags = (AI_V4MAPPED | AI_ADDRCONFIG); - if ((result = getaddrinfo (ic->hostname, port_str, &ai_hints, &ai_res )) != 0) + + if ((ret = getaddrinfo (ic->hostname, port_str, &ai_hints, &ai_res))) { - if (result == EAI_NODATA) + if (ret == EAI_NODATA + || ret == EAI_NONAME) ipmipower_output (IPMIPOWER_MSG_TYPE_HOSTNAME_INVALID, ic->hostname, NULL); else { - IPMIPOWER_ERROR (("getaddrinfo() %s: %s", ic->hostname, gai_strerror (result))); + IPMIPOWER_ERROR (("getaddrinfo() %s: %s", ic->hostname, gai_strerror (ret))); exit (EXIT_FAILURE); } goto cleanup; } + /* Try all of the different answers we got, until we succeed. */ for (ai = ai_res; ai != NULL; ai = ai->ai_next) { @@ -370,39 +372,60 @@ _connection_setup (struct ipmipower_connection *ic, const char *hostname) return (-1); } } - /* Secure ephemeral ports */ - bzero (&srcaddr, sizeof (struct sockaddr_in6)); - srcaddr.sin6_family = ai->ai_family; /* always the same place */ - /* All zero is otherwise correct. */ - if ((bind (ic->ipmi_fd, &srcaddr, sizeof (struct sockaddr_in6)) < 0) - || (bind (ic->ping_fd, &srcaddr, sizeof (struct sockaddr_in6)) < 0)) - { + if (ai->ai_family == AF_INET) + { + memcpy (&(ic->destaddr4), ai->ai_addr, ai->ai_addrlen); + ic->destaddr = (struct sockaddr *)&(ic->destaddr4); + ic->destaddrlen = sizeof (struct sockaddr_in); + + /* zero everywhere, secure ephemeral port */ + memset (&(ic->srcaddr4), '\0', sizeof (struct sockaddr_in)); + ic->srcaddr4.sin_family = AF_INET; + + ic->srcaddr = (struct sockaddr *)&(ic->srcaddr4); + ic->srcaddrlen = sizeof (struct sockaddr_in); + } + else if (ai->ai_family == AF_INET6) + { + memcpy (&(ic->destaddr6), ai->ai_addr, ai->ai_addrlen); + ic->destaddr = (struct sockaddr *)&(ic->destaddr6); + ic->destaddrlen = sizeof (struct sockaddr_in6); + + /* zero everywhere, secure ephemeral port */ + memset (&(ic->srcaddr6), '\0', sizeof (struct sockaddr_in6)); + ic->srcaddr6.sin6_family = AF_INET6; + ic->srcaddr = (struct sockaddr *)&(ic->srcaddr6); + ic->srcaddrlen = sizeof (struct sockaddr_in6); + } + else + { close(ic->ipmi_fd); close(ic->ping_fd); continue; - } + } - /* Determine the destination address */ - if (ai->ai_addrlen > sizeof (struct sockaddr_in6)) + if ((bind (ic->ipmi_fd, ic->srcaddr, ic->srcaddrlen) < 0) + || (bind (ic->ping_fd, ic->srcaddr, ic->srcaddrlen) < 0)) { - IPMIPOWER_ERROR (("getaddrinfo: unexpected address length %d", - ai->ai_addrlen)); - exit (EXIT_FAILURE); + close(ic->ipmi_fd); + close(ic->ping_fd); + continue; } - memcpy (&(ic->destaddr), ai->ai_addr, ai->ai_addrlen); + ic->skip = 0; break; } if (!ai) { - IPMIPOWER_ERROR (("socket/bind: %s", strerror (errno))); - exit (EXIT_FAILURE); + ipmipower_output (IPMIPOWER_MSG_TYPE_HOSTNAME_INVALID, hostname_second_parse_ptr, NULL); + goto cleanup; } rv = 0; cleanup: + freeaddrinfo (ai_res); free (hostname_first_parse_copy); free (hostname_second_parse_copy); return (rv); diff --git a/ipmipower/ipmipower_powercmd.c b/ipmipower/ipmipower_powercmd.c index deeb948fe..3fc67f701 100644 --- a/ipmipower/ipmipower_powercmd.c +++ b/ipmipower/ipmipower_powercmd.c @@ -1411,42 +1411,36 @@ _retry_packets (ipmipower_powercmd_t ip) * past the Get Session Challenge phase of the protocol. */ int new_fd, *old_fd; - struct sockaddr_in srcaddr; - - if ((new_fd = socket (AF_INET, SOCK_DGRAM, 0)) < 0) + + if ((new_fd = socket (ip->ic->srcaddr->sa_family, SOCK_DGRAM, 0)) < 0) { if (errno != EMFILE) { IPMIPOWER_ERROR (("socket: %s", strerror (errno))); exit (EXIT_FAILURE); } - + ipmipower_output (IPMIPOWER_MSG_TYPE_RESOURCES, ip->ic->hostname, ip->extra_arg); return (-1); } - - bzero (&srcaddr, sizeof (struct sockaddr_in)); - srcaddr.sin_family = AF_INET; - srcaddr.sin_port = htons (0); - srcaddr.sin_addr.s_addr = htonl (INADDR_ANY); - - if (bind (new_fd, &srcaddr, sizeof (struct sockaddr_in)) < 0) + + if (bind (new_fd, ip->ic->srcaddr, ip->ic->srcaddrlen) < 0) { IPMIPOWER_ERROR (("bind: %s", strerror (errno))); exit (EXIT_FAILURE); } - + if (!(old_fd = (int *)malloc (sizeof (int)))) { IPMIPOWER_ERROR (("malloc: %s", strerror (errno))); exit (EXIT_FAILURE); } - + *old_fd = ip->ic->ipmi_fd; list_push (ip->sockets_to_close, old_fd); - + ip->ic->ipmi_fd = new_fd; - + _send_packet (ip, IPMIPOWER_PACKET_TYPE_GET_SESSION_CHALLENGE_RQ); } break;