From 65c3cd4fd9f64584be60f46c9609d300cd55d196 Mon Sep 17 00:00:00 2001 From: "Nicholas J. Kain" Date: Fri, 4 Apr 2014 04:01:49 -0400 Subject: [PATCH] Make many more logging prints specify the interface and function, and make the return checks for safe_(read|write) stricter. --- ndhc/dhcp.c | 95 +++++++++++++++++++++++++++++------------------- ndhc/ifchange.c | 15 +++++--- ndhc/ifchd.c | 23 +++++------- ndhc/ifchd.h | 1 - ndhc/leasefile.c | 5 ++- ndhc/ndhc.c | 1 + 6 files changed, 81 insertions(+), 59 deletions(-) diff --git a/ndhc/dhcp.c b/ndhc/dhcp.c index 074c8d9..a07181a 100644 --- a/ndhc/dhcp.c +++ b/ndhc/dhcp.c @@ -147,21 +147,24 @@ static int send_dhcp_cooked(struct client_state_t *cs, struct dhcpmsg *payload) .sin_addr.s_addr = cs->serverAddr, }; if (connect(fd, (struct sockaddr *)&raddr, sizeof(struct sockaddr)) == -1) { - log_error("send_dhcp_cooked: connect failed: %s", strerror(errno)); + log_error("%s: (%s) connect failed: %s", client_config.interface, + __func__, strerror(errno)); goto out_fd; } // Send packets that are as short as possible. ssize_t endloc = get_end_option_idx(payload); if (endloc < 0) { - log_error("send_dhcp_cooked: No end marker. Not sending."); + log_error("%s: (%s) No end marker. Not sending.", + client_config.interface, __func__); goto out_fd; } size_t payload_len = sizeof *payload - (sizeof payload->options - 1 - endloc); ret = safe_write(fd, (const char *)payload, payload_len); - if (ret == -1) - log_error("send_dhcp_cooked: write failed: %s", strerror(errno)); + if (ret < 0 || (size_t)ret != payload_len) + log_error("%s: (%s) write failed: %d", client_config.interface, + __func__, ret); out_fd: close(fd); out: @@ -177,7 +180,8 @@ static int get_cooked_packet(struct dhcpmsg *packet, int fd) if (bytes == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) return -2; - log_line("Read on listen socket failed: %s", strerror(errno)); + log_line("%s: Read on listen socket failed: %s", + client_config.interface, strerror(errno)); return -1; } return bytes; @@ -252,24 +256,28 @@ static int udp_checksum(struct ip_udp_dhcp_packet *packet) static int get_raw_packet_validate_bpf(struct ip_udp_dhcp_packet *packet) { if (packet->ip.version != IPVERSION) { - log_line("IP version is not IPv4."); + log_warning("%s: IP version is not IPv4.", client_config.interface); return 0; } if (packet->ip.ihl != sizeof packet->ip >> 2) { - log_line("IP header length incorrect."); + log_warning("%s: IP header length incorrect.", + client_config.interface); return 0; } if (packet->ip.protocol != IPPROTO_UDP) { - log_line("IP header is not UDP: %d", packet->ip.protocol); + log_warning("%s: IP header is not UDP: %d", + client_config.interface, packet->ip.protocol); return 0; } if (ntohs(packet->udp.dest) != DHCP_CLIENT_PORT) { - log_line("UDP destination port incorrect: %d", ntohs(packet->udp.dest)); + log_warning("%s: UDP destination port incorrect: %d", + client_config.interface, ntohs(packet->udp.dest)); return 0; } if (ntohs(packet->udp.len) != ntohs(packet->ip.tot_len) - sizeof packet->ip) { - log_line("UDP header length incorrect."); + log_warning("%s: UDP header length incorrect.", + client_config.interface); return 0; } return 1; @@ -286,12 +294,14 @@ static int get_raw_packet(struct client_state_t *cs, struct dhcpmsg *payload) if (inc == -1) { if (errno == EAGAIN || errno == EWOULDBLOCK) return -2; - log_line("get_raw_packet: read error %s", strerror(errno)); + log_warning("%s: (%s) read error %s", client_config.interface, + __func__, strerror(errno)); return -1; } if (inc != ntohs(packet.ip.tot_len)) { - log_line("UDP length does not match header length fields."); + log_warning("%s: UDP length does not match header length fields.", + client_config.interface); return -2; } @@ -299,11 +309,13 @@ static int get_raw_packet(struct client_state_t *cs, struct dhcpmsg *payload) return -2; if (!ip_checksum(&packet)) { - log_line("IP header checksum incorrect."); + log_warning("%s: IP header checksum incorrect.", + client_config.interface); return -2; } if (packet.udp.check && !udp_checksum(&packet)) { - log_error("Packet with bad UDP checksum received. Ignoring."); + log_error("%s: Packet with bad UDP checksum received. Ignoring.", + client_config.interface); return -2; } @@ -422,7 +434,8 @@ static int send_dhcp_raw(struct dhcpmsg *payload) // Send packets that are as short as possible. ssize_t endloc = get_end_option_idx(payload); if (endloc < 0) { - log_error("send_dhcp_raw: No end marker. Not sending."); + log_error("%s: (%s) No end marker. Not sending.", + client_config.interface, __func__); close(fd); return ret; } @@ -462,7 +475,8 @@ static int send_dhcp_raw(struct dhcpmsg *payload) ret = safe_sendto(fd, (const char *)&iudmsg, iud_len, 0, (struct sockaddr *)&da, sizeof da); if (ret == -1) - log_error("send_dhcp_raw: sendto failed: %s", strerror(errno)); + log_error("%s: (%s) sendto failed: %s", client_config.interface, + __func__, strerror(errno)); close(fd); return ret; } @@ -482,7 +496,8 @@ static void change_listen_mode(struct client_state_t *cs, int new_mode) create_raw_listen_socket(cs, client_config.ifindex) : create_udp_listen_socket(client_config.interface); if (cs->listenFd < 0) - suicide("FATAL: Couldn't listen on socket: %s.", strerror(errno)); + suicide("%s: FATAL: Couldn't listen on socket: %s", + client_config.interface, strerror(errno)); epoll_add(cs->epollFd, cs->listenFd); } @@ -505,30 +520,34 @@ static int validate_dhcp_packet(struct client_state_t *cs, size_t len, struct dhcpmsg *packet, uint8_t *msgtype) { if (len < offsetof(struct dhcpmsg, options)) { - log_line("Packet is too short to contain magic cookie. Ignoring."); + log_warning("%s: Packet is too short to contain magic cookie. Ignoring.", + client_config.interface); return 0; } if (ntohl(packet->cookie) != DHCP_MAGIC) { - log_line("Packet with bad magic number. Ignoring."); + log_warning("%s: Packet with bad magic number. Ignoring.", + client_config.interface); return 0; } if (packet->xid != cs->xid) { - log_line("Packet XID %lx does not equal our XID %lx. Ignoring.", - packet->xid, cs->xid); + log_warning("%s: Packet XID %lx does not equal our XID %lx. Ignoring.", + client_config.interface, packet->xid, cs->xid); return 0; } if (memcmp(packet->chaddr, client_config.arp, sizeof client_config.arp)) { - log_line("Packet client MAC %2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x does not equal our MAC %2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x. Ignoring it.", - packet->chaddr[0], packet->chaddr[1], packet->chaddr[2], - packet->chaddr[3], packet->chaddr[4], packet->chaddr[5], - client_config.arp[0], client_config.arp[1], - client_config.arp[2], client_config.arp[3], - client_config.arp[4], client_config.arp[5]); + log_warning("%s: Packet client MAC %2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x does not equal our MAC %2.2x:%2.2x:%2.2x:%2.2x:%2.2x:%2.2x. Ignoring it.", + client_config.interface, + packet->chaddr[0], packet->chaddr[1], packet->chaddr[2], + packet->chaddr[3], packet->chaddr[4], packet->chaddr[5], + client_config.arp[0], client_config.arp[1], + client_config.arp[2], client_config.arp[3], + client_config.arp[4], client_config.arp[5]); return 0; } *msgtype = get_option_msgtype(packet); if (!*msgtype) { - log_line("Packet does not specify a DHCP message type. Ignoring."); + log_warning("%s: Packet does not specify a DHCP message type. Ignoring.", + client_config.interface); return 0; } char clientid[MAX_DOPT_SIZE]; @@ -537,7 +556,8 @@ static int validate_dhcp_packet(struct client_state_t *cs, size_t len, return 1; if (memcmp(client_config.clientid, clientid, min_size_t(cidlen, client_config.clientid_len))) { - log_line("Packet clientid does not match our clientid. Ignoring."); + log_warning("%s: Packet clientid does not match our clientid. Ignoring.", + client_config.interface); return 0; } return 1; @@ -556,8 +576,8 @@ void handle_packet(struct client_state_t *cs) // Transient issue handled by packet collection functions. if (r == -2 || (r == -1 && errno == EINTR)) return; - log_error("Error reading from listening socket: %s. Reopening.", - strerror(errno)); + log_error("%s: Error reading from listening socket: %s. Reopening.", + client_config.interface, strerror(errno)); change_listen_mode(cs, cs->listenMode); return; } @@ -595,7 +615,7 @@ int send_discover(struct client_state_t *cs) add_option_request_list(&packet); add_option_vendor(&packet); add_option_hostname(&packet); - log_line("Discovering DHCP servers..."); + log_line("%s: Discovering DHCP servers...", client_config.interface); return send_dhcp_raw(&packet); } @@ -611,7 +631,8 @@ int send_selecting(struct client_state_t *cs) add_option_hostname(&packet); inet_ntop(AF_INET, &(struct in_addr){.s_addr = cs->clientAddr}, clibuf, sizeof clibuf); - log_line("Sending a selection request for %s...", clibuf); + log_line("%s: Sending a selection request for %s...", + client_config.interface, clibuf); return send_dhcp_raw(&packet); } @@ -623,7 +644,7 @@ int send_renew(struct client_state_t *cs) add_option_request_list(&packet); add_option_vendor(&packet); add_option_hostname(&packet); - log_line("Sending a renew request..."); + log_line("%s: Sending a renew request...", client_config.interface); return send_dhcp_cooked(cs, &packet); } @@ -636,7 +657,7 @@ int send_rebind(struct client_state_t *cs) add_option_request_list(&packet); add_option_vendor(&packet); add_option_hostname(&packet); - log_line("Sending a rebind request..."); + log_line("%s: Sending a rebind request...", client_config.interface); return send_dhcp_raw(&packet); } @@ -645,7 +666,7 @@ int send_decline(struct client_state_t *cs, uint32_t server) struct dhcpmsg packet = init_packet(DHCPDECLINE, cs->xid); add_option_reqip(&packet, cs->clientAddr); add_option_serverid(&packet, server); - log_line("Sending a decline message..."); + log_line("%s: Sending a decline message...", client_config.interface); return send_dhcp_raw(&packet); } @@ -656,7 +677,7 @@ int send_release(struct client_state_t *cs) packet.ciaddr = cs->clientAddr; add_option_reqip(&packet, cs->clientAddr); add_option_serverid(&packet, cs->serverAddr); - log_line("Sending a release message..."); + log_line("%s: Sending a release message...", client_config.interface); return send_dhcp_cooked(cs, &packet); } diff --git a/ndhc/ifchange.c b/ndhc/ifchange.c index 6a6b4a0..ea6a8d9 100644 --- a/ndhc/ifchange.c +++ b/ndhc/ifchange.c @@ -179,18 +179,20 @@ static int ifchd_cmd(char *b, size_t bl, uint8_t *od, ssize_t ol, uint8_t code) case DCODE_IPTTL: return ifcmd_u8(b, bl, "ipttl", od, ol); default: break; } - log_line("Invalid option code (%c) for ifchd cmd.", code); + log_warning("%s: Invalid option code (%c) for ifchd cmd.", + client_config.interface, code); return -1; } static void pipewrite(struct client_state_t *cs, const char *buf, size_t count) { cs->ifchWorking = 1; - if (safe_write(pToIfchW, buf, count) == -1) { - log_error("pipewrite: write failed: %s", strerror(errno)); + int r = safe_write(pToIfchW, buf, count); + if (r < 0 || (size_t)r != count) { + log_error("%s: (%s) write failed: %d", client_config.interface); return; } - log_line("Sent to ifchd: %s", buf); + log_line("%s: Sent to ifchd: '%s'", client_config.interface, buf); } void ifchange_deconfig(struct client_state_t *cs) @@ -202,7 +204,7 @@ void ifchange_deconfig(struct client_state_t *cs) cs->ifDeconfig = 1; snprintf(buf, sizeof buf, "ip4:0.0.0.0,255.255.255.255;"); - log_line("Resetting %s IP configuration.", client_config.interface); + log_line("%s: Resetting IP configuration.", client_config.interface); pipewrite(cs, buf, strlen(buf)); memset(&cfg_packet, 0, sizeof cfg_packet); @@ -249,7 +251,8 @@ static size_t send_client_ip(char *out, size_t olen, struct dhcpmsg *packet) if (!have_subnet) { static char snClassC[] = "255.255.255.0"; - log_line("Server did not send a subnet mask. Assuming class C (255.255.255.0)."); + log_line("%s: Server did not send a subnet mask. Assuming 255.255.255.0.", + client_config.interface); memcpy(sn, snClassC, sizeof snClassC); } diff --git a/ndhc/ifchd.c b/ndhc/ifchd.c index 4154437..2748892 100644 --- a/ndhc/ifchd.c +++ b/ndhc/ifchd.c @@ -71,10 +71,12 @@ char pidfile_ifch[PATH_MAX] = PID_FILE_IFCH_DEFAULT; uid_t ifch_uid = 0; gid_t ifch_gid = 0; -static void writeordie(int fd, const char *buf, int len) +static void writeordie(int fd, const char *buf, size_t len) { - if (safe_write(fd, buf, len) == -1) - suicide("write returned error"); + ssize_t r = safe_write(fd, buf, len) == -1; + if (r < 0 || (size_t)r != len) + suicide("%s: (%s) write failed: %d", client_config.interface, + __func__, r); } /* Writes a new resolv.conf based on the information we have received. */ @@ -307,18 +309,13 @@ static void signal_dispatch(void) static void inform_execute(char c) { - int r; - retry: - r = safe_write(pToNdhcW, &c, sizeof c); + ssize_t r = safe_write(pToNdhcW, &c, sizeof c); if (r == 0) { // Remote end hung up. exit(EXIT_SUCCESS); - } else if (r < 0) { - if (errno == EAGAIN || errno == EWOULDBLOCK) - goto retry; + } else if (r < 0) suicide("%s: (%s) error writing to ifch -> ndhc pipe: %s", client_config.interface, __func__, strerror(errno)); - } } static void process_client_pipe(void) @@ -326,7 +323,7 @@ static void process_client_pipe(void) char buf[MAX_BUF]; memset(buf, '\0', sizeof buf); - int r = safe_read(pToIfchR, buf, sizeof buf - 1); + ssize_t r = safe_read(pToIfchR, buf, sizeof buf - 1); if (r == 0) { // Remote end hung up. exit(EXIT_SUCCESS); @@ -339,13 +336,13 @@ static void process_client_pipe(void) if (execute_buffer(buf) == -1) { inform_execute('-'); - suicide("%s: (%s) execute_buffer was passed invalid commands: '%s'", + suicide("%s: (%s) received invalid commands: '%s'", client_config.interface, __func__, buf); } else inform_execute('+'); } -void do_ifch_work(void) +static void do_ifch_work(void) { epollfd = epoll_create1(0); if (epollfd == -1) diff --git a/ndhc/ifchd.h b/ndhc/ifchd.h index 7330360..6aeb743 100644 --- a/ndhc/ifchd.h +++ b/ndhc/ifchd.h @@ -46,7 +46,6 @@ void perform_ipttl(const char *str, size_t len); void perform_ntpsrv(const char *str, size_t len); void perform_wins(const char *str, size_t len); -void do_ifch_work(void); void ifch_main(void); #endif /* NJK_IFCHD_H_ */ diff --git a/ndhc/leasefile.c b/ndhc/leasefile.c index 565ef53..45c44ad 100644 --- a/ndhc/leasefile.c +++ b/ndhc/leasefile.c @@ -95,8 +95,9 @@ void write_leasefile(struct in_addr ipnum) return; } lseek(leasefilefd, 0, SEEK_SET); - ret = safe_write(leasefilefd, out, strlen(out)); - if (ret == -1) + size_t outlen = strlen(out); + ret = safe_write(leasefilefd, out, outlen); + if (ret < 0 || (size_t)ret != outlen) log_warning("%s: Failed to write ip to lease file.", client_config.interface); else diff --git a/ndhc/ndhc.c b/ndhc/ndhc.c index 8421ece..8eaf74d 100644 --- a/ndhc/ndhc.c +++ b/ndhc/ndhc.c @@ -335,6 +335,7 @@ char resolv_conf_d[PATH_MAX] = ""; static char pidfile[PATH_MAX] = PID_FILE_DEFAULT; static uid_t ndhc_uid = 0; static gid_t ndhc_gid = 0; + int pToNdhcR; int pToNdhcW; int pToIfchR;