[DTrace-devel] [PATCH v6 03/19] Deprecate enabled probe ID (epid)

[Kris Van Hees]

On Thu, Sep 19, 2024 at 06:25:40AM -0400, Kris Van Hees wrote:
> On Thu, Sep 19, 2024 at 05:57:19AM -0400, Kris Van Hees wrote:
> > Great patch.  Found a few smallish things though that we probably want to
> > address before putting this in (potential risks for buffer overruns).
> > 
> > Also, because dt_stmts is constructed from the programs being presented for
> > loading by the consumer (which is a 2nd step after the consumer uses libdtrace
> > to compile the statements), it is technically possible that not all elements
> > in dt_stmts will be filled in.  There is no guarantee that a consumer will
> > present every compiled program for loading (though in practice that is of
> > course not going to happen).
> > 
> > But since libdtrace allows this by design, every indexing of dt_stmts (there
> > are only a few - marked below) should first make sure that the element is not
> > NULL.

Suggestion patch attached...
-------------- next part --------------
diff --git a/libdtrace/dt_handle.c b/libdtrace/dt_handle.c
index 3c118545..da70318d 100644
--- a/libdtrace/dt_handle.c
+++ b/libdtrace/dt_handle.c
@@ -127,16 +127,13 @@ dt_handle_err(dtrace_hdl_t *dtp, dtrace_probedata_t *data)
 {
 	dtrace_datadesc_t *dd = data->dtpda_ddesc, *errdd;
 	dtrace_probedesc_t *pd = data->dtpda_pdesc, *errpd;
+	dtrace_stmtdesc_t *stp;
 	dtrace_errdata_t err;
 	dtrace_id_t prid;
 	dtrace_stid_t stid;
 
-	char details[30];
-	char offinfo[30];
-	const int slop = 80;
-	const char *faultstr;
-	char *str;
-	int len;
+	char *str, *details, *offinfo;
+	int rc = 0;
 
 	assert(dd->dtdd_uarg == DT_ECB_ERROR);
 
@@ -164,17 +161,10 @@ dt_handle_err(dtrace_hdl_t *dtp, dtrace_probedata_t *data)
 	err.dteda_fault = (int)DT_REC(uint64_t, 3);
 	err.dteda_addr = DT_REC(uint64_t, 4);
 
-	faultstr = dtrace_faultstr(dtp, err.dteda_fault);
-	len = sizeof(offinfo) + strlen(faultstr) +
-	      strlen(errpd->prv) + strlen(errpd->mod) + strlen(errpd->fun) +
-	      strlen(errpd->prb) + slop;
-
-	str = (char *)alloca(len);
-
 	if (err.dteda_offset != -1)
-		sprintf(offinfo, " at BPF pc %d", err.dteda_offset);
+		asprintf(&offinfo, " at BPF pc %d", err.dteda_offset);
 	else
-		offinfo[0] = 0;
+		offinfo = "";
 
 	switch (err.dteda_fault) {
 	case DTRACEFLT_BADADDR:
@@ -183,33 +173,38 @@ dt_handle_err(dtrace_hdl_t *dtp, dtrace_probedata_t *data)
 	case DTRACEFLT_BADALIGN:
 	case DTRACEFLT_BADSTACK:
 	case DTRACEFLT_BADSIZE:
-		sprintf(details, " (0x%llx)", (unsigned long long)err.dteda_addr);
+		asprintf(&details, " (0x%llx)", (unsigned long long)err.dteda_addr);
 		break;
 	case DTRACEFLT_BADINDEX:
-		sprintf(details, " (%ld)", (int64_t)err.dteda_addr);
+		asprintf(&details, " (%ld)", (int64_t)err.dteda_addr);
 		break;
 
 	default:
 no_addr:
-		details[0] = 0;
+		details = "";
 	}
 
-	snprintf(str, len, "error in %s for probe ID %u (%s:%s:%s:%s): "
-			   "%s%s%s",
-		 dtp->dt_stmts[stid]->dtsd_clause->di_name,
-		 errpd->id, errpd->prv, errpd->mod, errpd->fun,
-		 errpd->prb, dtrace_faultstr(dtp, err.dteda_fault), details,
-		 offinfo);
+	stp = dtp->dt_stmts[stid];
+	assert(stp != NULL);
+	asprintf(&str, "error in %s for probe ID %u (%s:%s:%s:%s): %s%s%s",
+		 stp->dtsd_clause->di_name, errpd->id, errpd->prv, errpd->mod,
+		 errpd->fun, errpd->prb, dtrace_faultstr(dtp, err.dteda_fault),
+		 details, offinfo);
 
 	err.dteda_msg = str;
 
 	if (dtp->dt_errhdlr == NULL)
-		return dt_set_errno(dtp, EDT_ERRABORT);
+		rc = dt_set_errno(dtp, EDT_ERRABORT);
+	else if ((*dtp->dt_errhdlr)(&err, dtp->dt_errarg) == DTRACE_HANDLE_ABORT)
+		rc = dt_set_errno(dtp, EDT_ERRABORT);
 
-	if ((*dtp->dt_errhdlr)(&err, dtp->dt_errarg) == DTRACE_HANDLE_ABORT)
-		return dt_set_errno(dtp, EDT_ERRABORT);
+	free(str);
+	if (offinfo[0] != 0)
+		free(offinfo);
+	if (details[0] != 0)
+		free(details);
 
-	return 0;
+	return rc;
 }
 
 int
@@ -237,6 +232,7 @@ dt_handle_liberr(dtrace_hdl_t *dtp, const dtrace_probedata_t *data,
     const char *faultstr)
 {
 	dtrace_probedesc_t *errpd = data->dtpda_pdesc;
+	dtrace_stmtdesc_t *stp;
 	dtrace_errdata_t err;
 	const int slop = 80;
 	char *str;
@@ -255,10 +251,11 @@ dt_handle_liberr(dtrace_hdl_t *dtp, const dtrace_probedata_t *data,
 
 	str = alloca(len);
 
+	stp = dtp->dt_stmts[data->dtpda_stid];
+	assert(stp != NULL);
 	snprintf(str, len,
 		 "error in %s for probe ID %u (%s:%s:%s:%s): %s",
-		 dtp->dt_stmts[data->dtpda_stid]->dtsd_clause->di_name,
-		 errpd->id, errpd->prv, errpd->mod,
+		 stp->dtsd_clause->di_name, errpd->id, errpd->prv, errpd->mod,
 		 errpd->fun, errpd->prb, faultstr);
 
 	err.dteda_msg = str;
diff --git a/libdtrace/dt_map.c b/libdtrace/dt_map.c
index 50d0aaa7..9baf94d9 100644
--- a/libdtrace/dt_map.c
+++ b/libdtrace/dt_map.c
@@ -89,11 +89,14 @@ int
 dt_stid_lookup(dtrace_hdl_t *dtp, dtrace_stid_t stid, dtrace_datadesc_t **ddp)
 {
 	dtrace_difo_t *rdp;
+	dtrace_stmtdesc_t *stp;
 
 	if (stid >= dtp->dt_stmt_nextid)
 		return -1;
 
-	rdp = dt_dlib_get_func_difo(dtp, dtp->dt_stmts[stid]->dtsd_clause);
+	stp = dtp->dt_stmts[stid];
+	assert(stp != NULL);
+	rdp = dt_dlib_get_func_difo(dtp, stp->dtsd_clause);
 	*ddp = rdp->dtdo_ddesc;
 
 	return (*ddp == NULL) ? -1 : 0;