Date: Tue, 15 Oct 2013 21:50:27 -0400 From: Mark Johnston <markj@freebsd.org> To: Brendan Gregg <brendan.gregg@joyent.com> Cc: freebsd-dtrace@freebsd.org Subject: Re: [dtrace-discuss] Re: pr_psargs on FreeBSD Message-ID: <20131016015027.GA81435@raichu> In-Reply-To: <CA%2BXzFFgfcH=307mu-3EULt4nCFVvYiQDFDt1yi_3-m_dWgn0Og@mail.gmail.com> References: <20130915221622.GA2981@raichu> <CA%2BXzFFgfcH=307mu-3EULt4nCFVvYiQDFDt1yi_3-m_dWgn0Og@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <markj@freebsd.org> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20131016015027.GA81435>