From 924937cbc0bf692bc6e5b3a0bd3c18347d9521e9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 Apr 2023 16:40:36 +0900 Subject: timesync: drop unnecessary initialization --- src/timesync/timesyncd-manager.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index b8cc6f4ead..e024fa9715 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -411,7 +411,7 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re .msg_name = &server_addr, .msg_namelen = sizeof(server_addr), }; - struct timespec *recv_time = NULL; + struct timespec *recv_time; triple_timestamp dts; ssize_t len; double origin, receive, trans, dest, delay, offset, root_distance; -- cgit v1.2.3 From 4db752e4aaf807428458245e2b90beac4d779523 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 Apr 2023 15:20:49 +0900 Subject: socket-util: add one missing paren Follow-up for b6256af75e0609e451198ed90c293efd50827ab3. --- src/basic/socket-util.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 7d504319a8..00c2998adb 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -183,7 +183,7 @@ int flush_accept(int fd); * riscv32. */ #define CMSG_TYPED_DATA(cmsg, type) \ ({ \ - struct cmsghdr *_cmsg = cmsg; \ + struct cmsghdr *_cmsg = (cmsg); \ assert_cc(__alignof__(type) <= __alignof__(struct cmsghdr)); \ _cmsg ? CAST_ALIGN_PTR(type, CMSG_DATA(_cmsg)) : (type*) NULL; \ }) -- cgit v1.2.3 From 1113e50796315a2aaaf768a243e3788cdb4aac78 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 14 Apr 2023 13:55:31 +0900 Subject: tree-wide: replace __alignof__() with alignof() Addresses https://github.com/systemd/systemd/pull/27254#discussion_r1165267046. --- src/basic/socket-util.h | 2 +- src/boot/efi/pe.c | 2 +- src/fundamental/macro-fundamental.h | 11 ++++++----- src/network/networkd-nexthop.c | 2 +- src/test/test-sizeof.c | 10 +++++----- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index 00c2998adb..d6d63a4f33 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -184,7 +184,7 @@ int flush_accept(int fd); #define CMSG_TYPED_DATA(cmsg, type) \ ({ \ struct cmsghdr *_cmsg = (cmsg); \ - assert_cc(__alignof__(type) <= __alignof__(struct cmsghdr)); \ + assert_cc(alignof(type) <= alignof(struct cmsghdr)); \ _cmsg ? CAST_ALIGN_PTR(type, CMSG_DATA(_cmsg)) : (type*) NULL; \ }) diff --git a/src/boot/efi/pe.c b/src/boot/efi/pe.c index e516417c07..9759d036b3 100644 --- a/src/boot/efi/pe.c +++ b/src/boot/efi/pe.c @@ -191,7 +191,7 @@ static uint32_t get_compatibility_entry_address(const DosFileHeader *dos, const uint32_t entry_point; } _packed_ LinuxPeCompat1; - while (size >= sizeof(LinuxPeCompat1) && addr % __alignof__(LinuxPeCompat1) == 0) { + while (size >= sizeof(LinuxPeCompat1) && addr % alignof(LinuxPeCompat1) == 0) { LinuxPeCompat1 *compat = (LinuxPeCompat1 *) ((uint8_t *) dos + addr); if (compat->type == 0 || compat->size == 0 || compat->size > size) diff --git a/src/fundamental/macro-fundamental.h b/src/fundamental/macro-fundamental.h index fa5b5d221a..e901e8fb59 100644 --- a/src/fundamental/macro-fundamental.h +++ b/src/fundamental/macro-fundamental.h @@ -6,12 +6,13 @@ #endif #include +#include #include #include #include #define _align_(x) __attribute__((__aligned__(x))) -#define _alignas_(x) __attribute__((__aligned__(__alignof__(x)))) +#define _alignas_(x) __attribute__((__aligned__(alignof(x)))) #define _alignptr_ __attribute__((__aligned__(sizeof(void *)))) #define _cleanup_(x) __attribute__((__cleanup__(x))) #define _const_ __attribute__((__const__)) @@ -343,9 +344,9 @@ static inline size_t ALIGN_TO(size_t l, size_t ali) { #define ALIGN_PTR(p) ((void*) ALIGN((uintptr_t) (p))) /* Checks if the specified pointer is aligned as appropriate for the specific type */ -#define IS_ALIGNED16(p) (((uintptr_t) p) % __alignof__(uint16_t) == 0) -#define IS_ALIGNED32(p) (((uintptr_t) p) % __alignof__(uint32_t) == 0) -#define IS_ALIGNED64(p) (((uintptr_t) p) % __alignof__(uint64_t) == 0) +#define IS_ALIGNED16(p) (((uintptr_t) p) % alignof(uint16_t) == 0) +#define IS_ALIGNED32(p) (((uintptr_t) p) % alignof(uint32_t) == 0) +#define IS_ALIGNED64(p) (((uintptr_t) p) % alignof(uint64_t) == 0) /* Same as ALIGN_TO but callable in constant contexts. */ #define CONST_ALIGN_TO(l, ali) \ @@ -363,7 +364,7 @@ static inline size_t ALIGN_TO(size_t l, size_t ali) { #define CAST_ALIGN_PTR(t, p) \ ({ \ const void *_p = (p); \ - assert(((uintptr_t) _p) % __alignof__(t) == 0); \ + assert(((uintptr_t) _p) % alignof(t) == 0); \ (t *) _p; \ }) diff --git a/src/network/networkd-nexthop.c b/src/network/networkd-nexthop.c index d82766702a..0820e0db2d 100644 --- a/src/network/networkd-nexthop.c +++ b/src/network/networkd-nexthop.c @@ -894,7 +894,7 @@ int manager_rtnl_process_nexthop(sd_netlink *rtnl, sd_netlink_message *message, return 0; } - assert((uintptr_t) group % __alignof__(struct nexthop_grp) == 0); + assert((uintptr_t) group % alignof(struct nexthop_grp) == 0); n_group = raw_group_size / sizeof(struct nexthop_grp); for (size_t i = 0; i < n_group; i++) { diff --git a/src/test/test-sizeof.c b/src/test/test-sizeof.c index 55bd81e22f..30b252ecd9 100644 --- a/src/test/test-sizeof.c +++ b/src/test/test-sizeof.c @@ -17,16 +17,16 @@ DISABLE_WARNING_TYPE_LIMITS; #define info_no_sign(t) \ - printf("%s → %zu bits, %zu byte alignment\n", STRINGIFY(t), \ + printf("%s → %zu bits, %zu byte alignment\n", STRINGIFY(t), \ sizeof(t)*CHAR_BIT, \ - __alignof__(t)) + alignof(t)) #define info(t) \ - printf("%s → %zu bits%s, %zu byte alignment\n", STRINGIFY(t), \ + printf("%s → %zu bits%s, %zu byte alignment\n", STRINGIFY(t), \ sizeof(t)*CHAR_BIT, \ strstr(STRINGIFY(t), "signed") ? "" : \ (t)-1 < (t)0 ? ", signed" : ", unsigned", \ - __alignof__(t)) + alignof(t)) enum Enum { enum_value, @@ -44,7 +44,7 @@ enum BigEnum2 { int main(void) { int (*function_pointer)(void); - info_no_sign(function_pointer); + info_no_sign(typeof(function_pointer)); info_no_sign(void*); info(char*); -- cgit v1.2.3 From 4836f4c67d040f51b7d1aa45b73553eb0da26097 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 Apr 2023 18:00:41 +0900 Subject: socket-util: introduce CMSG_FIND_AND_COPY_DATA() The cmd(3) man page says about CMSG_DATA(): > The pointer returned cannot be assumed to be suitably aligned for > accessing arbitrary payload data types. Applications should not cast > it to a pointer type matching the payload, but should instead use > memcpy(3) to copy data to or from a suitably declared object. Hence, if we want to use unaligned data in cmsg, we need to copy it before use. That's typically important for reading timestamps in RISCV32, as the time_t is 64bit and size_t is 32bit on the system. --- src/basic/socket-util.c | 18 ++++++++++++++++++ src/basic/socket-util.h | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/src/basic/socket-util.c b/src/basic/socket-util.c index b35de4dad6..5b76948c06 100644 --- a/src/basic/socket-util.c +++ b/src/basic/socket-util.c @@ -1171,6 +1171,24 @@ struct cmsghdr* cmsg_find(struct msghdr *mh, int level, int type, socklen_t leng return NULL; } +void* cmsg_find_and_copy_data(struct msghdr *mh, int level, int type, void *buf, size_t buf_len) { + struct cmsghdr *cmsg; + + assert(mh); + assert(buf); + assert(buf_len > 0); + + /* This is similar to cmsg_find_data(), but copy the found data to buf. This should be typically used + * when reading possibly unaligned data such as timestamp, as time_t is 64bit and size_t is 32bit on + * RISCV32. See issue #27241. */ + + cmsg = cmsg_find(mh, level, type, CMSG_LEN(buf_len)); + if (!cmsg) + return NULL; + + return memcpy_safe(buf, CMSG_DATA(cmsg), buf_len); +} + int socket_ioctl_fd(void) { int fd; diff --git a/src/basic/socket-util.h b/src/basic/socket-util.h index d6d63a4f33..b323b1b99f 100644 --- a/src/basic/socket-util.h +++ b/src/basic/socket-util.h @@ -189,11 +189,16 @@ int flush_accept(int fd); }) struct cmsghdr* cmsg_find(struct msghdr *mh, int level, int type, socklen_t length); +void* cmsg_find_and_copy_data(struct msghdr *mh, int level, int type, void *buf, size_t buf_len); /* Type-safe, dereferencing version of cmsg_find() */ #define CMSG_FIND_DATA(mh, level, type, ctype) \ CMSG_TYPED_DATA(cmsg_find(mh, level, type, CMSG_LEN(sizeof(ctype))), ctype) +/* Type-safe version of cmsg_find_and_copy_data() */ +#define CMSG_FIND_AND_COPY_DATA(mh, level, type, ctype) \ + (ctype*) cmsg_find_and_copy_data(mh, level, type, &(ctype){}, sizeof(ctype)) + /* Resolves to a type that can carry cmsghdr structures. Make sure things are properly aligned, i.e. the type * itself is placed properly in memory and the size is also aligned to what's appropriate for "cmsghdr" * structures. */ -- cgit v1.2.3 From 789f5c6f706383442d85a9aedc6396a1a7877057 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 Apr 2023 18:02:48 +0900 Subject: tree-wide: copy timestamp data from cmsg On RISCV32, time_t is 64bit and size_t is 32bit, hence the timestamp data in message header may not be aligned. Fixes #27241. --- src/journal/journald-server.c | 6 +++--- src/libsystemd-network/icmp6-util.c | 8 +++++--- src/libsystemd-network/sd-dhcp6-client.c | 10 +++------- src/timesync/timesyncd-manager.c | 2 +- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c index b952269e2e..7be3763f15 100644 --- a/src/journal/journald-server.c +++ b/src/journal/journald-server.c @@ -1417,7 +1417,7 @@ int server_process_datagram( size_t label_len = 0, m; Server *s = ASSERT_PTR(userdata); struct ucred *ucred = NULL; - struct timeval *tv = NULL; + struct timeval tv_buf, *tv = NULL; struct cmsghdr *cmsg; char *label = NULL; struct iovec iovec; @@ -1493,10 +1493,10 @@ int server_process_datagram( label = CMSG_TYPED_DATA(cmsg, char); label_len = cmsg->cmsg_len - CMSG_LEN(0); } else if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SO_TIMESTAMP && + cmsg->cmsg_type == SCM_TIMESTAMP && cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval))) { assert(!tv); - tv = CMSG_TYPED_DATA(cmsg, struct timeval); + tv = memcpy(&tv_buf, CMSG_DATA(cmsg), sizeof(struct timeval)); } else if (cmsg->cmsg_level == SOL_SOCKET && cmsg->cmsg_type == SCM_RIGHTS) { assert(!fds); diff --git a/src/libsystemd-network/icmp6-util.c b/src/libsystemd-network/icmp6-util.c index fba5c3bd96..ecddab61e4 100644 --- a/src/libsystemd-network/icmp6-util.c +++ b/src/libsystemd-network/icmp6-util.c @@ -199,9 +199,11 @@ int icmp6_receive(int fd, void *buffer, size_t size, struct in6_addr *ret_dst, } if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SO_TIMESTAMP && - cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval))) - triple_timestamp_from_realtime(&t, timeval_load(CMSG_TYPED_DATA(cmsg, struct timeval))); + cmsg->cmsg_type == SCM_TIMESTAMP && + cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval))) { + struct timeval *tv = memcpy(&(struct timeval) {}, CMSG_DATA(cmsg), sizeof(struct timeval)); + triple_timestamp_from_realtime(&t, timeval_load(tv)); + } } if (!triple_timestamp_is_set(&t)) diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c index 57dd91f81f..6d62ba380b 100644 --- a/src/libsystemd-network/sd-dhcp6-client.c +++ b/src/libsystemd-network/sd-dhcp6-client.c @@ -1276,7 +1276,6 @@ static int client_receive_message( .msg_control = &control, .msg_controllen = sizeof(control), }; - struct cmsghdr *cmsg; triple_timestamp t = {}; _cleanup_free_ DHCP6Message *message = NULL; struct in6_addr *server_address = NULL; @@ -1320,12 +1319,9 @@ static int client_receive_message( server_address = &sa.in6.sin6_addr; } - CMSG_FOREACH(cmsg, &msg) { - if (cmsg->cmsg_level == SOL_SOCKET && - cmsg->cmsg_type == SO_TIMESTAMP && - cmsg->cmsg_len == CMSG_LEN(sizeof(struct timeval))) - triple_timestamp_from_realtime(&t, timeval_load(CMSG_TYPED_DATA(cmsg, struct timeval))); - } + struct timeval *tv = CMSG_FIND_AND_COPY_DATA(&msg, SOL_SOCKET, SCM_TIMESTAMP, struct timeval); + if (tv) + triple_timestamp_from_realtime(&t, timeval_load(tv)); if (client->transaction_id != (message->transaction_id & htobe32(0x00ffffff))) return 0; diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c index e024fa9715..569389b9d4 100644 --- a/src/timesync/timesyncd-manager.c +++ b/src/timesync/timesyncd-manager.c @@ -446,7 +446,7 @@ static int manager_receive_response(sd_event_source *source, int fd, uint32_t re return 0; } - recv_time = CMSG_FIND_DATA(&msghdr, SOL_SOCKET, SCM_TIMESTAMPNS, struct timespec); + recv_time = CMSG_FIND_AND_COPY_DATA(&msghdr, SOL_SOCKET, SCM_TIMESTAMPNS, struct timespec); if (!recv_time) return log_error_errno(SYNTHETIC_ERRNO(EINVAL), "Packet timestamp missing."); -- cgit v1.2.3 From 1ebb0953f01051cd85dd5d0a03acf309a8f3c3b9 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 Apr 2023 18:34:09 +0900 Subject: sd-dhcp-server: use CMSG_FIND_DATA() at one more place --- src/libsystemd-network/sd-dhcp-server.c | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index 15a0145959..b9c77eafa5 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -1270,7 +1270,6 @@ static int server_receive_message(sd_event_source *s, int fd, .msg_control = &control, .msg_controllen = sizeof(control), }; - struct cmsghdr *cmsg; ssize_t datagram_size, len; int r; @@ -1306,19 +1305,10 @@ static int server_receive_message(sd_event_source *s, int fd, if ((size_t) len < sizeof(DHCPMessage)) return 0; - CMSG_FOREACH(cmsg, &msg) - if (cmsg->cmsg_level == IPPROTO_IP && - cmsg->cmsg_type == IP_PKTINFO && - cmsg->cmsg_len == CMSG_LEN(sizeof(struct in_pktinfo))) { - struct in_pktinfo *info = CMSG_TYPED_DATA(cmsg, struct in_pktinfo); - - /* TODO figure out if this can be done as a filter on - * the socket, like for IPv6 */ - if (server->ifindex != info->ipi_ifindex) - return 0; - - break; - } + /* TODO figure out if this can be done as a filter on the socket, like for IPv6 */ + struct in_pktinfo *info = CMSG_FIND_DATA(&msg, IPPROTO_IP, IP_PKTINFO, struct in_pktinfo); + if (info && info->ipi_ifindex != server->ifindex) + return 0; if (sd_dhcp_server_is_in_relay_mode(server)) { r = dhcp_server_relay_message(server, message, len - sizeof(DHCPMessage), buflen); -- cgit v1.2.3 From b5d39bb3cae7e36c284fcfcc87a42ff8a2bae7f5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 13 Apr 2023 18:34:59 +0900 Subject: tree-wide: also use CMSG_TYPED_DATA() on writing message header --- src/libsystemd-network/sd-dhcp-server.c | 2 +- src/libsystemd/sd-daemon/sd-daemon.c | 2 +- src/resolve/resolved-manager.c | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index b9c77eafa5..05c0cddfd0 100644 --- a/src/libsystemd-network/sd-dhcp-server.c +++ b/src/libsystemd-network/sd-dhcp-server.c @@ -407,7 +407,7 @@ static int dhcp_server_send_udp(sd_dhcp_server *server, be32_t destination, rather than binding the socket. This will be mostly useful when we gain support for arbitrary number of server addresses */ - pktinfo = (struct in_pktinfo*) CMSG_DATA(cmsg); + pktinfo = CMSG_TYPED_DATA(cmsg, struct in_pktinfo); assert(pktinfo); pktinfo->ipi_ifindex = server->ifindex; diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c index 8dc11aeb30..f2f295d6e4 100644 --- a/src/libsystemd/sd-daemon/sd-daemon.c +++ b/src/libsystemd/sd-daemon/sd-daemon.c @@ -567,7 +567,7 @@ _public_ int sd_pid_notify_with_fds( cmsg->cmsg_type = SCM_CREDENTIALS; cmsg->cmsg_len = CMSG_LEN(sizeof(struct ucred)); - ucred = (struct ucred*) CMSG_DATA(cmsg); + ucred = CMSG_TYPED_DATA(cmsg, struct ucred); ucred->pid = pid != 0 ? pid : getpid_cached(); ucred->uid = getuid(); ucred->gid = getgid(); diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c index e5e1411a13..184d8e3f3d 100644 --- a/src/resolve/resolved-manager.c +++ b/src/resolve/resolved-manager.c @@ -1017,7 +1017,7 @@ static int manager_ipv4_send( cmsg->cmsg_level = IPPROTO_IP; cmsg->cmsg_type = IP_PKTINFO; - pi = (struct in_pktinfo*) CMSG_DATA(cmsg); + pi = CMSG_TYPED_DATA(cmsg, struct in_pktinfo); pi->ipi_ifindex = ifindex; if (source) @@ -1073,7 +1073,7 @@ static int manager_ipv6_send( cmsg->cmsg_level = IPPROTO_IPV6; cmsg->cmsg_type = IPV6_PKTINFO; - pi = (struct in6_pktinfo*) CMSG_DATA(cmsg); + pi = CMSG_TYPED_DATA(cmsg, struct in6_pktinfo); pi->ipi6_ifindex = ifindex; if (source) -- cgit v1.2.3