[PATCH] Fix types of arguments to dtrace syscall return probes
Paul Ambrose
ambrosehua at gmail.com
Mon Nov 7 14:16:04 UTC 2011
Thank you for your work. I will give it a try in stable/9. I have
another two PR:
kern/160307 and bin/160275 that maybe you can help;
the reason of bin/160275 is complicated, but the fix for kernel crash
caused by module without ctf section(for example, nvidia.ko) missed
somthing
(my mistake)
in kern/kern_ctf.c
--------------------------------------------------------------------------------
int link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
....
struct nameidata nd;
struct thread *td = curthread;
uint8_t ctf_hdr[CTF_HDR_SIZE];
#endif
int error = 0;
if (lf == NULL || lc == NULL)
return (EINVAL);
/* Set the defaults for no CTF present. That's not a crime! */
bzero(lc, sizeof(*lc));
#ifdef DDB_CTF
/*
* First check if we've tried to load CTF data previously and the
* CTF ELF section wasn't found. We flag that condition by setting
* ctfcnt to -1. See below.
*/
if (ef->ctfcnt < 0) //second
time with same module,
return (0);
/* Now check if we've already loaded the CTF data.. */
if (ef->ctfcnt > 0) {
/* We only need to load once. */
lc->ctftab = ef->ctftab;
lc->ctfcnt = ef->ctfcnt;
lc->symtab = ef->ddbsymtab;
lc->strtab = ef->ddbstrtab;
lc->strcnt = ef->ddbstrcnt;
lc->nsym = ef->ddbsymcnt;
lc->ctfoffp = (uint32_t **) &ef->ctfoff;
lc->typoffp = (uint32_t **) &ef->typoff;
lc->typlenp = &ef->typlen;
return (0);
}
/*
* We need to try reading the CTF data. Flag no CTF data present
* by default and if we actually succeed in reading it, we'll
* update ctfcnt to the number of bytes read.
*/
ef->ctfcnt = -1;
---------------------------------------------------------------------
fisrt time, set ctfcnt to -1, if module does not has ctf section, and
ef->ctfcnt does NOT update with a valid value(>0) later,
then second time enter this function with same module(ef), previous
if (ef->ctfcnt < 0)
return (0); // should not be 0 here
pass over first time ctf section check, I think we need another fix .
------------------------------------------------------------------------------------
diff --git a/sys/kern/kern_ctf.c b/sys/kern/kern_ctf.c
index bdff96e..2737860 100644
--- a/sys/kern/kern_ctf.c
+++ b/sys/kern/kern_ctf.c
@@ -90,7 +90,7 @@ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
* ctfcnt to -1. See below.
*/
if (ef->ctfcnt < 0)
- return (0);
+ return (EFTYPE);
/* Now check if we've already loaded the CTF data.. */
if (ef->ctfcnt > 0) {
---------------------------------------------------------------------------------------
And I post another fix for the ${NORMAL_CTFCONVERT} issue, could you
check it for me?
kern/160307, I check the /boot/kernel/kernel with ctfdump, and found
the kernel image has right ctf information, do you
have any idea?
And thank you again for your work!
--------------------------------------------------------------------------------------------------
[PATCH] Fix kernel panics when using dtrace fbt return probes on i386
[PATCH] Fix types of arguments to dtrace syscall return probes
walltimestamp curpsinfo->pr_psargs has no args from "Shawn Webb
lattera at gmail.com".
commit ec734d4fb07fec8d1b5fb8d1d4c8caa0fada4eea
Author: rstone <rstone at FreeBSD.org>
Replace fasttrap_copyout() with uwrite(). FreeBSD copyout() is not able to
write to the .text section of a process.
commit 83b04575eb8f73e557f245bb2814ac6db0a89696
Author: rstone <rstone at FreeBSD.org>
Fix the DTrace pid return trap interrupt vector. Previously we were using
31, but that vector is reserved.
2011/11/6 Ryan Stone <rysto32 at gmail.com>:
> Currently if you try to use the args[] array passed to a syscall
> return probe, you get variables with the wrong type. This is because
> the systrace implementation is currently using the same function to
> provide the same argument types for both the entry and return probes,
> which is completely wrong. For example:
>
> # dtrace -v -l -n syscall::mmap:return
> ID PROVIDER MODULE FUNCTION NAME
> 32159 syscall mmap return
>
> Probe Description Attributes
> Identifier Names: Private
> Data Semantics: Private
> Dependency Class: ISA
>
> Argument Attributes
> Identifier Names: Private
> Data Semantics: Private
> Dependency Class: ISA
>
> Argument Types
> args[0]: caddr_t
> args[1]: size_t
> args[2]: int
> args[3]: int
> args[4]: int
> args[5]: off_t
>
> The following patch[1] fixes this and provides the correct type to all
> return probes. For example,
>
> # dtrace -l -v -n syscall::mmap:return
> ID PROVIDER MODULE FUNCTION NAME
> 2000 syscall freebsd mmap return
>
> Probe Description Attributes
> Identifier Names: Private
> Data Semantics: Private
> Dependency Class: Unknown
>
> Argument Attributes
> Identifier Names: Private
> Data Semantics: Private
> Dependency Class: ISA
>
> Argument Types
> args[0]: caddr_t
> args[1]: caddr_t
>
>
> The patch:
> diff --git a/sys/cddl/dev/systrace/systrace.c b/sys/cddl/dev/systrace/systrace.c
> index cc48747..31c11c2 100644
> --- a/sys/cddl/dev/systrace/systrace.c
> +++ b/sys/cddl/dev/systrace/systrace.c
> @@ -220,8 +220,12 @@ systrace_getargdesc(void *arg, dtrace_id_t id,
> void *parg, dtrace_argdesc_t *des
> {
> int sysnum = SYSTRACE_SYSNUM((uintptr_t)parg);
>
> - systrace_setargdesc(sysnum, desc->dtargd_ndx, desc->dtargd_native,
> - sizeof(desc->dtargd_native));
> + if (SYSTRACE_ISENTRY((uintptr_t)parg))
> + systrace_entry_setargdesc(sysnum, desc->dtargd_ndx,
> + desc->dtargd_native, sizeof(desc->dtargd_native));
> + else
> + systrace_return_setargdesc(sysnum, desc->dtargd_ndx,
> + desc->dtargd_native, sizeof(desc->dtargd_native));
>
> if (desc->dtargd_native[0] == '\0')
> desc->dtargd_ndx = DTRACE_ARGNONE;
> diff --git a/sys/kern/makesyscalls.sh b/sys/kern/makesyscalls.sh
> index d1162b5..1f081ce 100644
> --- a/sys/kern/makesyscalls.sh
> +++ b/sys/kern/makesyscalls.sh
> @@ -38,6 +38,7 @@ sysinc="sysinc.switch.$$"
> sysarg="sysarg.switch.$$"
> sysprotoend="sysprotoend.$$"
> systracetmp="systrace.$$"
> +systraceret="systraceret.$$"
>
> if [ -r capabilities.conf ]; then
> capenabled=`cat capabilities.conf | grep -v "^#" | grep -v "^$"`
> @@ -46,9 +47,9 @@ else
> capenabled=""
> fi
>
> -trap "rm $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp" 0
> +trap "rm $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp $systraceret" 0
>
> -touch $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp
> +touch $sysaue $sysdcl $syscompat $syscompatdcl $syscompat4
> $syscompat4dcl $syscompat6 $syscompat6dcl $syscompat7 $syscompat7dcl
> $sysent $sysinc $sysarg $sysprotoend $systracetmp $systraceret
>
> case $# in
> 0) echo "usage: $0 input-file <config-file>" 1>&2
> @@ -96,6 +97,7 @@ s/\$//g
> sysmk = \"$sysmk\"
> systrace = \"$systrace\"
> systracetmp = \"$systracetmp\"
> + systraceret = \"$systraceret\"
> compat = \"$compat\"
> compat4 = \"$compat4\"
> compat6 = \"$compat6\"
> @@ -179,9 +181,12 @@ s/\$//g
> printf "\tint64_t *iarg = (int64_t *) uarg;\n" > systrace
> printf "\tswitch (sysnum) {\n" > systrace
>
> - printf "static void\nsystrace_setargdesc(int sysnum,
> int ndx, char *desc, size_t descsz)\n{\n\tconst char *p = NULL;\n" >
> systracetmp
> + printf "static void\nsystrace_entry_setargdesc(int
> sysnum, int ndx, char *desc, size_t descsz)\n{\n\tconst char *p =
> NULL;\n" > systracetmp
> printf "\tswitch (sysnum) {\n" > systracetmp
>
> + printf "static void\nsystrace_return_setargdesc(int
> sysnum, int ndx, char *desc, size_t descsz)\n{\n\tconst char *p =
> NULL;\n" > systraceret
> + printf "\tswitch (sysnum) {\n" > systraceret
> +
> next
> }
> NF == 0 || $1 ~ /^;/ {
> @@ -202,6 +207,7 @@ s/\$//g
> print > sysnames
> print > systrace
> print > systracetmp
> + print > systraceret
> savesyscall = syscall
> next
> }
> @@ -216,6 +222,7 @@ s/\$//g
> print > sysnames
> print > systrace
> print > systracetmp
> + print > systraceret
> syscall = savesyscall
> next
> }
> @@ -230,6 +237,7 @@ s/\$//g
> print > sysnames
> print > systrace
> print > systracetmp
> + print > systraceret
> next
> }
> syscall != $1 {
> @@ -303,7 +311,8 @@ s/\$//g
> parserr($end, ")")
> end--
>
> - f++ #function return type
> + syscallret=$f
> + f++
>
> funcname=$f
>
> @@ -387,6 +396,7 @@ s/\$//g
> parseline()
> printf("\t/* %s */\n\tcase %d: {\n", funcname,
> syscall) > systrace
> printf("\t/* %s */\n\tcase %d:\n", funcname, syscall)
>> systracetmp
> + printf("\t/* %s */\n\tcase %d:\n", funcname, syscall)
>> systraceret
> if (argc > 0) {
> printf("\t\tswitch(ndx) {\n") > systracetmp
> printf("\t\tstruct %s *p = params;\n",
> argalias) > systrace
> @@ -406,6 +416,10 @@ s/\$//g
> argname[i], argtype[i]) > systrace
> }
> printf("\t\tdefault:\n\t\t\tbreak;\n\t\t};\n")
>> systracetmp
> +
> + printf("\t\tif (ndx == 0 || ndx == 1)\n") > systraceret
> + printf("\t\t\tp = \"%s\";\n", syscallret) > systraceret
> + printf("\t\tbreak;\n") > systraceret
> }
> printf("\t\t*n_args = %d;\n\t\tbreak;\n\t}\n", argc) > systrace
> printf("\t\tbreak;\n") > systracetmp
> @@ -623,6 +637,7 @@ s/\$//g
> > syshdr
> printf "\tdefault:\n\t\t*n_args =
> 0;\n\t\tbreak;\n\t};\n}\n" > systrace
> printf "\tdefault:\n\t\tbreak;\n\t};\n\tif (p !=
> NULL)\n\t\tstrlcpy(desc, p, descsz);\n}\n" > systracetmp
> + printf "\tdefault:\n\t\tbreak;\n\t};\n\tif (p !=
> NULL)\n\t\tstrlcpy(desc, p, descsz);\n}\n" > systraceret
> } '
>
> cat $sysinc $sysent >> $syssw
> @@ -633,4 +648,5 @@ cat $sysarg $sysdcl \
> $syscompat7 $syscompat7dcl \
> $sysaue $sysprotoend > $sysproto
> cat $systracetmp >> $systrace
> +cat $systraceret >> $systrace
>
>
> [1] Can also be found at
> http://people.freebsd.org/~rstone/patches/dtrace_syscall_ret.diff
> _______________________________________________
> freebsd-current at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe at freebsd.org"
>
More information about the freebsd-current
mailing list