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
