From 9f324d8ac27d18a81dd1bb703b5f682857bfc37d Mon Sep 17 00:00:00 2001 From: "Alexander V. Chernikov" Date: Thu, 13 Apr 2023 11:53:49 +0000 Subject: [PATCH] netlink: make netlink work correctly on CHERI. Current Netlink message writer code relies on executing callbacks with arbitrary data (pointer or integer) to flush the completed messages. This arbitrary data is stored as a union of { void *, uint64_t }. At some stage, the message flushing code copied this data, using direct uint64_t assignment instead of copying the union. It lead to failure on CHERI, as sizeof(pointer) == 16 there. Fix the code by making union non-anonymous and copying it entirely. Reviewed by: br, jhb, jrtc27 Differential Revision: https://reviews.freebsd.org/D39557 MFC after: 2 weeks --- sys/netlink/netlink_message_writer.c | 39 +++++++++++++++------------- sys/netlink/netlink_message_writer.h | 9 ++++--- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/sys/netlink/netlink_message_writer.c b/sys/netlink/netlink_message_writer.c index 884295939ce5..ef568ecbca07 100644 --- a/sys/netlink/netlink_message_writer.c +++ b/sys/netlink/netlink_message_writer.c @@ -116,7 +116,7 @@ nlmsg_get_ns_buf(struct nl_writer *nw, int size, bool waitok) static bool nlmsg_write_socket_buf(struct nl_writer *nw, void *buf, int datalen, int cnt) { - NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw); + NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg.ptr); if (__predict_false(datalen == 0)) { free(buf, M_NETLINK); return (true); @@ -132,13 +132,14 @@ nlmsg_write_socket_buf(struct nl_writer *nw, void *buf, int datalen, int cnt) free(buf, M_NETLINK); int io_flags = (nw->ignore_limit) ? NL_IOF_IGNORE_LIMIT : 0; - return (nl_send_one(m, (struct nlpcb *)(nw->arg_ptr), cnt, io_flags)); + return (nl_send_one(m, (struct nlpcb *)(nw->arg.ptr), cnt, io_flags)); } static bool nlmsg_write_group_buf(struct nl_writer *nw, void *buf, int datalen, int cnt) { - NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg_ptr); + NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d proto: %d id: %d", buf, datalen, + nw->arg.group.proto, nw->arg.group.id); if (__predict_false(datalen == 0)) { free(buf, M_NETLINK); return (true); @@ -155,15 +156,15 @@ nlmsg_write_group_buf(struct nl_writer *nw, void *buf, int datalen, int cnt) if (!success) return (false); - nl_send_group(m, cnt, nw->arg_uint >> 16, nw->arg_uint & 0xFFFF); + nl_send_group(m, cnt, nw->arg.group.proto, nw->arg.group.id); return (true); } static bool nlmsg_write_chain_buf(struct nl_writer *nw, void *buf, int datalen, int cnt) { - struct mbuf **m0 = (struct mbuf **)(nw->arg_ptr); - NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg_ptr); + struct mbuf **m0 = (struct mbuf **)(nw->arg.ptr); + NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg.ptr); if (__predict_false(datalen == 0)) { free(buf, M_NETLINK); @@ -227,7 +228,7 @@ static bool nlmsg_write_socket_mbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) { struct mbuf *m = (struct mbuf *)buf; - NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg_ptr); + NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg.ptr); if (__predict_false(datalen == 0)) { m_freem(m); @@ -237,14 +238,15 @@ nlmsg_write_socket_mbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) m->m_pkthdr.len = datalen; m->m_len = datalen; int io_flags = (nw->ignore_limit) ? NL_IOF_IGNORE_LIMIT : 0; - return (nl_send_one(m, (struct nlpcb *)(nw->arg_ptr), cnt, io_flags)); + return (nl_send_one(m, (struct nlpcb *)(nw->arg.ptr), cnt, io_flags)); } static bool nlmsg_write_group_mbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) { struct mbuf *m = (struct mbuf *)buf; - NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg_ptr); + NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d proto: %d id: %d", buf, datalen, + nw->arg.group.proto, nw->arg.group.id); if (__predict_false(datalen == 0)) { m_freem(m); @@ -253,7 +255,7 @@ nlmsg_write_group_mbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) m->m_pkthdr.len = datalen; m->m_len = datalen; - nl_send_group(m, cnt, nw->arg_uint >> 16, nw->arg_uint & 0xFFFF); + nl_send_group(m, cnt, nw->arg.group.proto, nw->arg.group.id); return (true); } @@ -261,9 +263,9 @@ static bool nlmsg_write_chain_mbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) { struct mbuf *m_new = (struct mbuf *)buf; - struct mbuf **m0 = (struct mbuf **)(nw->arg_ptr); + struct mbuf **m0 = (struct mbuf **)(nw->arg.ptr); - NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg_ptr); + NL_LOG(LOG_DEBUG2, "IN: ptr: %p len: %d arg: %p", buf, datalen, nw->arg.ptr); if (__predict_false(datalen == 0)) { m_freem(m_new); @@ -324,7 +326,7 @@ nlmsg_write_socket_lbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) { struct linear_buffer *lb = (struct linear_buffer *)buf; char *data = (char *)(lb + 1); - struct nlpcb *nlp = (struct nlpcb *)(nw->arg_ptr); + struct nlpcb *nlp = (struct nlpcb *)(nw->arg.ptr); if (__predict_false(datalen == 0)) { free(buf, M_NETLINK); @@ -365,7 +367,7 @@ nlmsg_write_group_lbuf(struct nl_writer *nw, void *buf, int datalen, int cnt) m_append(m, datalen, data); free(buf, M_NETLINK); - nl_send_group(m, cnt, nw->arg_uint >> 16, nw->arg_uint & 0xFFFF); + nl_send_group(m, cnt, nw->arg.group.proto, nw->arg.group.id); return (true); } @@ -440,7 +442,7 @@ _nlmsg_get_unicast_writer(struct nl_writer *nw, int size, struct nlpcb *nlp) { if (!nlmsg_get_buf(nw, size, false, nlp->nl_linux)) return (false); - nw->arg_ptr = (void *)nlp; + nw->arg.ptr = (void *)nlp; nw->writer_target = NS_WRITER_TARGET_SOCKET; nlmsg_set_callback(nw); return (true); @@ -451,7 +453,8 @@ _nlmsg_get_group_writer(struct nl_writer *nw, int size, int protocol, int group_ { if (!nlmsg_get_buf(nw, size, false, false)) return (false); - nw->arg_uint = (uint64_t)protocol << 16 | (uint64_t)group_id; + nw->arg.group.proto = protocol; + nw->arg.group.id = group_id; nw->writer_target = NS_WRITER_TARGET_GROUP; nlmsg_set_callback(nw); return (true); @@ -463,7 +466,7 @@ _nlmsg_get_chain_writer(struct nl_writer *nw, int size, struct mbuf **pm) if (!nlmsg_get_buf(nw, size, false, false)) return (false); *pm = NULL; - nw->arg_ptr = (void *)pm; + nw->arg.ptr = (void *)pm; nw->writer_target = NS_WRITER_TARGET_CHAIN; nlmsg_set_callback(nw); NL_LOG(LOG_DEBUG3, "setup cb %p (need %p)", nw->cb, &nlmsg_write_chain_mbuf); @@ -542,7 +545,7 @@ _nlmsg_refill_buffer(struct nl_writer *nw, int required_len) /* Update callback data */ ns_new.writer_target = nw->writer_target; nlmsg_set_callback(&ns_new); - ns_new.arg_uint = nw->arg_uint; + ns_new.arg = nw->arg; /* Copy last (unfinished) header to the new storage */ int last_len = nw->offset - completed_len; diff --git a/sys/netlink/netlink_message_writer.h b/sys/netlink/netlink_message_writer.h index 91dbc9bb1312..57fc1bf342ea 100644 --- a/sys/netlink/netlink_message_writer.h +++ b/sys/netlink/netlink_message_writer.h @@ -49,9 +49,12 @@ struct nl_writer { void *_storage; /* Underlying storage pointer */ nl_writer_cb *cb; /* Callback to flush data */ union { - void *arg_ptr; /* Callback argument as pointer */ - uint64_t arg_uint; /* Callback argument as int */ - }; + void *ptr; + struct { + uint16_t proto; + uint16_t id; + } group; + } arg; int num_messages; /* Number of messages in the buffer */ int malloc_flag; /* M_WAITOK or M_NOWAIT */ uint8_t writer_type; /* NS_WRITER_TYPE_* */