netlink: Make room for the unfinished message when growing the buffer

When a message outgrows the writer's buffer, _nlmsg_refill_buffer()
allocates a larger one and moves the unfinished message there.  The new
buffer was sized for the data about to be added alone, so with enough of
the message already written the caller wrote past its end and left it
with datalen > buflen.  A 20-byte message plus an attribute of 8176
bytes got a buffer of 8192 bytes for 8200 bytes of data, and reading it
panicked an INVARIANTS kernel in nl_soreceive().

Size the new buffer for both, and add a ktest for it.

Reviewed by:		glebius
Fixes:			7e5bf68495 ("netlink: add netlink support")
MFC after:		1 week
Sponsored by:		Rubicon Communications, LLC ("Netgate")
Differential Revision:	https://reviews.freebsd.org/D60541

sys/netlink/ktest_netlink_message_writer.c

@@ -97,6 +97,97 @@ test_nlbuf_writer_allocation(struct ktest_test_context *ctx)
 	return (0);
 }
 
+struct test_nlbuf_refill_attrs {
+	uint32_t	size;
+	uint32_t	completed;
+	uint32_t	unfinished;
+	uint32_t	attr_len;
+};
+
+#define	_OUT(_field)	offsetof(struct test_nlbuf_refill_attrs, _field)
+static const struct nlattr_parser nla_p_nlbuf_refill[] = {
+	{ .type = 1, .off = _OUT(size), .cb = nlattr_get_uint32 },
+	{ .type = 2, .off = _OUT(completed), .cb = nlattr_get_uint32 },
+	{ .type = 3, .off = _OUT(unfinished), .cb = nlattr_get_uint32 },
+	{ .type = 4, .off = _OUT(attr_len), .cb = nlattr_get_uint32 },
+};
+#undef _OUT
+NL_DECLARE_ATTR_PARSER(nlbuf_refill_parser, nla_p_nlbuf_refill);
+
+static int
+test_nlbuf_refill_parser(struct ktest_test_context *ctx, struct nlattr *nla)
+{
+	struct test_nlbuf_refill_attrs *attrs;
+
+	attrs = npt_alloc(ctx->npt, sizeof(*attrs));
+	ctx->arg = attrs;
+	if (attrs != NULL)
+		return (nl_parse_nested(nla, &nlbuf_refill_parser, ctx->npt,
+		    attrs));
+	return (ENOMEM);
+}
+
+static bool
+test_nlbuf_discard(struct nl_writer *nw)
+{
+	nl_buf_free(nw->buf);
+	nw->buf = NULL;
+	return (true);
+}
+
+/*
+ * Add an attribute that does not fit into the buffer to an unfinished
+ * message, optionally preceded by a completed one.
+ */
+static int
+test_nlbuf_writer_refill(struct ktest_test_context *ctx)
+{
+	struct test_nlbuf_refill_attrs *attrs = ctx->arg;
+	struct nl_writer nw = {};
+	void *data;
+	u_int expected;
+	int error = 0;
+
+	if (attrs == NULL || attrs->unfinished < sizeof(struct nlmsghdr) ||
+	    (attrs->completed != 0 &&
+	    attrs->completed < sizeof(struct nlmsghdr)))
+		return (EINVAL);
+	if (!nlmsg_get_buf_wrapper(&nw, attrs->size, true))
+		return (EINVAL);
+	nw.cb = test_nlbuf_discard;
+	data = malloc(attrs->attr_len, M_TEMP, M_WAITOK | M_ZERO);
+
+	if (attrs->completed != 0) {
+		if (!nlmsg_add(&nw, 0, 0, NLMSG_NOOP, 0, 0) ||
+		    nlmsg_reserve_data(&nw, attrs->completed -
+		    sizeof(struct nlmsghdr), void *) == NULL ||
+		    !nlmsg_end(&nw)) {
+			error = EINVAL;
+			goto out;
+		}
+	}
+	if (!nlmsg_add(&nw, 0, 0, NLMSG_NOOP, 0, 0) ||
+	    nlmsg_reserve_data(&nw, attrs->unfinished -
+	    sizeof(struct nlmsghdr), void *) == NULL ||
+	    !nlattr_add(&nw, 1, attrs->attr_len, data)) {
+		error = EINVAL;
+		goto out;
+	}
+
+	expected = NETLINK_ALIGN(attrs->unfinished) +
+	    NLA_ALIGN(attrs->attr_len + sizeof(struct nlattr));
+	KTEST_LOG(ctx, "message of %u bytes in a buffer of %u, expected %u",
+	    nw.buf->datalen, nw.buf->buflen, expected);
+	if (nw.buf->datalen > nw.buf->buflen || nw.buf->datalen != expected)
+		error = EINVAL;
+out:
+	if (nw.buf != NULL)
+		nl_buf_free(nw.buf);
+	free(data, M_TEMP);
+
+	return (error);
+}
+
 static const struct ktest_test_info tests[] = {
 	{
 		.name = "test_nlbuf_writer_allocation",
@@ -104,5 +195,11 @@ static const struct ktest_test_info tests[] = {
 		.func = &test_nlbuf_writer_allocation,
 		.parse = &test_nlbuf_parser,
 	},
+	{
+		.name = "test_nlbuf_writer_refill",
+		.desc = "test growing the buffer under an unfinished message",
+		.func = &test_nlbuf_writer_refill,
+		.parse = &test_nlbuf_refill_parser,
+	},
 };
 KTEST_MODULE_DECLARE(ktest_netlink_message_writer, tests);

sys/netlink/netlink_message_writer.c

@@ -156,6 +156,7 @@ _nlmsg_refill_buffer(struct nl_writer *nw, size_t required_len)
 	/* Calculate new buffer size and allocate it. */
 	completed_len = (nw->hdr != NULL) ?
 	    (char *)nw->hdr - nw->buf->data : nw->buf->datalen;
+	last_len = nw->buf->datalen - completed_len;
 	if (completed_len > 0 && required_len < NLMBUFSIZE) {
 		/* We already ran out of space, use largest effective size. */
 		new_len = max(nw->buf->buflen, NLMBUFSIZE);
@@ -165,9 +166,10 @@ _nlmsg_refill_buffer(struct nl_writer *nw, size_t required_len)
 			new_len = NLMBUFSIZE;
 		else
 			new_len = nw->buf->buflen * 2;
-		while (new_len < required_len)
-			new_len *= 2;
 	}
+	/* The unfinished message is copied over and needs room as well. */
+	while (new_len < last_len + required_len)
+		new_len *= 2;
 
 	new = nl_buf_alloc(new_len, nw->malloc_flag | M_ZERO);
 	if (__predict_false(new == NULL)) {
@@ -177,7 +179,6 @@ _nlmsg_refill_buffer(struct nl_writer *nw, size_t required_len)
 	}
 
 	/* Copy last (unfinished) header to the new storage. */
-	last_len = nw->buf->datalen - completed_len;
 	if (last_len > 0) {
 		memcpy(new->data, nw->hdr, last_len);
 		new->datalen = last_len;

tests/sys/netlink/test_netlink_message_writer.py

@@ -37,3 +37,24 @@ class TestNetlinkMessageWriter(BaseKernelTest):
             NlAttrU32(3, malloc_flags),
         ]
         self.runtest(test_meta)
+
+    @pytest.mark.parametrize(
+        "sz",
+        [
+            # buffer size, completed message, unfinished message, attribute
+            pytest.param([NLMSG_SMALL, 0, 20, 112], id="small"),
+            pytest.param([NLMSG_SMALL, 0, 20, 8176], id="large_attr"),
+            pytest.param([NLMSG_LARGE, 0, 2000, 4092], id="large_msg"),
+            pytest.param([NLMSG_LARGE, 100, 1900, 196], id="after_completed"),
+        ],
+    )
+    def test_nlbuf_writer_refill(self, sz):
+        """override to parametrize"""
+
+        test_meta = [
+            NlAttrU32(1, sz[0]),  # size
+            NlAttrU32(2, sz[1]),  # completed
+            NlAttrU32(3, sz[2]),  # unfinished
+            NlAttrU32(4, sz[3]),  # attr_len
+        ]
+        self.runtest(test_meta)