From owner-freebsd-dtrace@FreeBSD.ORG Wed Oct 16 01:50:34 2013 Return-Path: Delivered-To: freebsd-dtrace@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTP id 9E98BDCC for ; Wed, 16 Oct 2013 01:50:34 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-qe0-x22d.google.com (mail-qe0-x22d.google.com [IPv6:2607:f8b0:400d:c02::22d]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (No client certificate requested) by mx1.freebsd.org (Postfix) with ESMTPS id 5E1E0263A for ; Wed, 16 Oct 2013 01:50:34 +0000 (UTC) Received: by mail-qe0-f45.google.com with SMTP id 8so80253qea.4 for ; Tue, 15 Oct 2013 18:50:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=TTPqCLK9MwKmdbQfx2a94wOeSgw+RCsx4wnG+4BTyYA=; b=Iz3+R+X8ZxtZq5irAh4xDt7TI6X9revzp/P+OTXFbIyZ2qe3p+ODh6QXcHec5jDHZT kYs7uLKB/fBpNemHEBSFjc1I7jPHwL3nT3UZ9eMCOKjA65UTZiKhDKnSlQFQBApTx3t0 qAZJTe1fn9Z0QOrRkE1VUwdlmn6hR/5O+cTd43f0CSeptHhr0NpKM5cLEtc4X5gt7+7D oAygLdSaY+9UWyj9d9TF5r19boS/ZTvnzAD/0MOK6P1+rAat6BZ85R/JyRrnGbfknhY5 lz/dHDkTH2hDL1OkCdSYDpr9WZuCcS1VV4o65HBTlPbj9IGsjtsBKOeg8DTxkuK2hvEE Cnsg== X-Received: by 10.224.28.65 with SMTP id l1mr1126362qac.47.1381888233534; Tue, 15 Oct 2013 18:50:33 -0700 (PDT) Received: from raichu (24-212-218-13.cable.teksavvy.com. [24.212.218.13]) by mx.google.com with ESMTPSA id h6sm4365124qej.4.1969.12.31.16.00.00 (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 15 Oct 2013 18:50:32 -0700 (PDT) Sender: Mark Johnston Date: Tue, 15 Oct 2013 21:50:27 -0400 From: Mark Johnston To: Brendan Gregg Subject: Re: [dtrace-discuss] Re: pr_psargs on FreeBSD Message-ID: <20131016015027.GA81435@raichu> References: <20130915221622.GA2981@raichu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Cc: freebsd-dtrace@freebsd.org X-BeenThere: freebsd-dtrace@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "A discussion list for developers working on DTrace in FreeBSD." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 Oct 2013 01:50:34 -0000 On Mon, Sep 23, 2013 at 09:04:02AM -0700, Brendan Gregg wrote: > G'Day Mark, > > Looks good, and I can't think of a better way. Cool, thanks! > > In the DIF_SUBR_MEMSTR loop, do you want to add a tunable so that it aborts > after some large number of iterations? Otherwise a typo could put us in a > very long loop in DTrace/interrupts-disabled context. The code could be > like the following from DIF_SUBR_MSGSIZE: > > /* > * We want to prevent against denial-of-service > here, > * so we're only going to search the list for > * dtrace_msgdsize_max mblks. > */ > if (cont++ > dtrace_msgdsize_max) { > *flags |= CPU_DTRACE_ILLOP; > break; > } > > So, create dtrace_memstr_max that's set to 512 or something. I added a memstr_max sysctl and committed the change as r256571: http://svnweb.freebsd.org/base?view=revision&revision=256571 > > Having execsnoop work would be great, along with opensnoop and friends. > Hopefully they can join dtruss in /usr/bin. :) > > Brendan > > > > On Sun, Sep 15, 2013 at 3:16 PM, Mark Johnston wrote: > > > Hi! > > > > One of the problems with FreeBSD's DTrace implementation is that it > > doesn't have a good way of getting the arguments of a given process. > > This is because of the way that they're stored, which is as a single > > buffer with null-terminator between consecutive arguments. In > > particular, there is no way within DTrace to convert such a buffer to a > > single string (say by replacing all but the last terminator with > > spaces). So scripts like execsnoop will only print the first element of > > a process' argv on FreeBSD, which isn't particularly helpful for > > interpreted programs. > > > > You can get a fixed number of arguments with something like > > > > printf("%s %s %s", cupsinfo->pr_psargs, > > curpsinfo->pr_psargs + strlen(curpsinfo->pr_psargs) + 1, > > curpsinfo->pr_psargs + strlen(curpsinfo->pr_psargs) + 1 + > > strlen(curpsinfo->pr_psargs + strlen(curpsinfo->pr_psargs) + 1) + 1); > > > > but this is less than ideal. :) > > > > Apparently OS X has the same problem, but I don't have a machine to test > > with, so I'm not completely sure. > > > > It seems to me that there are two ways to solve this problem: change the > > format that FreeBSD uses to store process arguments, or add a function > > to DTrace which can convert the argument buffer to a string. I'm not too > > keen on the former approach, so here's a proposal for the latter. It'd > > be great to get some feedback on it and find out whether anyone thinks > > it's a terrible idea. :) > > > > It's kind of a strong-armed approach, but the problem's been around for a > > while and I can't think of any clever solutions - I'd love to see an > > alternative solution. > > > > My patch against FreeBSD (pasted below) adds a function called memstr() > > to DTrace. memstr() takes three arguments: addr, c, and len, and returns > > a copy of addr of length len with all null-terminators replaced by c, > > and with the last byte replaced by a null-terminator. In particular, > > memstr() always returns a string of length len - 1, unless len == 0, in > > which case we return the empty string. > > > > With this function, the translator for the pr_psargs fields becomes > > > > translator psinfo_t < struct proc *T > { > > ... > > pr_psargs = memstr(T->p_args->ar_args, ' ', T->p_args->ar_length); > > ... > > }; > > > > and execsnoop works properly. Any thoughts on this function? Have I missed > > a better solution? A patch for FreeBSD is below. > > > > Thanks, > > -Mark > > > > diff --git a/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > > b/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > > index 9c9b2a6..83c81e5 100644 > > --- a/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > > +++ b/cddl/contrib/opensolaris/lib/libdtrace/common/dt_open.c > > @@ -122,8 +122,9 @@ > > #define DT_VERS_1_8_1 DT_VERSION_NUMBER(1, 8, 1) > > #define DT_VERS_1_9 DT_VERSION_NUMBER(1, 9, 0) > > #define DT_VERS_1_9_1 DT_VERSION_NUMBER(1, 9, 1) > > -#define DT_VERS_LATEST DT_VERS_1_9_1 > > -#define DT_VERS_STRING "Sun D 1.9.1" > > +#define DT_VERS_1_9_2 DT_VERSION_NUMBER(1, 9, 2) > > +#define DT_VERS_LATEST DT_VERS_1_9_2 > > +#define DT_VERS_STRING "Sun D 1.9.2" > > > > const dt_version_t _dtrace_versions[] = { > > DT_VERS_1_0, /* D API 1.0.0 (PSARC 2001/466) Solaris 10 FCS */ > > @@ -145,6 +146,7 @@ const dt_version_t _dtrace_versions[] = { > > DT_VERS_1_8_1, /* D API 1.8.1 */ > > DT_VERS_1_9, /* D API 1.9 */ > > DT_VERS_1_9_1, /* D API 1.9.1 */ > > + DT_VERS_1_9_2, /* D API 1.9.2 */ > > 0 > > }; > > > > @@ -311,6 +313,8 @@ static const dt_ident_t _dtrace_globals[] = { > > &dt_idops_func, "void(@)" }, > > { "memref", DT_IDENT_FUNC, 0, DIF_SUBR_MEMREF, DT_ATTR_STABCMN, > > DT_VERS_1_1, > > &dt_idops_func, "uintptr_t *(void *, size_t)" }, > > +{ "memstr", DT_IDENT_FUNC, 0, DIF_SUBR_MEMSTR, DT_ATTR_STABCMN, > > DT_VERS_1_0, > > + &dt_idops_func, "string(void *, char, size_t)" }, > > { "min", DT_IDENT_AGGFUNC, 0, DTRACEAGG_MIN, DT_ATTR_STABCMN, DT_VERS_1_0, > > &dt_idops_func, "void(@)" }, > > { "mod", DT_IDENT_ACTFUNC, 0, DT_ACT_MOD, DT_ATTR_STABCMN, > > diff --git a/cddl/lib/libdtrace/psinfo.d b/cddl/lib/libdtrace/psinfo.d > > index 068e72e..b2a009a 100644 > > --- a/cddl/lib/libdtrace/psinfo.d > > +++ b/cddl/lib/libdtrace/psinfo.d > > @@ -57,7 +57,7 @@ translator psinfo_t < struct proc *T > { > > pr_gid = T->p_ucred->cr_rgid; > > pr_egid = T->p_ucred->cr_groups[0]; > > pr_addr = 0; > > - pr_psargs = stringof(T->p_args->ar_args); > > + pr_psargs = memstr(T->p_args->ar_args, ' ', T->p_args->ar_length); > > pr_arglen = T->p_args->ar_length; > > pr_jailid = T->p_ucred->cr_prison->pr_id; > > }; > > diff --git a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > > b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > > index babc42c4..6d991fb 100644 > > --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > > +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c > > @@ -4920,6 +4920,38 @@ inetout: regs[rd] = (uintptr_t)end + 1; > > break; > > } > > > > + case DIF_SUBR_MEMSTR: { > > + char *str = (char *)mstate->dtms_scratch_ptr; > > + uintptr_t mem = tupregs[0].dttk_value; > > + char c = tupregs[1].dttk_value; > > + size_t size = tupregs[2].dttk_value; > > + uint8_t n; > > + int i; > > + > > + regs[rd] = 0; > > + > > + if (size == 0) > > + break; > > + > > + if (!dtrace_canload(mem, size - 1, mstate, vstate)) > > + break; > > + > > + if (!DTRACE_INSCRATCH(mstate, size)) { > > + DTRACE_CPUFLAG_SET(CPU_DTRACE_NOSCRATCH); > > + break; > > + } > > + > > + for (i = 0; i < size - 1; i++) { > > + n = dtrace_load8(mem++); > > + str[i] = (n == 0) ? c : n; > > + } > > + str[size - 1] = 0; > > + > > + regs[rd] = (uintptr_t)str; > > + mstate->dtms_scratch_ptr += size; > > + break; > > + } > > + > > case DIF_SUBR_TYPEREF: { > > uintptr_t size = 4 * sizeof(uintptr_t); > > uintptr_t *typeref = (uintptr_t *) > > P2ROUNDUP(mstate->dtms_scratch_ptr, sizeof(uintptr_t)); > > @@ -9102,6 +9134,7 @@ dtrace_difo_validate_helper(dtrace_difo_t *dp) > > subr == DIF_SUBR_NTOHL || > > subr == DIF_SUBR_NTOHLL || > > subr == DIF_SUBR_MEMREF || > > + subr == DIF_SUBR_MEMSTR || > > subr == DIF_SUBR_TYPEREF) > > break; > > > > diff --git a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > > b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > > index 8728e30..295457c 100644 > > --- a/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > > +++ b/sys/cddl/contrib/opensolaris/uts/common/sys/dtrace.h > > @@ -311,8 +311,9 @@ typedef enum dtrace_probespec { > > #define DIF_SUBR_SX_SHARED_HELD 48 > > #define DIF_SUBR_SX_EXCLUSIVE_HELD 49 > > #define DIF_SUBR_SX_ISEXCLUSIVE 50 > > +#define DIF_SUBR_MEMSTR 51 > > > > -#define DIF_SUBR_MAX 50 /* max subroutine > > value */ > > +#define DIF_SUBR_MAX 51 /* max subroutine > > value */ > > > > typedef uint32_t dif_instr_t; > > > > _______________________________________________ > > freebsd-dtrace@freebsd.org mailing list > > https://lists.freebsd.org/mailman/listinfo/freebsd-dtrace > > To unsubscribe, send any mail to "freebsd-dtrace-unsubscribe@freebsd.org" > > > > > > -- > Brendan Gregg, Joyent http://dtrace.org/blogs/brendan > > > > ------------------------------------------- > dtrace-discuss > Archives: https://www.listbox.com/member/archive/184261/=now > RSS Feed: https://www.listbox.com/member/archive/rss/184261/24010072-2177618e > Modify Your Subscription: https://www.listbox.com/member/?member_id=24010072&id_secret=24010072-bf518af7 > Powered by Listbox: http://www.listbox.com