Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 14 Apr 2023 16:33:58 GMT
From:      "Alexander V. Chernikov" <melifaro@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 9f324d8ac27d - main - netlink: make netlink work correctly on CHERI.
Message-ID:  <202304141633.33EGXwRT038427@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by melifaro:

URL: https://cgit.FreeBSD.org/src/commit/?id=9f324d8ac27d18a81dd1bb703b5f682857bfc37d

commit 9f324d8ac27d18a81dd1bb703b5f682857bfc37d
Author:     Alexander V. Chernikov <melifaro@FreeBSD.org>
AuthorDate: 2023-04-13 11:53:49 +0000
Commit:     Alexander V. Chernikov <melifaro@FreeBSD.org>
CommitDate: 2023-04-14 16:33:43 +0000

    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_* */



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202304141633.33EGXwRT038427>