From 0f01c1f9185ac28c53f52166f718ff04ce1fe75a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 20:25:46 +0100 Subject: [PATCH 1/6] dhcp-server: don't assign sendmsg() return value to "int" The type is "ssize_t", not "int", let's be accurate about that, as these types are different on some archs. Given that we don't actually care about the return value reall, drop the whole assignment, just check if negative. --- src/libsystemd-network/sd-dhcp-server.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index d64a47d2fce..c947174b1c8 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -313,7 +313,6 @@ static int dhcp_server_send_udp(sd_dhcp_server *server, be32_t destination, }; struct cmsghdr *cmsg; struct in_pktinfo *pktinfo; - int r; assert(server); assert(server->fd >= 0); @@ -337,8 +336,7 @@ static int dhcp_server_send_udp(sd_dhcp_server *server, be32_t destination, pktinfo->ipi_ifindex = server->ifindex; pktinfo->ipi_spec_dst.s_addr = server->address; - r = sendmsg(server->fd, &msg, 0); - if (r < 0) + if (sendmsg(server->fd, &msg, 0) < 0) return -errno; return 0; From 6e741541ed7109d736746dcefc11b5d119bc18fd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 20:28:01 +0100 Subject: [PATCH 2/6] dhcp-server: introduce log_dhcp_server_errno() Sometimes we want to print the error number, hence do so properly, and avoid to use strerror() which is not reentrant. --- src/libsystemd-network/dhcp-server-internal.h | 1 + src/libsystemd-network/sd-dhcp-server.c | 32 +++++++------------ 2 files changed, 12 insertions(+), 21 deletions(-) diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index 8b5620e1383..f2ffb39e440 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -86,6 +86,7 @@ typedef struct DHCPRequest { } DHCPRequest; #define log_dhcp_server(client, fmt, ...) log_internal(LOG_DEBUG, 0, __FILE__, __LINE__, __func__, "DHCP SERVER: " fmt, ##__VA_ARGS__) +#define log_dhcp_server_errno(client, error, fmt, ...) log_internal(LOG_DEBUG, error, __FILE__, __LINE__, __func__, "DHCP SERVER: " fmt, ##__VA_ARGS__) int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, size_t length); diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index c947174b1c8..ab86a86bb42 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -786,18 +786,12 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, return 0; r = server_send_offer(server, req, address); - if (r < 0) { + if (r < 0) /* this only fails on critical errors */ - log_dhcp_server(server, "could not send offer: %s", - strerror(-r)); - return r; - } else { - log_dhcp_server(server, "OFFER (0x%x)", - be32toh(req->message->xid)); - return DHCP_OFFER; - } + return log_dhcp_server_errno(server, r, "Could not send offer: %m"); - break; + log_dhcp_server(server, "OFFER (0x%x)", be32toh(req->message->xid)); + return DHCP_OFFER; } case DHCP_DECLINE: log_dhcp_server(server, "DECLINE (0x%x): %s", be32toh(req->message->xid), strna(error_message)); @@ -897,8 +891,7 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, r = server_send_ack(server, req, address); if (r < 0) { /* this only fails on critical errors */ - log_dhcp_server(server, "could not send ack: %s", - strerror(-r)); + log_dhcp_server_errno(server, r, "Could not send ack: %m"); if (!existing_lease) dhcp_lease_free(lease); @@ -914,18 +907,15 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, return DHCP_ACK; } + } else if (init_reboot) { r = server_send_nak(server, req); - if (r < 0) { + if (r < 0) /* this only fails on critical errors */ - log_dhcp_server(server, "could not send nak: %s", - strerror(-r)); - return r; - } else { - log_dhcp_server(server, "NAK (0x%x)", - be32toh(req->message->xid)); - return DHCP_NAK; - } + return log_dhcp_server_errno(server, r, "Could not send nak: %m"); + + log_dhcp_server(server, "NAK (0x%x)", be32toh(req->message->xid)); + return DHCP_NAK; } break; From 6408ba5fa9129d3e27a7c7f102fbe8d7afe0326a Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 20:29:07 +0100 Subject: [PATCH 3/6] dhcp-server: reduce level of indentation a bit Less indentation is good, let's do that where it's easy. --- src/libsystemd-network/sd-dhcp-server.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index ab86a86bb42..a1b51f1c575 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -776,8 +776,9 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, if (!server->bound_leases[next_offer]) { address = server->subnet | htobe32(server->pool_offset + next_offer); break; - } else - next_offer = (next_offer + 1) % server->pool_size; + } + + next_offer = (next_offer + 1) % server->pool_size; } } @@ -985,7 +986,8 @@ static int server_receive_message(sd_event_source *s, int fd, return 0; return -errno; - } else if ((size_t)len < sizeof(DHCPMessage)) + } + if ((size_t)len < sizeof(DHCPMessage)) return 0; CMSG_FOREACH(cmsg, &msg) { @@ -1069,8 +1071,8 @@ int sd_dhcp_server_forcerenew(sd_dhcp_server *server) { lease->chaddr); if (r < 0) return r; - else - log_dhcp_server(server, "FORCERENEW"); + + log_dhcp_server(server, "FORCERENEW"); } return r; From c3922c0c1c426b33c1c783b57f2eafba8ff06f19 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 20:29:43 +0100 Subject: [PATCH 4/6] dhcp_server_handle_message: don't pretend there was a difference between return code 0 or 1 We ignore the difference anyway, hence let's systematically return 0. --- src/libsystemd-network/sd-dhcp-server.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index a1b51f1c575..1a15d77db3a 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -942,12 +942,10 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message, server->bound_leases[pool_offset] = NULL; hashmap_remove(server->leases_by_client_id, existing_lease); dhcp_lease_free(existing_lease); + } - return 1; - } else - return 0; - } - } + return 0; + }} return 0; } From cfcbb135837a6e5fe79c9ebb10da8c30c489fb89 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 20:30:29 +0100 Subject: [PATCH 5/6] dhcp-sever: check properly for invalid fds We generally just compare for negativity, not for equlity to -1, let's do so here too. --- src/libsystemd-network/sd-dhcp-server.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 1a15d77db3a..7cc25ef82a9 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -1012,8 +1012,8 @@ int sd_dhcp_server_start(sd_dhcp_server *server) { assert_return(server, -EINVAL); assert_return(server->event, -EINVAL); assert_return(!server->receive_message, -EBUSY); - assert_return(server->fd_raw == -1, -EBUSY); - assert_return(server->fd == -1, -EBUSY); + assert_return(server->fd_raw < 0, -EBUSY); + assert_return(server->fd < 0, -EBUSY); assert_return(server->address != htobe32(INADDR_ANY), -EUNATCH); r = socket(AF_PACKET, SOCK_DGRAM | SOCK_NONBLOCK, 0); From 57027d035671f7b8ef98539c31712f27a2a6e87c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 21 Mar 2018 20:30:56 +0100 Subject: [PATCH 6/6] dhcp-server: don't propagate erros up the event loop If we can't send a message this is no reason to completely abort the event handler. Issue identified by Nandor Han , Sebastian Reichel . Replaces: #8525 --- src/libsystemd-network/sd-dhcp-server.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 7cc25ef82a9..25f5b378bc4 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -964,6 +964,7 @@ static int server_receive_message(sd_event_source *s, int fd, }; struct cmsghdr *cmsg; ssize_t buflen, len; + int r; assert(server); @@ -1003,7 +1004,11 @@ static int server_receive_message(sd_event_source *s, int fd, } } - return dhcp_server_handle_message(server, message, (size_t)len); + r = dhcp_server_handle_message(server, message, (size_t) len); + if (r < 0) + log_dhcp_server_errno(server, r, "Couldn't process incoming message: %m"); + + return 0; } int sd_dhcp_server_start(sd_dhcp_server *server) {