[DTrace-devel] [PATCH 3/6] usdt_parser: fix parser pipe I/O error handling
Alan Maguire
alan.maguire at oracle.com
Sun Aug 30 10:49:59 UTC 2026
On 28/08/2026 20:09, Kris Van Hees wrote:
> 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 at oracle.com>
Reviewed-by: Alan Maguire <alan.maguire at 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.52.0
>
>
More information about the DTrace-devel
mailing list