On Sun, Apr 09, 2023 at 09:15:12AM +0200, Otto Moerbeek wrote: > On Sun, Apr 09, 2023 at 08:20:43AM +0200, Otto Moerbeek wrote: > > > > > Thanks! Your suggestions make a lot of sense. I'll rework the kdump > > part to make it more flexable for different purposes. > > Anothew aspect of safety is the information availble in the heap > itself. I'm pretty sure the addresses of call sites into malloc are > interesting to attackers. To prevent a program having access to those > (even if they are stored inside the malloc meta data that is not > directly accesible to a program), I'm making sure the recording only > takes place if malloc option D is used. > > This is included in a diff with the kdump changes and a few other > things below. I'm also compiling with MALLOC_STATS if !SMALL. > > usage is now: > > $ MALLOC_OPTIONS=D ktrace -tu a.out > $ kdump -u malloc > > -Otto
few comments regarding kdump below. I will try to look at malloc part too. > Index: usr.bin/kdump/kdump.1 > =================================================================== > RCS file: /home/cvs/src/usr.bin/kdump/kdump.1,v > retrieving revision 1.35 > diff -u -p -r1.35 kdump.1 > --- usr.bin/kdump/kdump.1 30 Jul 2022 07:19:30 -0000 1.35 > +++ usr.bin/kdump/kdump.1 9 Apr 2023 07:08:20 -0000 > @@ -42,6 +42,7 @@ > .Op Fl m Ar maxdata > .Op Fl p Ar pid > .Op Fl t Ar trstr > +.Op Fl u Ar word I would use the term 'label' (instead of 'word'), as it is the way it is called inside utrace(2) man page. > .Sh DESCRIPTION > .Nm > displays the kernel trace files produced with > @@ -106,6 +107,18 @@ See the > option of > .Xr ktrace 1 > for the meaning of the letters. > +.It Fl u Ar word > +Display > +.Xr utrace 2 > +records having > +.XR utrace 2 > +header > +.Ar word > +as strings with > +.Xr vis 3 > +escaping, without > +.Xr ktrace 2 > +header information. > .It Fl X > Display I/O data with hexadecimal data and printable ASCII characters > side by side. > Index: usr.bin/kdump/kdump.c > =================================================================== > RCS file: /home/cvs/src/usr.bin/kdump/kdump.c,v > retrieving revision 1.156 > diff -u -p -r1.156 kdump.c > --- usr.bin/kdump/kdump.c 17 Feb 2023 18:01:26 -0000 1.156 > +++ usr.bin/kdump/kdump.c 9 Apr 2023 07:08:20 -0000 > @@ -88,6 +88,7 @@ int needtid, tail, basecol; > char *tracefile = DEF_TRACEFILE; > struct ktr_header ktr_header; > pid_t pid_opt = -1; > +char* utracefilter; > > #define eqs(s1, s2) (strcmp((s1), (s2)) == 0) > > @@ -168,7 +169,7 @@ main(int argc, char *argv[]) > screenwidth = 80; > } > > - while ((ch = getopt(argc, argv, "f:dHlm:np:RTt:xX")) != -1) > + while ((ch = getopt(argc, argv, "f:dHlm:np:RTt:u:xX")) != -1) > switch (ch) { > case 'f': > tracefile = optarg; > @@ -212,6 +213,9 @@ main(int argc, char *argv[]) > if (trpoints < 0) > errx(1, "unknown trace point in %s", optarg); > break; > + case 'u': > + utracefilter = optarg; I think it would make sense to also force trpoints to only user data: trpoints = KTRFAC_USER; this way, if you trace others informations in the dump file, you only get the related utrace(2) entries. -t and -u should be also mutually exclusive. > + break; > case 'x': > iohex = 1; > break; > @@ -246,7 +250,7 @@ main(int argc, char *argv[]) > silent = 0; > if (pid_opt != -1 && pid_opt != ktr_header.ktr_pid) > silent = 1; > - if (silent == 0 && trpoints & (1<<ktr_header.ktr_type)) > + if (utracefilter == NULL && silent == 0 && trpoints & > (1<<ktr_header.ktr_type)) > dumpheader(&ktr_header); > ktrlen = ktr_header.ktr_len; > if (ktrlen > size) { > @@ -1254,10 +1258,18 @@ showbufc(int col, unsigned char *dp, siz > static void > showbuf(unsigned char *dp, size_t datalen) > { > - int i, j; > + size_t i, j; > int col = 0, bpl; > unsigned char c; > + char visbuf[5]; > > + if (utracefilter != NULL) { > + for (i = 0; i < datalen; i++) { > + vis(visbuf, dp[i], VIS_SAFE | VIS_OCTAL, 0); > + printf("%s", visbuf); I don't know if it would make a difference or not, but it is possible to use strvis(3) as dp is fixed size (KTR_USER_MAXLEN). so visbuf would be KTR_USER_MAXLEN*4+1 in size. but the way you did is fine too. > + } > + return; > + } > if (iohex == 1) { > putchar('\t'); > col = 8; > @@ -1280,7 +1292,7 @@ showbuf(unsigned char *dp, size_t datale > if (bpl <= 0) > bpl = 1; > for (i = 0; i < datalen; i += bpl) { > - printf(" %04x: ", i); > + printf(" %04zx: ", i); > for (j = 0; j < bpl; j++) { > if (i+j >= datalen) > printf(" "); > @@ -1413,9 +1425,13 @@ ktruser(struct ktr_user *usr, size_t len > if (len < sizeof(struct ktr_user)) > errx(1, "invalid ktr user length %zu", len); > len -= sizeof(struct ktr_user); > - printf("%.*s:", KTR_USER_MAXIDLEN, usr->ktr_id); > - printf(" %zu bytes\n", len); > - showbuf((unsigned char *)(usr + 1), len); > + if (utracefilter == NULL) { > + printf("%.*s:", KTR_USER_MAXIDLEN, usr->ktr_id); > + printf(" %zu bytes\n", len); > + showbuf((unsigned char *)(usr + 1), len); > + } > + else if (strncmp(usr->ktr_id, utracefilter, KTR_USER_MAXIDLEN) == 0) > + showbuf((unsigned char *)(usr + 1), len); > } > > static void > @@ -1473,8 +1489,8 @@ usage(void) > > extern char *__progname; > fprintf(stderr, "usage: %s " > - "[-dHlnRTXx] [-f file] [-m maxdata] [-p pid] [-t trstr]\n", > - __progname); > + "[-dHlnRTXx] [-f file] [-m maxdata] [-p pid] [-t trstr] " > + "[-u word]\n", __progname); > exit(1); > } > > > > -- Sebastien Marie