From: Yu Watanabe Date: Mon, 4 May 2026 10:57:49 +0000 (+0900) Subject: sd-dhcp-server: refactoring for socket fd handling X-Git-Tag: v261-rc1~22^2~5 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0bb26684d042d92eaadd1ceb713c76ad6f8ba75b;p=thirdparty%2Fsystemd.git sd-dhcp-server: refactoring for socket fd handling This makes - UDP socket fd is owned by IO event source, - open RAW socket fd just before sending first packet, - set TOS and socket priority, - use AF_UNIX soxket pair in the unit test and fuzzer, so the unit test can now run by unprivileged user. --- diff --git a/src/libsystemd-network/dhcp-server-internal.h b/src/libsystemd-network/dhcp-server-internal.h index c03ce7dddbb..76db80b77d5 100644 --- a/src/libsystemd-network/dhcp-server-internal.h +++ b/src/libsystemd-network/dhcp-server-internal.h @@ -29,9 +29,10 @@ typedef struct sd_dhcp_server { sd_event *event; int event_priority; - sd_event_source *receive_message; - int fd; - int fd_raw; + sd_event_source *io_event_source; + uint8_t ip_service_type; + int socket_fd; /* socket fd set externally, used by unit tests */ + int raw_socket_fd; /* send-only raw socket fd, used on sending L2 unicast message. */ int ifindex; char *ifname; diff --git a/src/libsystemd-network/dhcp-server-request.c b/src/libsystemd-network/dhcp-server-request.c index 78d641141c3..39c11e69c86 100644 --- a/src/libsystemd-network/dhcp-server-request.c +++ b/src/libsystemd-network/dhcp-server-request.c @@ -3,13 +3,13 @@ #include "sd-event.h" #include "alloc-util.h" -#include "dhcp-network.h" #include "dhcp-protocol.h" #include "dhcp-server-internal.h" #include "dhcp-server-lease-internal.h" #include "dhcp-server-request.h" #include "dhcp-server-send.h" #include "errno-util.h" +#include "fd-util.h" #include "hashmap.h" #include "iovec-util.h" #include "memory-util.h" @@ -495,38 +495,92 @@ static int server_receive_message(sd_event_source *s, int fd, uint32_t revents, return 0; } +static int server_open_socket(sd_dhcp_server *server) { + int r; + + assert(server); + + _cleanup_close_ int fd = RET_NERRNO(socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0)); + if (fd < 0) + return fd; + + r = socket_bind_to_ifindex(fd, server->ifindex); + if (r < 0) + return r; + + r = setsockopt_int(fd, SOL_SOCKET, SO_TIMESTAMP, true); + if (r < 0) + return r; + + r = setsockopt_int(fd, SOL_SOCKET, SO_REUSEADDR, true); + if (r < 0) + return r; + + r = setsockopt_int(fd, SOL_SOCKET, SO_BROADCAST, true); + if (r < 0) + return r; + + r = setsockopt_int(fd, SOL_SOCKET, SO_PRIORITY, tos_to_priority(server->ip_service_type)); + if (r < 0) + return r; + + r = setsockopt_int(fd, IPPROTO_IP, IP_TOS, server->ip_service_type); + if (r < 0) + return r; + + r = setsockopt_int(fd, IPPROTO_IP, IP_PKTINFO, true); + if (r < 0) + return r; + + union sockaddr_union sa = { + .in.sin_family = AF_INET, + .in.sin_port = htobe16(DHCP_PORT_SERVER), + .in.sin_addr.s_addr = htobe32(INADDR_ANY), + }; + + if (bind(fd, &sa.sa, sizeof(sa.in)) < 0) + return -errno; + + return TAKE_FD(fd); +} + int dhcp_server_setup_io_event_source(sd_dhcp_server *server) { int r; assert(server); assert(server->event); - r = socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0); - if (r < 0) { - r = -errno; - goto on_error; + _cleanup_close_ int fd_close = -EBADF; + int fd; + if (server->socket_fd >= 0) + /* When a socket fd is given externally, unconditionally use it and do not close the socket + * even if we fail to set up the event source. */ + fd = server->socket_fd; + else { + fd = fd_close = server_open_socket(server); + if (fd < 0) + return fd; } - server->fd_raw = r; - r = dhcp_network_bind_udp_socket(server->ifindex, INADDR_ANY, DHCP_PORT_SERVER, -1); + _cleanup_(sd_event_source_unrefp) sd_event_source *s = NULL; + r = sd_event_add_io(server->event, &s, fd, EPOLLIN, server_receive_message, server); if (r < 0) - goto on_error; - server->fd = r; + return r; - r = sd_event_add_io(server->event, &server->receive_message, - server->fd, EPOLLIN, - server_receive_message, server); + r = sd_event_source_set_priority(s, server->event_priority); if (r < 0) - goto on_error; + return r; - r = sd_event_source_set_priority(server->receive_message, - server->event_priority); - if (r < 0) - goto on_error; + (void) sd_event_source_set_description(s, "dhcp-server-io"); - return 0; + if (fd_close >= 0) { + r = sd_event_source_set_io_fd_own(s, true); + if (r < 0) + return r; + TAKE_FD(fd_close); + } - on_error: - sd_dhcp_server_stop(server); - return r; + sd_event_source_disable_unref(server->io_event_source); + server->io_event_source = TAKE_PTR(s); + return 0; } diff --git a/src/libsystemd-network/dhcp-server-send.c b/src/libsystemd-network/dhcp-server-send.c index 67fc390bca3..11d553404c5 100644 --- a/src/libsystemd-network/dhcp-server-send.c +++ b/src/libsystemd-network/dhcp-server-send.c @@ -1,19 +1,47 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +#include "sd-event.h" + #include "alloc-util.h" -#include "dhcp-network.h" #include "dhcp-option.h" #include "dhcp-packet.h" #include "dhcp-server-lease-internal.h" #include "dhcp-server-send.h" #include "dns-domain.h" #include "errno-util.h" +#include "fd-util.h" #include "hashmap.h" #include "in-addr-util.h" #include "iovec-util.h" #include "iovec-wrapper.h" #include "socket-util.h" +static int server_acquire_raw_socket(sd_dhcp_server *server) { + int r; + + assert(server); + + if (server->socket_fd >= 0) + /* When a socket fd is given externally, unconditionally use it and do not close the socket. */ + return server->socket_fd; + + if (server->raw_socket_fd >= 0) + /* Already opened. */ + return server->raw_socket_fd; + + /* This is a send-only socket, hence it is opened with protocol=0, and do not call bind(). + * The interface binding will be done on send. */ + _cleanup_close_ int fd = RET_NERRNO(socket(AF_PACKET, SOCK_DGRAM | SOCK_CLOEXEC | SOCK_NONBLOCK, 0)); + if (fd < 0) + return fd; + + r = setsockopt_int(fd, SOL_SOCKET, SO_PRIORITY, tos_to_priority(server->ip_service_type)); + if (r < 0) + return r; + + return server->raw_socket_fd = TAKE_FD(fd); +} + static int dhcp_server_send_unicast_raw( sd_dhcp_server *server, uint8_t hlen, @@ -21,12 +49,6 @@ static int dhcp_server_send_unicast_raw( DHCPPacket *packet, size_t len) { - union sockaddr_union link = { - .ll.sll_family = AF_PACKET, - .ll.sll_protocol = htobe16(ETH_P_IP), - .ll.sll_ifindex = server->ifindex, - .ll.sll_halen = hlen, - }; int r; assert(server); @@ -37,11 +59,13 @@ static int dhcp_server_send_unicast_raw( assert(packet); assert(len > sizeof(DHCPPacket)); - memcpy(link.ll.sll_addr, chaddr, hlen); - if (len > UINT16_MAX) return -EOVERFLOW; + int fd = server_acquire_raw_socket(server); + if (fd < 0) + return fd; + r = dhcp_packet_append_ip_headers( packet, server->address, @@ -49,63 +73,69 @@ static int dhcp_server_send_unicast_raw( packet->dhcp.yiaddr, DHCP_PORT_CLIENT, len, - /* ip_service_type= */ -1); + server->ip_service_type); if (r < 0) return r; - return dhcp_network_send_raw_socket( - server->fd_raw, - &link, - &(struct iovec_wrapper) { - .iovec = &IOVEC_MAKE(packet, len), - .count = 1, - }); + union sockaddr_union sa = { + .ll.sll_family = AF_PACKET, + .ll.sll_protocol = htobe16(ETH_P_IP), + .ll.sll_ifindex = server->ifindex, + .ll.sll_halen = hlen, + }; + + memcpy(sa.ll.sll_addr, chaddr, hlen); + + struct msghdr mh = { + .msg_name = &sa.sa, + .msg_namelen = sockaddr_ll_len(&sa.ll), + .msg_iov = &IOVEC_MAKE(packet, len), + .msg_iovlen = 1, + }; + + if (sendmsg(fd, &mh, MSG_NOSIGNAL) < 0) + return -errno; + + return 0; } static int dhcp_server_send_udp(sd_dhcp_server *server, be32_t destination, uint16_t destination_port, DHCPMessage *message, size_t len) { - union sockaddr_union dest = { + + assert(server); + assert(message); + assert(len >= sizeof(DHCPMessage)); + + int fd = sd_event_source_get_io_fd(server->io_event_source); + if (fd < 0) + return fd; + + union sockaddr_union sa = { .in.sin_family = AF_INET, .in.sin_port = htobe16(destination_port), .in.sin_addr.s_addr = destination, }; - struct iovec iov = { - .iov_base = message, - .iov_len = len, - }; CMSG_BUFFER_TYPE(CMSG_SPACE(sizeof(struct in_pktinfo))) control = {}; struct msghdr msg = { - .msg_name = &dest, - .msg_namelen = sizeof(dest.in), - .msg_iov = &iov, + .msg_name = &sa, + .msg_namelen = sizeof(sa.in), + .msg_iov = &IOVEC_MAKE(message, len), .msg_iovlen = 1, + .msg_control = &control, + .msg_controllen = sizeof(control), }; - struct cmsghdr *cmsg; - struct in_pktinfo *pktinfo; - - assert(server); - assert(server->fd >= 0); - assert(message); - assert(len >= sizeof(DHCPMessage)); - - msg.msg_control = &control; - msg.msg_controllen = sizeof(control); - - cmsg = CMSG_FIRSTHDR(&msg); - assert(cmsg); + struct cmsghdr *cmsg = ASSERT_PTR(CMSG_FIRSTHDR(&msg)); cmsg->cmsg_level = IPPROTO_IP; cmsg->cmsg_type = IP_PKTINFO; cmsg->cmsg_len = CMSG_LEN(sizeof(struct in_pktinfo)); - pktinfo = CMSG_TYPED_DATA(cmsg, struct in_pktinfo); - assert(pktinfo); - + struct in_pktinfo *pktinfo = ASSERT_PTR(CMSG_TYPED_DATA(cmsg, struct in_pktinfo)); pktinfo->ipi_ifindex = server->ifindex; pktinfo->ipi_spec_dst.s_addr = server->address; - if (sendmsg(server->fd, &msg, 0) < 0) + if (sendmsg(fd, &msg, MSG_NOSIGNAL) < 0) return -errno; return 0; diff --git a/src/libsystemd-network/fuzz-dhcp-server.c b/src/libsystemd-network/fuzz-dhcp-server.c index 9d927472e00..ac006bc2aa5 100644 --- a/src/libsystemd-network/fuzz-dhcp-server.c +++ b/src/libsystemd-network/fuzz-dhcp-server.c @@ -68,13 +68,16 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { _cleanup_(rm_rf_physical_and_freep) char *tmpdir = NULL; _cleanup_close_ int dir_fd = ASSERT_OK(mkdtemp_open(NULL, 0, &tmpdir)); + _cleanup_close_pair_ int socket_fd[2] = EBADF_PAIR; + ASSERT_OK_ERRNO(socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, socket_fd)); + _cleanup_(sd_event_unrefp) sd_event *event = NULL; ASSERT_OK(sd_event_new(&event)); _cleanup_(sd_dhcp_server_unrefp) sd_dhcp_server *server = NULL; ASSERT_OK(sd_dhcp_server_new(&server, 1)); ASSERT_OK(sd_dhcp_server_attach_event(server, event, SD_EVENT_PRIORITY_NORMAL)); - server->fd = ASSERT_OK_ERRNO(open("/dev/null", O_RDWR|O_CLOEXEC|O_NOCTTY)); + server->socket_fd = TAKE_FD(socket_fd[0]); ASSERT_OK(sd_dhcp_server_set_lease_file(server, dir_fd, "leases")); ASSERT_OK(sd_dhcp_server_configure_pool(server, &address, 24, 0, 0)); @@ -87,6 +90,7 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { ASSERT_OK(add_static_lease(server, 4)); _cleanup_free_ uint8_t *duped = ASSERT_NOT_NULL(memdup(data, size)); + ASSERT_OK(sd_dhcp_server_start(server)); (void) dhcp_server_handle_message(server, (DHCPMessage*) duped, size, NULL); ASSERT_OK(dhcp_server_save_leases(server)); diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 5267137540d..772bba48382 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -3,6 +3,8 @@ Copyright © 2013 Intel Corporation. All rights reserved. ***/ +#include + #include "sd-dhcp-server.h" #include "sd-event.h" @@ -99,10 +101,7 @@ int sd_dhcp_server_configure_pool( } int sd_dhcp_server_is_running(sd_dhcp_server *server) { - if (!server) - return false; - - return !!server->receive_message; + return server && sd_event_source_get_enabled(server->io_event_source, /* ret= */ NULL) > 0; } static sd_dhcp_server* dhcp_server_free(sd_dhcp_server *server) { @@ -111,6 +110,8 @@ static sd_dhcp_server* dhcp_server_free(sd_dhcp_server *server) { sd_dhcp_server_stop(server); sd_event_unref(server->event); + safe_close(server->socket_fd); + safe_close(server->raw_socket_fd); free(server->boot_server_name); free(server->boot_filename); @@ -149,8 +150,9 @@ int sd_dhcp_server_new(sd_dhcp_server **ret, int ifindex) { *server = (sd_dhcp_server) { .n_ref = 1, - .fd_raw = -EBADF, - .fd = -EBADF, + .ip_service_type = IPTOS_CLASS_CS6, + .socket_fd = -EBADF, + .raw_socket_fd = -EBADF, .address = htobe32(INADDR_ANY), .netmask = htobe32(INADDR_ANY), .ifindex = ifindex, @@ -262,17 +264,13 @@ int sd_dhcp_server_set_boot_filename(sd_dhcp_server *server, const char *filenam } int sd_dhcp_server_stop(sd_dhcp_server *server) { - bool running; - if (!server) return 0; - running = sd_dhcp_server_is_running(server); - - server->receive_message = sd_event_source_disable_unref(server->receive_message); + bool running = sd_dhcp_server_is_running(server); - server->fd_raw = safe_close(server->fd_raw); - server->fd = safe_close(server->fd); + server->raw_socket_fd = safe_close(server->raw_socket_fd); + server->io_event_source = sd_event_source_disable_unref(server->io_event_source); if (running) log_dhcp_server(server, "STOPPED"); @@ -327,15 +325,11 @@ int sd_dhcp_server_start(sd_dhcp_server *server) { assert_return(server, -EINVAL); assert_return(server->event, -EINVAL); + assert_return(server->address != INADDR_ANY, -EUNATCH); if (sd_dhcp_server_is_running(server)) return 0; - assert_return(!server->receive_message, -EBUSY); - assert_return(server->fd_raw < 0, -EBUSY); - assert_return(server->fd < 0, -EBUSY); - assert_return(server->address != htobe32(INADDR_ANY), -EUNATCH); - dhcp_server_update_lease_servers(server); r = dhcp_server_setup_io_event_source(server); diff --git a/src/libsystemd-network/test-dhcp-server.c b/src/libsystemd-network/test-dhcp-server.c index 86face3406c..bc9ab1e9ac2 100644 --- a/src/libsystemd-network/test-dhcp-server.c +++ b/src/libsystemd-network/test-dhcp-server.c @@ -10,6 +10,7 @@ #include "dhcp-server-internal.h" #include "dhcp-server-request.h" +#include "fd-util.h" #include "tests.h" TEST(basic) { @@ -19,14 +20,12 @@ TEST(basic) { struct in_addr address_any = { .s_addr = htobe32(INADDR_ANY), }; - int r; _cleanup_(sd_event_unrefp) sd_event *event = NULL; ASSERT_OK(sd_event_new(&event)); - /* attach to loopback interface */ _cleanup_(sd_dhcp_server_unrefp) sd_dhcp_server *server = NULL; - ASSERT_OK(sd_dhcp_server_new(&server, 1)); + ASSERT_OK(sd_dhcp_server_new(&server, 4242)); ASSERT_NOT_NULL(server); ASSERT_OK(sd_dhcp_server_attach_event(server, event, SD_EVENT_PRIORITY_NORMAL)); @@ -49,13 +48,12 @@ TEST(basic) { ASSERT_RETURN_EXPECTED(ASSERT_ERROR(sd_dhcp_server_configure_pool(server, &address_any, 8, 0, 1), EINVAL)); ASSERT_OK(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 1)); - r = sd_dhcp_server_start(server); - /* skip test if running in an environment with no full networking support, CONFIG_PACKET not - * compiled in kernel, nor af_packet module available. */ - if (IN_SET(r, -EPERM, -EAFNOSUPPORT)) - return (void) log_tests_skipped_errno(r, "cannot start dhcp server"); - ASSERT_OK(r); + _cleanup_close_pair_ int socket_fd[2] = EBADF_PAIR; + ASSERT_OK_ERRNO(socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, socket_fd)); + server->socket_fd = TAKE_FD(socket_fd[0]); + + ASSERT_OK(sd_dhcp_server_start(server)); ASSERT_OK(sd_dhcp_server_start(server)); ASSERT_OK(sd_dhcp_server_stop(server)); ASSERT_OK(sd_dhcp_server_stop(server)); @@ -112,13 +110,15 @@ TEST(dhcp_server_handle_message) { .s_addr = htobe32(INADDR_LOOPBACK + 42), }; static uint8_t static_lease_client_id[7] = {0x01, 'A', 'B', 'C', 'D', 'E', 'G' }; - int r; + + _cleanup_close_pair_ int socket_fd[2] = EBADF_PAIR; + ASSERT_OK_ERRNO(socketpair(AF_UNIX, SOCK_SEQPACKET | SOCK_CLOEXEC | SOCK_NONBLOCK, 0, socket_fd)); _cleanup_(sd_event_unrefp) sd_event *event = NULL; ASSERT_OK(sd_event_new(&event)); _cleanup_(sd_dhcp_server_unrefp) sd_dhcp_server *server = NULL; - ASSERT_OK(sd_dhcp_server_new(&server, 1)); + ASSERT_OK(sd_dhcp_server_new(&server, 4242)); ASSERT_OK(sd_dhcp_server_configure_pool(server, &address_lo, 8, 0, 0)); ASSERT_OK(sd_dhcp_server_set_static_lease( server, @@ -127,15 +127,10 @@ TEST(dhcp_server_handle_message) { ELEMENTSOF(static_lease_client_id), /* hostname= */ NULL)); ASSERT_OK(sd_dhcp_server_attach_event(server, event, SD_EVENT_PRIORITY_NORMAL)); - r = sd_dhcp_server_start(server); - if (IN_SET(r, -EPERM, -EAFNOSUPPORT)) - return (void) log_tests_skipped_errno(r, "cannot start dhcp server"); - ASSERT_OK(r); + server->socket_fd = TAKE_FD(socket_fd[0]); + ASSERT_OK(sd_dhcp_server_start(server)); - r = dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL); - if (r == -ENETDOWN) - return (void) log_tests_skipped("Network is not available"); - ASSERT_OK_EQ(r, DHCP_OFFER); + ASSERT_OK_EQ(dhcp_server_handle_message(server, (DHCPMessage*)&test, sizeof(test), NULL), DHCP_OFFER); test.end = 0; /* TODO, shouldn't this fail? */ @@ -273,7 +268,7 @@ TEST(dhcp_server_handle_message) { TEST(sd_dhcp_server_set_static_lease) { _cleanup_(sd_dhcp_server_unrefp) sd_dhcp_server *server = NULL; - ASSERT_OK(sd_dhcp_server_new(&server, 1)); + ASSERT_OK(sd_dhcp_server_new(&server, 4242)); ASSERT_OK(sd_dhcp_server_set_static_lease( server, @@ -351,7 +346,7 @@ TEST(sd_dhcp_server_set_static_lease) { TEST(sd_dhcp_server_set_domain_name) { _cleanup_(sd_dhcp_server_unrefp) sd_dhcp_server *server = NULL; - ASSERT_OK(sd_dhcp_server_new(&server, 1)); + ASSERT_OK(sd_dhcp_server_new(&server, 4242)); /* Test setting domain name */ ASSERT_OK_POSITIVE(sd_dhcp_server_set_domain_name(server, "example.com"));