[PATCH 3/6] usdt_parser: fix parser pipe I/O error handling

[PATCH 3/6] usdt_parser: fix parser pipe I/O error handling

[Kris Van Hees]

Use ssize_t for read() and write() return values, return negative errno
from usdt_parser_write_one() to match its callers, and cap the reply
size advertised by the jailed parser before allocating host memory.

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
 dtprobed/dtprobed.c          |  8 +++-
 libcommon/usdt_parser.c      |  2 +-
 libcommon/usdt_parser.h      | 13 +++++-
 libcommon/usdt_parser_host.c | 84 +++++++++++++++++++++++++++++++++---
 4 files changed, 95 insertions(+), 12 deletions(-)

diff --git a/dtprobed/dtprobed.c b/dtprobed/dtprobed.c
index 81b28e6d..9ce941ee 100644
--- a/dtprobed/dtprobed.c
+++ b/dtprobed/dtprobed.c
@@ -935,10 +935,14 @@ process_dof(pid_t pid, int out, int in, dev_t dev, ino_t inum, dev_t exec_dev,
 	dt_list_t accum = {0};
 
 	do {
+		int parser_err;
+
 		errmsg = "DOF parser write failed";
-		while ((errno = usdt_parser_host_write(out, dh, data)) == EAGAIN);
-		if (errno != 0)
+		while ((parser_err = usdt_parser_host_write(out, dh, data)) == -EAGAIN);
+		if (parser_err != 0) {
+			errno = parser_err < 0 ? -parser_err : parser_err;
 			goto err;
+		}
 
 		/*
 		 * Wait for parsed reply.  If it fails, try once more; possibly
diff --git a/libcommon/usdt_parser.c b/libcommon/usdt_parser.c
index 1dc2fffb..18808b9a 100644
--- a/libcommon/usdt_parser.c
+++ b/libcommon/usdt_parser.c
@@ -71,7 +71,7 @@ usdt_copyin(int in, char *buf_, size_t sz)
 	memset(buf, 0, sz);
 
 	for (i = 0; i < sz; ) {
-		size_t ret;
+		ssize_t ret;
 
 		ret = read(in, buf + i, sz - i);
 
diff --git a/libcommon/usdt_parser.h b/libcommon/usdt_parser.h
index 99fc3033..a8298cbf 100644
--- a/libcommon/usdt_parser.h
+++ b/libcommon/usdt_parser.h
@@ -178,6 +178,15 @@ typedef struct dof_parsed {
 #define DIT_ARGS_NATIVE_HEADSZ	offsetof(dof_parsed_t, nargs.args)
 #define DIT_ARGS_XLAT_HEADSZ	offsetof(dof_parsed_t, xargs.args)
 #define DIT_ARGS_MAP_HEADSZ	offsetof(dof_parsed_t, argmap.argmap)
+#define DIT_BASE_HEADSZ		offsetof(dof_parsed_t, provider)
+#define DIT_EOF_HEADSZ		DIT_BASE_HEADSZ
+
+/*
+ * This is the maximum message size for dof_parsed_t (header + extra data).
+ * Parser replies are derived from one parser input block, which is already
+ * capped at DOF_MAXSZ.
+ */
+#define DIT_MAX_SIZE		DOF_MAXSZ
 
 /*
  * Host-side: in usdt_parser_host.c.
@@ -187,7 +196,7 @@ typedef struct dof_parsed {
 /*
  * Write the USDT definitions data to the parser pipe OUT.
  *
- * Returns 0 on success or a positive errno value on error.
+ * Returns 0 on success or a negative errno value on error.
  */
 int usdt_parser_host_write(int out, const dof_helper_t *dh,
 			   const usdt_data_t *data);
@@ -250,7 +259,7 @@ int usdt_parse_notes(int out, dof_helper_t *dhp, usdt_data_t *data);
 /*
  * Write something to the parser pipe OUT.
  *
- * Returns 0 on success or a positive errno value on error.
+ * Returns 0 on success or a negative errno value on error.
  */
 int usdt_parser_write_one(int out, const void *buf, size_t size);
 
diff --git a/libcommon/usdt_parser_host.c b/libcommon/usdt_parser_host.c
index 80dcf10f..939c66e7 100644
--- a/libcommon/usdt_parser_host.c
+++ b/libcommon/usdt_parser_host.c
@@ -18,7 +18,7 @@
 /*
  * Write BUF to the parser pipe OUT.
  *
- * Returns 0 on success or a positive errno value on error.
+ * Returns 0 on success or a negative errno value on error.
  */
 int
 usdt_parser_write_one(int out, const void *buf_, size_t size)
@@ -27,7 +27,7 @@ usdt_parser_write_one(int out, const void *buf_, size_t size)
 	char *buf = (char *) buf_;
 
 	for (i = 0; i < size; ) {
-		size_t ret;
+		ssize_t ret;
 
 		ret = write(out, buf + i, size - i);
 		if (ret < 0) {
@@ -35,7 +35,7 @@ usdt_parser_write_one(int out, const void *buf_, size_t size)
 			case EINTR:
 				continue;
 			default:
-				return errno;
+				return -errno;
 			}
 		}
 
@@ -45,10 +45,72 @@ usdt_parser_write_one(int out, const void *buf_, size_t size)
 	return 0;
 }
 
+static int
+usdt_parser_validate_reply(dof_parsed_t *reply)
+{
+	size_t	min_size;
+	size_t	payload_size;
+	const char *payload;
+
+	switch (reply->type) {
+	case DIT_PROVIDER:
+		min_size = DIT_PROVIDER_HEADSZ + 1;
+		payload = reply->provider.name;
+		break;
+	case DIT_PROBE:
+		min_size = DIT_PROBE_HEADSZ + 1;
+		payload = reply->probe.name;
+		break;
+	case DIT_TRACEPOINT:
+		min_size = DIT_TRACEPOINT_HEADSZ + 1;
+		payload = reply->tracepoint.args;
+		break;
+	case DIT_ERR:
+		min_size = DIT_ERR_HEADSZ + 1;
+		payload = reply->err.err;
+		break;
+	case DIT_ARGS_NATIVE:
+		min_size = DIT_ARGS_NATIVE_HEADSZ + 1;
+		payload = reply->nargs.args;
+		break;
+	case DIT_ARGS_XLAT:
+		min_size = DIT_ARGS_XLAT_HEADSZ + 1;
+		payload = reply->xargs.args;
+		break;
+	case DIT_ARGS_MAP:
+		min_size = DIT_ARGS_MAP_HEADSZ + 1;
+		payload = NULL;
+		break;
+	case DIT_EOF:
+		min_size = DIT_EOF_HEADSZ;
+		payload = NULL;
+		break;
+	default:
+		errno = EPROTO;
+		return -1;
+	}
+
+	if (reply->size < min_size) {
+		errno = EPROTO;
+		return -1;
+	}
+
+	if (payload == NULL)
+		return 0;
+
+	payload_size = reply->size - (payload - (const char *)reply);
+	if (memchr(payload, '\0', payload_size) == NULL) {
+		errno = EPROTO;
+		return -1;
+	}
+
+	return 0;
+}
+
 /*
  * Write the DOF to the parser pipe OUT.
  *
- * Returns 0 on success or a positive errno value on error.
+ * Returns 0 on success or a negative errno value on error.
  */
 int
 usdt_parser_host_write(int out, const dof_helper_t *dh, const usdt_data_t *data)
@@ -118,7 +180,7 @@ usdt_parser_host_read(int in, int timeout)
 	 * longer than expected is better than no read at all.
 	 */
 	for (i = 0, sz = offsetof(dof_parsed_t, type); i < sz;) {
-		size_t ret;
+		ssize_t ret;
 		struct timespec start, end;
 		int no_adjustment = 0;
 		long timeout_msec = timeout * MILLISEC;
@@ -151,10 +213,16 @@ usdt_parser_host_read(int in, int timeout)
 		/*
 		 * Fix up the size once it's received.  Might be large enough
 		 * that we've done the initial size read...
+		 * The size is bound to avoid memory starvation.
 		 */
 		if (i < offsetof(dof_parsed_t, type) &&
-		    i + ret >= offsetof(dof_parsed_t, type))
+		    i + ret >= offsetof(dof_parsed_t, type)) {
 			sz = reply->size;
+			if (sz < DIT_BASE_HEADSZ || sz > DIT_MAX_SIZE) {
+				errno = EPROTO;
+				goto err;
+			}
+		}
 
 		/* Allocate more room if needed for the reply.  */
 		if (sz > sizeof(dof_parsed_t)) {
@@ -171,10 +239,12 @@ usdt_parser_host_read(int in, int timeout)
 		i += ret;
 	}
 
+	if (usdt_parser_validate_reply(reply) < 0)
+		goto err;
+
 	return reply;
 
 err:
 	free(reply);
 	return NULL;
 }
-
-- 
2.47.3