From 0404a29bfe7a24ca0c6d4f94405a01f6ff942f43 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 9 Jul 2014 15:47:24 +0200 Subject: [PATCH 1/8] kernel-netlink: Define netlink buffer as an union having a netlink header This allows us to streamline the netlink buffers, and avoid extensive casting. --- src/conftest/hooks/reset_seq.c | 2 +- .../kernel_netlink/kernel_netlink_ipsec.c | 28 +++++++++---------- .../kernel_netlink/kernel_netlink_net.c | 10 +++---- .../kernel_netlink/kernel_netlink_shared.h | 5 +++- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/src/conftest/hooks/reset_seq.c b/src/conftest/hooks/reset_seq.c index a77b10eee..717bcdbb9 100644 --- a/src/conftest/hooks/reset_seq.c +++ b/src/conftest/hooks/reset_seq.c @@ -108,7 +108,7 @@ static job_requeue_t reset_cb(struct reset_cb_data_t *data) memset(&request, 0, sizeof(request)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_REPLACE; hdr->nlmsg_seq = 201; hdr->nlmsg_pid = getpid(); diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index 274af7954..b00c04102 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -1065,7 +1065,7 @@ static status_t get_spi_internal(private_kernel_netlink_ipsec_t *this, memset(&request, 0, sizeof(request)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST; hdr->nlmsg_type = XFRM_MSG_ALLOCSPI; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userspi_info)); @@ -1214,7 +1214,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, DBG2(DBG_KNL, "adding SAD entry with SPI %.8x and reqid {%u} (mark " "%u/0x%08x)", ntohl(spi), reqid, mark.value, mark.mask); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = inbound ? XFRM_MSG_UPDSA : XFRM_MSG_NEWSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info)); @@ -1521,7 +1521,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, status = SUCCESS; failed: - memwipe(request, sizeof(request)); + memwipe(&request, sizeof(request)); return status; } @@ -1549,7 +1549,7 @@ static void get_replay_state(private_kernel_netlink_ipsec_t *this, DBG2(DBG_KNL, "querying replay state from SAD entry with SPI %.8x", ntohl(spi)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST; hdr->nlmsg_type = XFRM_MSG_GETAE; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_aevent_id)); @@ -1641,7 +1641,7 @@ METHOD(kernel_ipsec_t, query_sa, status_t, DBG2(DBG_KNL, "querying SAD entry with SPI %.8x (mark %u/0x%08x)", ntohl(spi), mark.value, mark.mask); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST; hdr->nlmsg_type = XFRM_MSG_GETSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_id)); @@ -1744,7 +1744,7 @@ METHOD(kernel_ipsec_t, del_sa, status_t, DBG2(DBG_KNL, "deleting SAD entry with SPI %.8x (mark %u/0x%08x)", ntohl(spi), mark.value, mark.mask); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = XFRM_MSG_DELSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_id)); @@ -1813,7 +1813,7 @@ METHOD(kernel_ipsec_t, update_sa, status_t, DBG2(DBG_KNL, "querying SAD entry with SPI %.8x for update", ntohl(spi)); /* query the existing SA first */ - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST; hdr->nlmsg_type = XFRM_MSG_GETSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_id)); @@ -1876,7 +1876,7 @@ METHOD(kernel_ipsec_t, update_sa, status_t, DBG2(DBG_KNL, "updating SAD entry with SPI %.8x from %#H..%#H to %#H..%#H", ntohl(spi), src, dst, new_src, new_dst); /* copy over the SA from out to request */ - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = XFRM_MSG_NEWSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info)); @@ -1967,7 +1967,7 @@ failed: free(replay); free(replay_esn); memwipe(out, len); - memwipe(request, sizeof(request)); + memwipe(&request, sizeof(request)); free(out); return status; @@ -1984,7 +1984,7 @@ METHOD(kernel_ipsec_t, flush_sas, status_t, DBG2(DBG_KNL, "flushing all SAD entries"); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = XFRM_MSG_FLUSHSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_flush)); @@ -2020,7 +2020,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this, memcpy(&clone, policy, sizeof(policy_entry_t)); memset(&request, 0, sizeof(request)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = update ? XFRM_MSG_UPDPOLICY : XFRM_MSG_NEWPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_info)); @@ -2344,7 +2344,7 @@ METHOD(kernel_ipsec_t, query_policy, status_t, src_ts, dst_ts, policy_dir_names, direction, mark.value, mark.mask); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST; hdr->nlmsg_type = XFRM_MSG_GETPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_id)); @@ -2501,7 +2501,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t, memset(&request, 0, sizeof(request)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = XFRM_MSG_DELPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_id)); @@ -2560,7 +2560,7 @@ METHOD(kernel_ipsec_t, flush_policies, status_t, DBG2(DBG_KNL, "flushing all policies from SPD"); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = XFRM_MSG_FLUSHPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(0); /* no data associated */ diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c index 6594b8cca..ccb097e0e 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c @@ -1625,7 +1625,7 @@ static host_t *get_route(private_kernel_netlink_net_t *this, host_t *dest, memset(&request, 0, sizeof(request)); family = dest->get_family(dest); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST; if (family == AF_INET || this->rta_prefsrc_for_ipv6 || this->routing_table || match_net) @@ -1864,7 +1864,7 @@ static status_t manage_ipaddr(private_kernel_netlink_net_t *this, int nlmsg_type chunk = ip->get_address(ip); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | flags; hdr->nlmsg_type = nlmsg_type; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg)); @@ -2092,7 +2092,7 @@ static status_t manage_srcroute(private_kernel_netlink_net_t *this, memset(&request, 0, sizeof(request)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | flags; hdr->nlmsg_type = nlmsg_type; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)); @@ -2221,7 +2221,7 @@ static status_t init_address_list(private_kernel_netlink_net_t *this) memset(&request, 0, sizeof(request)); - in = (struct nlmsghdr*)&request; + in = &request.hdr; in->nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)); in->nlmsg_flags = NLM_F_REQUEST | NLM_F_MATCH | NLM_F_ROOT; msg = (struct rtgenmsg*)NLMSG_DATA(in); @@ -2308,7 +2308,7 @@ static status_t manage_rule(private_kernel_netlink_net_t *this, int nlmsg_type, char *fwmark; memset(&request, 0, sizeof(request)); - hdr = (struct nlmsghdr*)request; + hdr = &request.hdr; hdr->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; hdr->nlmsg_type = nlmsg_type; if (nlmsg_type == RTM_NEWRULE) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h index 8be935bc3..1a03a89a8 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h @@ -26,7 +26,10 @@ * 1024 byte is currently sufficient for all operations. Some platform * require an enforced aligment to four bytes (e.g. ARM). */ -typedef u_char netlink_buf_t[1024] __attribute__((aligned(RTA_ALIGNTO))); +typedef union { + struct nlmsghdr hdr; + u_char bytes[1024]; +} netlink_buf_t __attribute__((aligned(RTA_ALIGNTO))); typedef struct netlink_socket_t netlink_socket_t; From 4c438cf0992ff20b02e2f5ef0c656fcb18fac02b Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 9 Jul 2014 15:53:14 +0200 Subject: [PATCH 2/8] kernel-netlink: Avoid casting the NLMSG_DATA() return value There is really no need for doing so, and it makes the code just unreadable. --- .../kernel_netlink/kernel_netlink_ipsec.c | 32 +++++++++---------- .../kernel_netlink/kernel_netlink_net.c | 18 +++++------ .../kernel_netlink/kernel_netlink_shared.c | 2 +- 3 files changed, 26 insertions(+), 26 deletions(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index b00c04102..cd94168c4 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -816,7 +816,7 @@ static void process_acquire(private_kernel_netlink_ipsec_t *this, u_int32_t reqid = 0; int proto = 0; - acquire = (struct xfrm_user_acquire*)NLMSG_DATA(hdr); + acquire = NLMSG_DATA(hdr); rta = XFRM_RTA(hdr, struct xfrm_user_acquire); rtasize = XFRM_PAYLOAD(hdr, struct xfrm_user_acquire); @@ -862,7 +862,7 @@ static void process_expire(private_kernel_netlink_ipsec_t *this, u_int32_t spi, reqid; u_int8_t protocol; - expire = (struct xfrm_user_expire*)NLMSG_DATA(hdr); + expire = NLMSG_DATA(hdr); protocol = expire->state.id.proto; spi = expire->state.id.spi; reqid = expire->state.reqid; @@ -896,7 +896,7 @@ static void process_migrate(private_kernel_netlink_ipsec_t *this, u_int32_t reqid = 0; policy_dir_t dir; - policy_id = (struct xfrm_userpolicy_id*)NLMSG_DATA(hdr); + policy_id = NLMSG_DATA(hdr); rta = XFRM_RTA(hdr, struct xfrm_userpolicy_id); rtasize = XFRM_PAYLOAD(hdr, struct xfrm_userpolicy_id); @@ -963,7 +963,7 @@ static void process_mapping(private_kernel_netlink_ipsec_t *this, struct xfrm_user_mapping *mapping; u_int32_t spi, reqid; - mapping = (struct xfrm_user_mapping*)NLMSG_DATA(hdr); + mapping = NLMSG_DATA(hdr); spi = mapping->id.spi; reqid = mapping->reqid; @@ -1070,7 +1070,7 @@ static status_t get_spi_internal(private_kernel_netlink_ipsec_t *this, hdr->nlmsg_type = XFRM_MSG_ALLOCSPI; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userspi_info)); - userspi = (struct xfrm_userspi_info*)NLMSG_DATA(hdr); + userspi = NLMSG_DATA(hdr); host2xfrm(src, &userspi->info.saddr); host2xfrm(dst, &userspi->info.id.daddr); userspi->info.id.proto = proto; @@ -1219,7 +1219,7 @@ METHOD(kernel_ipsec_t, add_sa, status_t, hdr->nlmsg_type = inbound ? XFRM_MSG_UPDSA : XFRM_MSG_NEWSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_info)); - sa = (struct xfrm_usersa_info*)NLMSG_DATA(hdr); + sa = NLMSG_DATA(hdr); host2xfrm(src, &sa->saddr); host2xfrm(dst, &sa->id.daddr); sa->id.spi = spi; @@ -1554,7 +1554,7 @@ static void get_replay_state(private_kernel_netlink_ipsec_t *this, hdr->nlmsg_type = XFRM_MSG_GETAE; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_aevent_id)); - aevent_id = (struct xfrm_aevent_id*)NLMSG_DATA(hdr); + aevent_id = NLMSG_DATA(hdr); aevent_id->flags = XFRM_AE_RVAL; host2xfrm(dst, &aevent_id->sa_id.daddr); @@ -1646,7 +1646,7 @@ METHOD(kernel_ipsec_t, query_sa, status_t, hdr->nlmsg_type = XFRM_MSG_GETSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_id)); - sa_id = (struct xfrm_usersa_id*)NLMSG_DATA(hdr); + sa_id = NLMSG_DATA(hdr); host2xfrm(dst, &sa_id->daddr); sa_id->spi = spi; sa_id->proto = protocol; @@ -1666,7 +1666,7 @@ METHOD(kernel_ipsec_t, query_sa, status_t, { case XFRM_MSG_NEWSA: { - sa = (struct xfrm_usersa_info*)NLMSG_DATA(hdr); + sa = NLMSG_DATA(hdr); break; } case NLMSG_ERROR: @@ -1749,7 +1749,7 @@ METHOD(kernel_ipsec_t, del_sa, status_t, hdr->nlmsg_type = XFRM_MSG_DELSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_id)); - sa_id = (struct xfrm_usersa_id*)NLMSG_DATA(hdr); + sa_id = NLMSG_DATA(hdr); host2xfrm(dst, &sa_id->daddr); sa_id->spi = spi; sa_id->proto = protocol; @@ -1818,7 +1818,7 @@ METHOD(kernel_ipsec_t, update_sa, status_t, hdr->nlmsg_type = XFRM_MSG_GETSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_id)); - sa_id = (struct xfrm_usersa_id*)NLMSG_DATA(hdr); + sa_id = NLMSG_DATA(hdr); host2xfrm(dst, &sa_id->daddr); sa_id->spi = spi; sa_id->proto = protocol; @@ -1989,7 +1989,7 @@ METHOD(kernel_ipsec_t, flush_sas, status_t, hdr->nlmsg_type = XFRM_MSG_FLUSHSA; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_usersa_flush)); - flush = (struct xfrm_usersa_flush*)NLMSG_DATA(hdr); + flush = NLMSG_DATA(hdr); flush->proto = IPSEC_PROTO_ANY; if (this->socket_xfrm->send_ack(this->socket_xfrm, hdr) != SUCCESS) @@ -2025,7 +2025,7 @@ static status_t add_policy_internal(private_kernel_netlink_ipsec_t *this, hdr->nlmsg_type = update ? XFRM_MSG_UPDPOLICY : XFRM_MSG_NEWPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_info)); - policy_info = (struct xfrm_userpolicy_info*)NLMSG_DATA(hdr); + policy_info = NLMSG_DATA(hdr); policy_info->sel = policy->sel; policy_info->dir = policy->direction; @@ -2349,7 +2349,7 @@ METHOD(kernel_ipsec_t, query_policy, status_t, hdr->nlmsg_type = XFRM_MSG_GETPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_id)); - policy_id = (struct xfrm_userpolicy_id*)NLMSG_DATA(hdr); + policy_id = NLMSG_DATA(hdr); policy_id->sel = ts2selector(src_ts, dst_ts); policy_id->dir = direction; @@ -2367,7 +2367,7 @@ METHOD(kernel_ipsec_t, query_policy, status_t, { case XFRM_MSG_NEWPOLICY: { - policy = (struct xfrm_userpolicy_info*)NLMSG_DATA(hdr); + policy = NLMSG_DATA(hdr); break; } case NLMSG_ERROR: @@ -2506,7 +2506,7 @@ METHOD(kernel_ipsec_t, del_policy, status_t, hdr->nlmsg_type = XFRM_MSG_DELPOLICY; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_id)); - policy_id = (struct xfrm_userpolicy_id*)NLMSG_DATA(hdr); + policy_id = NLMSG_DATA(hdr); policy_id->sel = current->sel; policy_id->dir = direction; diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c index ccb097e0e..3d5538c35 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c @@ -938,7 +938,7 @@ static void addr_entry_unregister(addr_entry_t *addr, iface_entry_t *iface, static void process_link(private_kernel_netlink_net_t *this, struct nlmsghdr *hdr, bool event) { - struct ifinfomsg* msg = (struct ifinfomsg*)(NLMSG_DATA(hdr)); + struct ifinfomsg* msg = NLMSG_DATA(hdr); struct rtattr *rta = IFLA_RTA(msg); size_t rtasize = IFLA_PAYLOAD (hdr); enumerator_t *enumerator; @@ -1040,7 +1040,7 @@ static void process_link(private_kernel_netlink_net_t *this, static void process_addr(private_kernel_netlink_net_t *this, struct nlmsghdr *hdr, bool event) { - struct ifaddrmsg* msg = (struct ifaddrmsg*)(NLMSG_DATA(hdr)); + struct ifaddrmsg* msg = NLMSG_DATA(hdr); struct rtattr *rta = IFA_RTA(msg); size_t rtasize = IFA_PAYLOAD (hdr); host_t *host = NULL; @@ -1183,7 +1183,7 @@ static void process_addr(private_kernel_netlink_net_t *this, */ static void process_route(private_kernel_netlink_net_t *this, struct nlmsghdr *hdr) { - struct rtmsg* msg = (struct rtmsg*)(NLMSG_DATA(hdr)); + struct rtmsg* msg = NLMSG_DATA(hdr); struct rtattr *rta = RTM_RTA(msg); size_t rtasize = RTM_PAYLOAD(hdr); u_int32_t rta_oif = 0; @@ -1540,7 +1540,7 @@ static rt_entry_t *parse_route(struct nlmsghdr *hdr, rt_entry_t *route) struct rtmsg *msg; size_t rtasize; - msg = (struct rtmsg*)(NLMSG_DATA(hdr)); + msg = NLMSG_DATA(hdr); rta = RTM_RTA(msg); rtasize = RTM_PAYLOAD(hdr); @@ -1637,7 +1637,7 @@ static host_t *get_route(private_kernel_netlink_net_t *this, host_t *dest, hdr->nlmsg_type = RTM_GETROUTE; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)); - msg = (struct rtmsg*)NLMSG_DATA(hdr); + msg = NLMSG_DATA(hdr); msg->rtm_family = family; if (candidate) { @@ -1869,7 +1869,7 @@ static status_t manage_ipaddr(private_kernel_netlink_net_t *this, int nlmsg_type hdr->nlmsg_type = nlmsg_type; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct ifaddrmsg)); - msg = (struct ifaddrmsg*)NLMSG_DATA(hdr); + msg = NLMSG_DATA(hdr); msg->ifa_family = ip->get_family(ip); msg->ifa_flags = 0; msg->ifa_prefixlen = prefix < 0 ? chunk.len * 8 : prefix; @@ -2097,7 +2097,7 @@ static status_t manage_srcroute(private_kernel_netlink_net_t *this, hdr->nlmsg_type = nlmsg_type; hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)); - msg = (struct rtmsg*)NLMSG_DATA(hdr); + msg = NLMSG_DATA(hdr); msg->rtm_family = src_ip->get_family(src_ip); msg->rtm_dst_len = prefixlen; msg->rtm_table = this->routing_table; @@ -2224,7 +2224,7 @@ static status_t init_address_list(private_kernel_netlink_net_t *this) in = &request.hdr; in->nlmsg_len = NLMSG_LENGTH(sizeof(struct rtgenmsg)); in->nlmsg_flags = NLM_F_REQUEST | NLM_F_MATCH | NLM_F_ROOT; - msg = (struct rtgenmsg*)NLMSG_DATA(in); + msg = NLMSG_DATA(in); msg->rtgen_family = AF_UNSPEC; /* get all links */ @@ -2317,7 +2317,7 @@ static status_t manage_rule(private_kernel_netlink_net_t *this, int nlmsg_type, } hdr->nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg)); - msg = (struct rtmsg*)NLMSG_DATA(hdr); + msg = NLMSG_DATA(hdr); msg->rtm_table = table; msg->rtm_family = family; msg->rtm_protocol = RTPROT_BOOT; diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c index fd00c23af..30d5aaabd 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c @@ -197,7 +197,7 @@ METHOD(netlink_socket_t, netlink_send_ack, status_t, { case NLMSG_ERROR: { - struct nlmsgerr* err = (struct nlmsgerr*)NLMSG_DATA(hdr); + struct nlmsgerr* err = NLMSG_DATA(hdr); if (err->error) { From de11d34abb49a6ca64b98f586821d98312162a64 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 9 Jul 2014 16:04:16 +0200 Subject: [PATCH 3/8] kernel-netlink: Use recv() instead of recvfrom() As we are not interested in the returned address, there is really no need in passing that argument. --- .../plugins/kernel_netlink/kernel_netlink_shared.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c index 30d5aaabd..427499108 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c @@ -65,7 +65,7 @@ METHOD(netlink_socket_t, netlink_send, status_t, private_netlink_socket_t *this, struct nlmsghdr *in, struct nlmsghdr **out, size_t *out_len) { - int len, addr_len; + int len; struct sockaddr_nl addr; chunk_t result = chunk_empty, tmp; struct nlmsghdr *msg, peek; @@ -113,14 +113,7 @@ METHOD(netlink_socket_t, netlink_send, status_t, tmp.ptr = buf; msg = (struct nlmsghdr*)tmp.ptr; - memset(&addr, 0, sizeof(addr)); - addr.nl_family = AF_NETLINK; - addr.nl_pid = getpid(); - addr.nl_groups = 0; - addr_len = sizeof(addr); - - len = recvfrom(this->socket, tmp.ptr, tmp.len, 0, - (struct sockaddr*)&addr, &addr_len); + len = recv(this->socket, tmp.ptr, tmp.len, 0); if (len < 0) { @@ -161,8 +154,7 @@ METHOD(netlink_socket_t, netlink_send, status_t, /* NLM_F_MULTI flag does not seem to be set correctly, we use sequence * numbers to detect multi header messages */ - len = recvfrom(this->socket, &peek, sizeof(peek), MSG_PEEK | MSG_DONTWAIT, - (struct sockaddr*)&addr, &addr_len); + len = recv(this->socket, &peek, sizeof(peek), MSG_PEEK | MSG_DONTWAIT); if (len == sizeof(peek) && peek.nlmsg_seq == this->seq) { From 4c3aca273fdf14f12072f3e9fd87ef75782e55fb Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 9 Jul 2014 16:16:16 +0200 Subject: [PATCH 4/8] kernel-netlink: Clean up response buffer management --- .../kernel_netlink/kernel_netlink_shared.c | 40 ++++++++----------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c index 427499108..12df7c5ad 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c @@ -65,10 +65,13 @@ METHOD(netlink_socket_t, netlink_send, status_t, private_netlink_socket_t *this, struct nlmsghdr *in, struct nlmsghdr **out, size_t *out_len) { - int len; + union { + struct nlmsghdr hdr; + u_char bytes[4096]; + } response; struct sockaddr_nl addr; - chunk_t result = chunk_empty, tmp; - struct nlmsghdr *msg, peek; + chunk_t result = chunk_empty; + int len; this->mutex->lock(this->mutex); @@ -82,11 +85,9 @@ METHOD(netlink_socket_t, netlink_send, status_t, if (this->protocol == NETLINK_XFRM) { - chunk_t in_chunk = { (u_char*)in, in->nlmsg_len }; - - DBG3(DBG_KNL, "sending %N: %B", xfrm_msg_names, in->nlmsg_type, &in_chunk); + DBG3(DBG_KNL, "sending %N: %b", + xfrm_msg_names, in->nlmsg_type, in, in->nlmsg_len); } - while (TRUE) { len = sendto(this->socket, in, in->nlmsg_len, 0, @@ -108,13 +109,7 @@ METHOD(netlink_socket_t, netlink_send, status_t, while (TRUE) { - char buf[4096]; - tmp.len = sizeof(buf); - tmp.ptr = buf; - msg = (struct nlmsghdr*)tmp.ptr; - - len = recv(this->socket, tmp.ptr, tmp.len, 0); - + len = recv(this->socket, &response, sizeof(response), 0); if (len < 0) { if (errno == EINTR) @@ -128,17 +123,17 @@ METHOD(netlink_socket_t, netlink_send, status_t, free(result.ptr); return FAILED; } - if (!NLMSG_OK(msg, len)) + if (!NLMSG_OK(&response.hdr, len)) { DBG1(DBG_KNL, "received corrupted netlink message"); this->mutex->unlock(this->mutex); free(result.ptr); return FAILED; } - if (msg->nlmsg_seq != this->seq) + if (response.hdr.nlmsg_seq != this->seq) { DBG1(DBG_KNL, "received invalid netlink sequence number"); - if (msg->nlmsg_seq < this->seq) + if (response.hdr.nlmsg_seq < this->seq) { continue; } @@ -147,16 +142,13 @@ METHOD(netlink_socket_t, netlink_send, status_t, return FAILED; } - tmp.len = len; - result.ptr = realloc(result.ptr, result.len + tmp.len); - memcpy(result.ptr + result.len, tmp.ptr, tmp.len); - result.len += tmp.len; + result = chunk_cat("mc", result, chunk_create(response.bytes, len)); /* NLM_F_MULTI flag does not seem to be set correctly, we use sequence * numbers to detect multi header messages */ - len = recv(this->socket, &peek, sizeof(peek), MSG_PEEK | MSG_DONTWAIT); - - if (len == sizeof(peek) && peek.nlmsg_seq == this->seq) + len = recv(this->socket, &response.hdr, sizeof(response.hdr), + MSG_PEEK | MSG_DONTWAIT); + if (len == sizeof(response.hdr) && response.hdr.nlmsg_seq == this->seq) { /* seems to be multipart */ continue; From 5957e76b381179d45ba1094ce5085db42cbad5ac Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Wed, 9 Jul 2014 16:47:06 +0200 Subject: [PATCH 5/8] kernel-netlink: Clean up socket initialization, handle 0 as valid socket fd --- .../kernel_netlink/kernel_netlink_shared.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c index 12df7c5ad..74466e291 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c @@ -219,7 +219,7 @@ METHOD(netlink_socket_t, netlink_send_ack, status_t, METHOD(netlink_socket_t, destroy, void, private_netlink_socket_t *this) { - if (this->socket > 0) + if (this->socket != -1) { close(this->socket); } @@ -233,7 +233,9 @@ METHOD(netlink_socket_t, destroy, void, netlink_socket_t *netlink_socket_create(int protocol) { private_netlink_socket_t *this; - struct sockaddr_nl addr; + struct sockaddr_nl addr = { + .nl_family = AF_NETLINK, + }; INIT(this, .public = { @@ -243,21 +245,16 @@ netlink_socket_t *netlink_socket_create(int protocol) }, .seq = 200, .mutex = mutex_create(MUTEX_TYPE_DEFAULT), + .socket = socket(AF_NETLINK, SOCK_RAW, protocol), .protocol = protocol, ); - memset(&addr, 0, sizeof(addr)); - addr.nl_family = AF_NETLINK; - - this->socket = socket(AF_NETLINK, SOCK_RAW, protocol); - if (this->socket < 0) + if (this->socket == -1) { DBG1(DBG_KNL, "unable to create netlink socket"); destroy(this); return NULL; } - - addr.nl_groups = 0; if (bind(this->socket, (struct sockaddr*)&addr, sizeof(addr))) { DBG1(DBG_KNL, "unable to bind netlink socket"); From c94e93a7ab2f07ee7197fd33d4f0e20f2d7d9671 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 10 Jul 2014 14:21:20 +0200 Subject: [PATCH 6/8] kernel-netlink: Pass protocol specific enum names to socket constructor This avoid the hard dependency on enum names, and makes kernel_netlink_shared independent of kernel_netlink_ipsec. --- .../kernel_netlink/kernel_netlink_ipsec.c | 2 +- .../kernel_netlink/kernel_netlink_net.c | 2 +- .../kernel_netlink/kernel_netlink_shared.c | 18 +++++++++--------- .../kernel_netlink/kernel_netlink_shared.h | 5 +++-- 4 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c index cd94168c4..dfd71f3bd 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_ipsec.c @@ -2711,7 +2711,7 @@ kernel_netlink_ipsec_t *kernel_netlink_ipsec_create() fclose(f); } - this->socket_xfrm = netlink_socket_create(NETLINK_XFRM); + this->socket_xfrm = netlink_socket_create(NETLINK_XFRM, xfrm_msg_names); if (!this->socket_xfrm) { destroy(this); diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c index 3d5538c35..e8d146c91 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c @@ -2469,7 +2469,7 @@ kernel_netlink_net_t *kernel_netlink_net_create() .destroy = _destroy, }, }, - .socket = netlink_socket_create(NETLINK_ROUTE), + .socket = netlink_socket_create(NETLINK_ROUTE, NULL), .rt_exclude = linked_list_create(), .routes = hashtable_create((hashtable_hash_t)route_entry_hash, (hashtable_equals_t)route_entry_equals, 16), diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c index 74466e291..b4cece720 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.c @@ -45,15 +45,15 @@ struct private_netlink_socket_t { */ int seq; - /** - * netlink socket protocol - */ - int protocol; - /** * netlink socket */ int socket; + + /** + * Enum names for Netlink messages + */ + enum_name_t *names; }; /** @@ -83,10 +83,10 @@ METHOD(netlink_socket_t, netlink_send, status_t, addr.nl_pid = 0; addr.nl_groups = 0; - if (this->protocol == NETLINK_XFRM) + if (this->names) { DBG3(DBG_KNL, "sending %N: %b", - xfrm_msg_names, in->nlmsg_type, in, in->nlmsg_len); + this->names, in->nlmsg_type, in, in->nlmsg_len); } while (TRUE) { @@ -230,7 +230,7 @@ METHOD(netlink_socket_t, destroy, void, /** * Described in header. */ -netlink_socket_t *netlink_socket_create(int protocol) +netlink_socket_t *netlink_socket_create(int protocol, enum_name_t *names) { private_netlink_socket_t *this; struct sockaddr_nl addr = { @@ -246,7 +246,7 @@ netlink_socket_t *netlink_socket_create(int protocol) .seq = 200, .mutex = mutex_create(MUTEX_TYPE_DEFAULT), .socket = socket(AF_NETLINK, SOCK_RAW, protocol), - .protocol = protocol, + .names = names, ); if (this->socket == -1) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h index 1a03a89a8..069f746d1 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_shared.h @@ -64,9 +64,10 @@ struct netlink_socket_t { /** * Create a netlink_socket_t object. * - * @param protocol protocol type (e.g. NETLINK_XFRM or NETLINK_ROUTE) + * @param protocol protocol type (e.g. NETLINK_XFRM or NETLINK_ROUTE) + * @param names optional enum names for Netlink messages */ -netlink_socket_t *netlink_socket_create(int protocol); +netlink_socket_t *netlink_socket_create(int protocol, enum_name_t *names); /** * Creates an rtattr and adds it to the given netlink message. From 12c0bde6891b5ed21807116d0ab7e8d07e0c5290 Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 10 Jul 2014 18:01:21 +0200 Subject: [PATCH 7/8] kernel-netlink: Define and use rtnetlink message types --- .../kernel_netlink/kernel_netlink_net.c | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c index e8d146c91..9d9f15974 100644 --- a/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c +++ b/src/libhydra/plugins/kernel_netlink/kernel_netlink_net.c @@ -78,6 +78,27 @@ #define ROUTING_TABLE_PRIO 0 #endif +ENUM(rt_msg_names, RTM_NEWLINK, RTM_GETRULE, + "RTM_NEWLINK", + "RTM_DELLINK", + "RTM_GETLINK", + "RTM_SETLINK", + "RTM_NEWADDR", + "RTM_DELADDR", + "RTM_GETADDR", + "31", + "RTM_NEWROUTE", + "RTM_DELROUTE", + "RTM_GETROUTE", + "35", + "RTM_NEWNEIGH", + "RTM_DELNEIGH", + "RTM_GETNEIGH", + "RTM_NEWRULE", + "RTM_DELRULE", + "RTM_GETRULE", +); + typedef struct addr_entry_t addr_entry_t; /** @@ -2469,7 +2490,7 @@ kernel_netlink_net_t *kernel_netlink_net_create() .destroy = _destroy, }, }, - .socket = netlink_socket_create(NETLINK_ROUTE, NULL), + .socket = netlink_socket_create(NETLINK_ROUTE, rt_msg_names), .rt_exclude = linked_list_create(), .routes = hashtable_create((hashtable_hash_t)route_entry_hash, (hashtable_equals_t)route_entry_equals, 16), From 5ea627bdfad4026ca6cc4fe87f6f1959b8b057fe Mon Sep 17 00:00:00 2001 From: Martin Willi Date: Thu, 10 Jul 2014 16:27:18 +0200 Subject: [PATCH 8/8] watcher: Add a method to query the watcher state This allows a user to check if the watcher is actually running, and potentially perform read operations directly instead of relying on watcher. --- src/libstrongswan/processing/watcher.c | 32 ++++++++++++++++++++------ src/libstrongswan/processing/watcher.h | 20 ++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/src/libstrongswan/processing/watcher.c b/src/libstrongswan/processing/watcher.c index 3518dfdae..d4de2a907 100644 --- a/src/libstrongswan/processing/watcher.c +++ b/src/libstrongswan/processing/watcher.c @@ -52,9 +52,9 @@ struct private_watcher_t { bool pending; /** - * Is watcher running? + * Running state of watcher */ - bool running; + watcher_state_t state; /** * Lock to access FD list @@ -239,7 +239,7 @@ static void activate_all(private_watcher_t *this) entry->in_callback = 0; } enumerator->destroy(enumerator); - this->running = FALSE; + this->state = WATCHER_STOPPED; this->condvar->broadcast(this->condvar); this->mutex->unlock(this->mutex); } @@ -263,10 +263,14 @@ static job_requeue_t watch(private_watcher_t *this) if (this->fds->get_count(this->fds) == 0) { - this->running = FALSE; + this->state = WATCHER_STOPPED; this->mutex->unlock(this->mutex); return JOB_REQUEUE_NONE; } + if (this->state == WATCHER_QUEUED) + { + this->state = WATCHER_RUNNING; + } if (this->notify[0] != -1) { @@ -407,9 +411,9 @@ METHOD(watcher_t, add, void, this->mutex->lock(this->mutex); this->fds->insert_last(this->fds, entry); - if (!this->running) + if (this->state == WATCHER_STOPPED) { - this->running = TRUE; + this->state = WATCHER_QUEUED; lib->processor->queue_job(lib->processor, (job_t*)callback_job_create_with_prio((void*)watch, this, NULL, (callback_job_cancel_t)return_false, JOB_PRIO_CRITICAL)); @@ -437,7 +441,7 @@ METHOD(watcher_t, remove_, void, { if (entry->fd == fd) { - if (this->running && entry->in_callback) + if (this->state != WATCHER_STOPPED && entry->in_callback) { is_in_callback = TRUE; break; @@ -458,6 +462,18 @@ METHOD(watcher_t, remove_, void, this->mutex->unlock(this->mutex); } +METHOD(watcher_t, get_state, watcher_state_t, + private_watcher_t *this) +{ + watcher_state_t state; + + this->mutex->lock(this->mutex); + state = this->state; + this->mutex->unlock(this->mutex); + + return state; +} + METHOD(watcher_t, destroy, void, private_watcher_t *this) { @@ -535,6 +551,7 @@ watcher_t *watcher_create() .public = { .add = _add, .remove = _remove_, + .get_state = _get_state, .destroy = _destroy, }, .fds = linked_list_create(), @@ -542,6 +559,7 @@ watcher_t *watcher_create() .condvar = condvar_create(CONDVAR_TYPE_DEFAULT), .jobs = linked_list_create(), .notify = {-1, -1}, + .state = WATCHER_STOPPED, ); if (!create_notify(this)) diff --git a/src/libstrongswan/processing/watcher.h b/src/libstrongswan/processing/watcher.h index 6e158cec2..f5ecceeb7 100644 --- a/src/libstrongswan/processing/watcher.h +++ b/src/libstrongswan/processing/watcher.h @@ -23,6 +23,7 @@ typedef struct watcher_t watcher_t; typedef enum watcher_event_t watcher_event_t; +typedef enum watcher_state_t watcher_state_t; #include @@ -56,6 +57,18 @@ enum watcher_event_t { WATCHER_EXCEPT = (1<<2), }; +/** + * State the watcher currently is in + */ +enum watcher_state_t { + /** no watcher thread running or queued */ + WATCHER_STOPPED = 0, + /** a job has been queued for watching, but not yet started */ + WATCHER_QUEUED, + /** a watcher thread is active, dispatching socket events */ + WATCHER_RUNNING, +}; + /** * Watch multiple file descriptors using select(). */ @@ -85,6 +98,13 @@ struct watcher_t { */ void (*remove)(watcher_t *this, int fd); + /** + * Get the current watcher state + * + * @reutrn currently active watcher state + */ + watcher_state_t (*get_state)(watcher_t *this); + /** * Destroy a watcher_t. */