btrace(8): fix out of bounds read using argN in BEGIN/END.
authordv <dv@openbsd.org>
Tue, 15 Aug 2023 20:51:45 +0000 (20:51 +0000)
committerdv <dv@openbsd.org>
Tue, 15 Aug 2023 20:51:45 +0000 (20:51 +0000)
The argN builtins aren't valid in BEGIN or END actions. The fake
probe number btrace uses to facilitate other valid builtins caused
an out of bound read of an array, producing a segfault.

Change the fake probe number to 0 as it's an unsigned int and check
for that condition.

Adds asserts near other probe array indexing to catch future issues.

ok kn@

usr.sbin/btrace/btrace.c

index 2a114c3..4ee377d 100644 (file)
@@ -1,4 +1,4 @@
-/*     $OpenBSD: btrace.c,v 1.72 2023/08/13 09:52:47 mpi Exp $ */
+/*     $OpenBSD: btrace.c,v 1.73 2023/08/15 20:51:45 dv Exp $ */
 
 /*
  * Copyright (c) 2019 - 2021 Martin Pieuchot <mpi@openbsd.org>
@@ -117,6 +117,7 @@ size_t                       dt_ndtpi;      /* # of elements in the array */
 struct dtioc_arg_info  **dt_args;      /* array of probe arguments */
 
 struct dt_evt           bt_devt;       /* fake event for BEGIN/END */
+#define DTEV_PBN_BEGINEND      0       /* fake probe number for BEGIN/END */
 uint64_t                bt_filtered;   /* # of events filtered out */
 
 struct syms            *kelf, *uelf;
@@ -297,6 +298,7 @@ dtai_cache(int fd, struct dtioc_probe_info *dtpi)
                        err(1, NULL);
        }
 
+       assert(dtpi->dtpi_pbn > 0);
        if (dt_args[dtpi->dtpi_pbn - 1] != NULL)
                return;
 
@@ -325,6 +327,7 @@ dtpi_print_list(int fd)
                    dtpi->dtpi_name);
                if (strncmp(dtpi->dtpi_prov, "tracepoint", DTNAMESIZE) == 0) {
                        dtai_cache(fd, dtpi);
+                       assert(dtpi->dtpi_pbn > 0);
                        dtai = dt_args[dtpi->dtpi_pbn - 1];
                        printf("(");
                        for (j = 0; j < dtpi->dtpi_nargs; j++, dtai++) {
@@ -543,7 +546,7 @@ rules_setup(int fd)
                kelf = kelf_open(_PATH_KSYMS);
 
        /* Initialize "fake" event for BEGIN/END */
-       bt_devt.dtev_pbn = -1;
+       bt_devt.dtev_pbn = DTEV_PBN_BEGINEND;
        strlcpy(bt_devt.dtev_comm, getprogname(), sizeof(bt_devt.dtev_comm));
        bt_devt.dtev_pid = getpid();
        bt_devt.dtev_tid = getthrid();
@@ -581,7 +584,7 @@ rules_apply(int fd, struct dt_evt *dtev)
                        if (bp->bp_type != B_PT_PROBE ||
                            bp->bp_pbn != dtev->dtev_pbn)
                                continue;
-
+                       assert(dtev->dtev_pbn > 0);
                        dtai_cache(fd, &dt_dtpis[dtev->dtev_pbn - 1]);
                        rule_eval(r, dtev);
                }
@@ -781,8 +784,12 @@ builtin_arg(struct dt_evt *dtev, enum bt_argtype dat)
        const char *argtype, *fmt;
        long value;
 
-       dtai = dt_args[dtev->dtev_pbn - 1];
        argn = dat - B_AT_BI_ARG0;
+
+       if (dtev->dtev_pbn == DTEV_PBN_BEGINEND)
+               errx(1, "arg%d builtin is not valid in BEGIN or END actions",
+                   argn);
+       dtai = dt_args[dtev->dtev_pbn - 1];
        argtype = dtai[argn].dtai_argtype;
 
        if (strncmp(argtype, "int", DTNAMESIZE) == 0) {
@@ -793,7 +800,7 @@ builtin_arg(struct dt_evt *dtev, enum bt_argtype dat)
                value = dtev->dtev_args[argn];
        }
 
-       snprintf(buf, sizeof(buf), fmt, dtev->dtev_args[argn]);
+       snprintf(buf, sizeof(buf), fmt, value);
 
        return buf;
 }
@@ -1600,6 +1607,7 @@ ba2str(struct bt_arg *ba, struct dt_evt *dtev)
                str = buf;
                break;
        case B_AT_BI_PROBE:
+               assert(dtev->dtev_pbn > 0);
                dtpi = &dt_dtpis[dtev->dtev_pbn - 1];
                if (dtpi != NULL)
                        snprintf(buf, sizeof(buf), "%s:%s:%s",