From fff145dba309df2a6ba2986ad2b690f7e0858cad Mon Sep 17 00:00:00 2001 From: Denis Vlasenko Date: Thu, 20 Dec 2007 21:11:38 +0000 Subject: [PATCH] udhcp: fix oversized packet sending (introduced by "slack for bad dhcp servers" options); slight optimizations and function renaming udhcp_send_raw_packet - 391 +391 udhcp_send_kernel_packet - 197 +197 udhcp_recv_packet - 134 +134 get_raw_packet 353 326 -27 udhcp_get_packet 134 - -134 udhcp_kernel_packet 197 - -197 udhcp_raw_packet 391 - -391 ------------------------------------------------------------------------------ (add/remove: 3/3 grow/shrink: 0/1 up/down: 722/-749) Total: -27 bytes --- networking/udhcp/clientpacket.c | 36 ++++++++++++++------------------- networking/udhcp/common.h | 10 +++++---- networking/udhcp/dhcpc.c | 2 +- networking/udhcp/dhcpd.c | 2 +- networking/udhcp/dhcprelay.c | 2 +- networking/udhcp/packet.c | 36 +++++++++++++++++++++------------ networking/udhcp/serverpacket.c | 4 ++-- 7 files changed, 49 insertions(+), 43 deletions(-) diff --git a/networking/udhcp/clientpacket.c b/networking/udhcp/clientpacket.c index 54f3f0e49..03473109f 100644 --- a/networking/udhcp/clientpacket.c +++ b/networking/udhcp/clientpacket.c @@ -86,7 +86,7 @@ int send_decline(uint32_t xid, uint32_t server) bb_info_msg("Sending decline..."); - return udhcp_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, + return udhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, SERVER_PORT, MAC_BCAST_ADDR, client_config.ifindex); } #endif @@ -106,7 +106,7 @@ int send_discover(uint32_t xid, uint32_t requested) add_simple_option(packet.options, DHCP_MAX_SIZE, htons(576)); add_requests(&packet); bb_info_msg("Sending discover..."); - return udhcp_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, + return udhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, SERVER_PORT, MAC_BCAST_ADDR, client_config.ifindex); } @@ -126,7 +126,7 @@ int send_selecting(uint32_t xid, uint32_t server, uint32_t requested) add_requests(&packet); addr.s_addr = requested; bb_info_msg("Sending select for %s...", inet_ntoa(addr)); - return udhcp_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, + return udhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, SERVER_PORT, MAC_BCAST_ADDR, client_config.ifindex); } @@ -143,9 +143,9 @@ int send_renew(uint32_t xid, uint32_t server, uint32_t ciaddr) add_requests(&packet); bb_info_msg("Sending renew..."); if (server) - return udhcp_kernel_packet(&packet, ciaddr, CLIENT_PORT, server, SERVER_PORT); + return udhcp_send_kernel_packet(&packet, ciaddr, CLIENT_PORT, server, SERVER_PORT); - return udhcp_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, + return udhcp_send_raw_packet(&packet, INADDR_ANY, CLIENT_PORT, INADDR_BROADCAST, SERVER_PORT, MAC_BCAST_ADDR, client_config.ifindex); } @@ -163,7 +163,7 @@ int send_release(uint32_t server, uint32_t ciaddr) add_simple_option(packet.options, DHCP_SERVER_ID, server); bb_info_msg("Sending release..."); - return udhcp_kernel_packet(&packet, ciaddr, CLIENT_PORT, server, SERVER_PORT); + return udhcp_send_kernel_packet(&packet, ciaddr, CLIENT_PORT, server, SERVER_PORT); } @@ -172,7 +172,6 @@ int get_raw_packet(struct dhcpMessage *payload, int fd) { int bytes; struct udp_dhcp_packet packet; - uint32_t source, dest; uint16_t check; memset(&packet, 0, sizeof(struct udp_dhcp_packet)); @@ -207,36 +206,31 @@ int get_raw_packet(struct dhcpMessage *payload, int fd) return -2; } - /* check IP checksum */ + /* verify IP checksum */ check = packet.ip.check; packet.ip.check = 0; - if (check != udhcp_checksum(&(packet.ip), sizeof(packet.ip))) { - DEBUG("bad IP header checksum, ignoring"); + if (check != udhcp_checksum(&packet.ip, sizeof(packet.ip))) { + DEBUG("Bad IP header checksum, ignoring"); return -1; } - /* verify the UDP checksum by replacing the header with a pseudo header */ - source = packet.ip.saddr; - dest = packet.ip.daddr; + /* verify UDP checksum. IP header has to be modified for this */ + memset(&packet.ip, 0, offsetof(struct iphdr, protocol)); + /* fields which are not memset: protocol, check, saddr, daddr */ + packet.ip.tot_len = packet.udp.len; /* yes, this is needed */ check = packet.udp.check; packet.udp.check = 0; - memset(&packet.ip, 0, sizeof(packet.ip)); - - packet.ip.protocol = IPPROTO_UDP; - packet.ip.saddr = source; - packet.ip.daddr = dest; - packet.ip.tot_len = packet.udp.len; /* cheat on the psuedo-header */ if (check && check != udhcp_checksum(&packet, bytes)) { bb_error_msg("packet with bad UDP checksum received, ignoring"); return -2; } - memcpy(payload, &(packet.data), bytes - (sizeof(packet.ip) + sizeof(packet.udp))); + memcpy(payload, &packet.data, bytes - (sizeof(packet.ip) + sizeof(packet.udp))); if (payload->cookie != htonl(DHCP_MAGIC)) { bb_error_msg("received bogus message (bad magic) - ignoring"); return -2; } - DEBUG("oooooh!!! got some!"); + DEBUG("Got valid DHCP packet"); return bytes - (sizeof(packet.ip) + sizeof(packet.udp)); } diff --git a/networking/udhcp/common.h b/networking/udhcp/common.h index 55782b51b..38ede0124 100644 --- a/networking/udhcp/common.h +++ b/networking/udhcp/common.h @@ -54,14 +54,16 @@ struct BUG_bad_sizeof_struct_udp_dhcp_packet { [(sizeof(struct udp_dhcp_packet) != 576 + CONFIG_UDHCPC_SLACK_FOR_BUGGY_SERVERS) ? -1 : 1]; }; -void udhcp_init_header(struct dhcpMessage *packet, char type); -int udhcp_get_packet(struct dhcpMessage *packet, int fd); uint16_t udhcp_checksum(void *addr, int count); -int udhcp_raw_packet(struct dhcpMessage *payload, + +void udhcp_init_header(struct dhcpMessage *packet, char type); + +int udhcp_recv_packet(struct dhcpMessage *packet, int fd); +int udhcp_send_raw_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port, uint32_t dest_ip, int dest_port, const uint8_t *dest_arp, int ifindex); -int udhcp_kernel_packet(struct dhcpMessage *payload, +int udhcp_send_kernel_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port, uint32_t dest_ip, int dest_port); diff --git a/networking/udhcp/dhcpc.c b/networking/udhcp/dhcpc.c index 69c35ca50..3de389f7b 100644 --- a/networking/udhcp/dhcpc.c +++ b/networking/udhcp/dhcpc.c @@ -451,7 +451,7 @@ int udhcpc_main(int argc, char **argv) /* A packet is ready, read it */ if (listen_mode == LISTEN_KERNEL) - len = udhcp_get_packet(&packet, sockfd); + len = udhcp_recv_packet(&packet, sockfd); else len = get_raw_packet(&packet, sockfd); if (len == -1 && errno != EINTR) { diff --git a/networking/udhcp/dhcpd.c b/networking/udhcp/dhcpd.c index 9679e086c..45f445b48 100644 --- a/networking/udhcp/dhcpd.c +++ b/networking/udhcp/dhcpd.c @@ -133,7 +133,7 @@ int udhcpd_main(int argc, char **argv) default: continue; /* signal or error (probably EINTR) */ } - bytes = udhcp_get_packet(&packet, server_socket); /* this waits for a packet - idle */ + bytes = udhcp_recv_packet(&packet, server_socket); /* this waits for a packet - idle */ if (bytes < 0) { if (bytes == -1 && errno != EINTR) { DEBUG("error on read, %s, reopening socket", strerror(errno)); diff --git a/networking/udhcp/dhcprelay.c b/networking/udhcp/dhcprelay.c index 42378d602..a6483fc1f 100644 --- a/networking/udhcp/dhcprelay.c +++ b/networking/udhcp/dhcprelay.c @@ -256,7 +256,7 @@ static void dhcprelay_loop(int *fds, int num_sockets, int max_socket, char **cli if (select(max_socket + 1, &rfds, NULL, NULL, &tv) > 0) { /* server */ if (FD_ISSET(fds[0], &rfds)) { - packlen = udhcp_get_packet(&dhcp_msg, fds[0]); + packlen = udhcp_recv_packet(&dhcp_msg, fds[0]); if (packlen > 0) { pass_back(&dhcp_msg, packlen, fds); } diff --git a/networking/udhcp/packet.c b/networking/udhcp/packet.c index 0abe851a4..c3890229f 100644 --- a/networking/udhcp/packet.c +++ b/networking/udhcp/packet.c @@ -39,7 +39,7 @@ void udhcp_init_header(struct dhcpMessage *packet, char type) /* read a packet from socket fd, return -1 on read error, -2 on packet error */ -int udhcp_get_packet(struct dhcpMessage *packet, int fd) +int udhcp_recv_packet(struct dhcpMessage *packet, int fd) { #if 0 static const char broken_vendors[][8] = { @@ -111,7 +111,7 @@ uint16_t udhcp_checksum(void *addr, int count) /* Make sure that the left-over byte is added correctly both * with little and big endian hosts */ uint16_t tmp = 0; - *(uint8_t *) (&tmp) = * (uint8_t *) source; + *(uint8_t*)&tmp = *(uint8_t*)source; sum += tmp; } /* Fold 32-bit sum to 16 bits */ @@ -123,7 +123,7 @@ uint16_t udhcp_checksum(void *addr, int count) /* Construct a ip/udp header for a packet, and specify the source and dest hardware address */ -int udhcp_raw_packet(struct dhcpMessage *payload, +int udhcp_send_raw_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port, uint32_t dest_ip, int dest_port, const uint8_t *dest_arp, int ifindex) { @@ -132,6 +132,11 @@ int udhcp_raw_packet(struct dhcpMessage *payload, struct sockaddr_ll dest; struct udp_dhcp_packet packet; + enum { + IP_UPD_DHCP_SIZE = sizeof(struct udp_dhcp_packet) - CONFIG_UDHCPC_SLACK_FOR_BUGGY_SERVERS, + UPD_DHCP_SIZE = IP_UPD_DHCP_SIZE - offsetof(struct udp_dhcp_packet, udp), + }; + fd = socket(PF_PACKET, SOCK_DGRAM, htons(ETH_P_IP)); if (fd < 0) { bb_perror_msg("socket"); @@ -140,6 +145,7 @@ int udhcp_raw_packet(struct dhcpMessage *payload, memset(&dest, 0, sizeof(dest)); memset(&packet, 0, sizeof(packet)); + packet.data = *payload; /* struct copy */ dest.sll_family = AF_PACKET; dest.sll_protocol = htons(ETH_P_IP); @@ -157,19 +163,19 @@ int udhcp_raw_packet(struct dhcpMessage *payload, packet.ip.daddr = dest_ip; packet.udp.source = htons(source_port); packet.udp.dest = htons(dest_port); - packet.udp.len = htons(sizeof(packet.udp) + sizeof(struct dhcpMessage)); /* cheat on the psuedo-header */ + /* size, excluding IP header: */ + packet.udp.len = htons(UPD_DHCP_SIZE); + /* for UDP checksumming, ip.len is set to UDP packet len */ packet.ip.tot_len = packet.udp.len; - memcpy(&(packet.data), payload, sizeof(struct dhcpMessage)); - packet.udp.check = udhcp_checksum(&packet, sizeof(struct udp_dhcp_packet)); - - packet.ip.tot_len = htons(sizeof(struct udp_dhcp_packet)); + packet.udp.check = udhcp_checksum(&packet, IP_UPD_DHCP_SIZE); + /* but for sending, it is set to IP packet len */ + packet.ip.tot_len = htons(IP_UPD_DHCP_SIZE); packet.ip.ihl = sizeof(packet.ip) >> 2; packet.ip.version = IPVERSION; packet.ip.ttl = IPDEFTTL; - packet.ip.check = udhcp_checksum(&(packet.ip), sizeof(packet.ip)); + packet.ip.check = udhcp_checksum(&packet.ip, sizeof(packet.ip)); - result = sendto(fd, &packet, sizeof(struct udp_dhcp_packet), 0, - (struct sockaddr *) &dest, sizeof(dest)); + result = sendto(fd, &packet, IP_UPD_DHCP_SIZE, 0, (struct sockaddr *) &dest, sizeof(dest)); if (result <= 0) { bb_perror_msg("sendto"); } @@ -179,13 +185,17 @@ int udhcp_raw_packet(struct dhcpMessage *payload, /* Let the kernel do all the work for packet generation */ -int udhcp_kernel_packet(struct dhcpMessage *payload, +int udhcp_send_kernel_packet(struct dhcpMessage *payload, uint32_t source_ip, int source_port, uint32_t dest_ip, int dest_port) { int fd, result; struct sockaddr_in client; + enum { + DHCP_SIZE = sizeof(struct dhcpMessage) - CONFIG_UDHCPC_SLACK_FOR_BUGGY_SERVERS, + }; + fd = socket(PF_INET, SOCK_DGRAM, IPPROTO_UDP); if (fd < 0) return -1; @@ -212,7 +222,7 @@ int udhcp_kernel_packet(struct dhcpMessage *payload, return -1; } - result = write(fd, payload, sizeof(struct dhcpMessage)); + result = write(fd, payload, DHCP_SIZE); close(fd); return result; } diff --git a/networking/udhcp/serverpacket.c b/networking/udhcp/serverpacket.c index ecbf50a14..764b9a8f6 100644 --- a/networking/udhcp/serverpacket.c +++ b/networking/udhcp/serverpacket.c @@ -30,7 +30,7 @@ static int send_packet_to_relay(struct dhcpMessage *payload) { DEBUG("Forwarding packet to relay"); - return udhcp_kernel_packet(payload, server_config.server, SERVER_PORT, + return udhcp_send_kernel_packet(payload, server_config.server, SERVER_PORT, payload->giaddr, SERVER_PORT); } @@ -58,7 +58,7 @@ static int send_packet_to_client(struct dhcpMessage *payload, int force_broadcas ciaddr = payload->yiaddr; chaddr = payload->chaddr; } - return udhcp_raw_packet(payload, server_config.server, SERVER_PORT, + return udhcp_send_raw_packet(payload, server_config.server, SERVER_PORT, ciaddr, CLIENT_PORT, chaddr, server_config.ifindex); } -- 2.25.1