Re: [PATCH] dtrace crashes when trying to trace fbt probes

From: Paul Ambrose <ambrosehua_at_gmail.com>
Date: Thu, 29 Sep 2011 22:04:39 +0800
sorry, I miss a check, here is the patch
.............................................................................................
diff --git a/sys/kern/kern_ctf.c b/sys/kern/kern_ctf.c
index 758ad81..6beefcc 100644
--- a/sys/kern/kern_ctf.c
+++ b/sys/kern/kern_ctf.c
_at__at_ -164,8 +164,14 _at__at_ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
      * section names aren't present, then we can't locate the
      * .SUNW_ctf section containing the CTF data.
      */
-    if (hdr->e_shstrndx == 0 || shdr[hdr->e_shstrndx].sh_type !=
SHT_STRTAB)
+    if (hdr->e_shstrndx == 0 || shdr[hdr->e_shstrndx].sh_type !=
SHT_STRTAB) {
+
+        error = EFTYPE;
+        printf("%s(%d): module %s e_shstrndx is %d, sh_type is %d\n",
__func__,
+                __LINE__, lf->pathname, hdr->e_shstrndx,
+                shdr[hdr->e_shstrndx].sh_type);
         goto out;
+    }

     /* Allocate memory to buffer the section header strings. */
     if ((shstrtab = malloc(shdr[hdr->e_shstrndx].sh_size, M_LINKER,
_at__at_ -187,8 +193,12 _at__at_ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
             break;

     /* Check if the CTF section wasn't found. */
-    if (i >= hdr->e_shnum)
+    if (i >= hdr->e_shnum) {
+        error = EFTYPE;
+        printf("%s(%d): module %s has no .SUNW_ctf section\n", __func__,
+                __LINE__, lf->pathname);
         goto out;
+    }

     /* Read the CTF header. */
     if ((error = vn_rdwr(UIO_READ, nd.ni_vp, ctf_hdr, sizeof(ctf_hdr),
_at__at_ -197,12 +207,20 _at__at_ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
         goto out;

     /* Check the CTF magic number. (XXX check for big endian!) */
-    if (ctf_hdr[0] != 0xf1 || ctf_hdr[1] != 0xcf)
+    if (ctf_hdr[0] != 0xf1 || ctf_hdr[1] != 0xcf) {
+        error = EFTYPE;
+        printf("%s(%d): module %s has wrong format\n", __func__, __LINE__,
+                lf->pathname);
         goto out;
+    }

     /* Check if version 2. */
-    if (ctf_hdr[2] != 2)
+    if (ctf_hdr[2] != 2) {
+        error = EFTYPE;
+        printf("%s(%d): module %s CTF format version is %d\n", __func__,
__LINE__,
+                lf->pathname, ctf_hdr[2]);
         goto out;
+    }

     /* Check if the data is compressed. */
     if ((ctf_hdr[3] & 0x1) != 0) {

............................................................................................................

2011/9/29 Paul Ambrose <ambrosehua_at_gmail.com>

> In 8-stable, WITH_CTF=1 configure item is  enabled in command line, not in
> make.conf, so when I build kernel module out of /usr/src source tree, such
> as  x11/nvidia-driver, I forgot to use WITH_CTF=1 and nvidia.ko was built
> without .SUNW_ctf  section.  However, when  I run:
> #dtrace -lv
>
> trigger the NULL pointer dereference at: /usr/src/sys/cddl/dev/fbt/fbt.c
>
>> ..........................................
>>     if (*lc.ctfoffp == NULL) {       // page fault here
>>
>>         /*
>>          * Initialise the CTF object and function symindx to
>>          * byte offset array.
>>          */
>>         if (fbt_ctfoff_init(ctl, &lc) != 0)
>>             return;
>>
>>         /* Initialise the CTF type to byte offset array. */
>>         if (fbt_typoff_init(&lc) != 0)
>>             return;
>>     }
>> ........................................................................
>>
> the reason is at /usr/src/sys/kern/kern_ctf.c:
>
> ................................................................................
>
> ......
>
>     /* Search for the section containing the CTF data. */
>     for (i = 0; i < hdr->e_shnum; i++)
>         if (strcmp(".SUNW_ctf", shstrtab + shdr[i].sh_name) == 0)
>             break;
>
>     /* Check if the CTF section wasn't found. */
>     if (i >= hdr->e_shnum)          //here we found no ctf data, but NOT
> update the varible "error"
>         goto out;                           // see label out
>
>     /* Read the CTF header. */
>     if ((error = vn_rdwr(UIO_READ, nd.ni_vp, ctf_hdr, sizeof(ctf_hdr),
>         shdr[i].sh_offset, UIO_SYSSPACE, IO_NODELOCKED, td->td_ucred,
>         NOCRED, &resid, td)) != 0)
>         goto out;
>
>     /* Check the CTF magic number. (XXX check for big endian!) */
>     if (ctf_hdr[0] != 0xf1 || ctf_hdr[1] != 0xcf)
>         goto out;
>
>     /* Check if version 2. */
>     if (ctf_hdr[2] != 2)
>         goto out;
>
>     /* Check if the data is compressed. */
>     if ((ctf_hdr[3] & 0x1) != 0) {
>         uint32_t *u32 = (uint32_t *) ctf_hdr;
>
>         /*
>          * The last two fields in the CTF header are the offset
>          * from the end of the header to the start of the string
>          * data and the length of that string data. se this
>          * information to determine the decompressed CTF data
>          * buffer required.
>          */
>         sz = u32[CTF_HDR_STRTAB_U32] + u32[CTF_HDR_STRLEN_U32] +
>             sizeof(ctf_hdr);
>
>         /*
>          * Allocate memory for the compressed CTF data, including
>          * the header (which isn't compressed).
>          */
>         if ((raw = malloc(shdr[i].sh_size, M_LINKER, M_WAITOK)) == NULL) {
>             error = ENOMEM;
>             goto out;
>         }
>     } else {
>         /*
>          * The CTF data is not compressed, so the ELF section
>          * size is the same as the buffer size required.
>          */
>         sz = shdr[i].sh_size;
>     }
>
>     /*
>      * Allocate memory to buffer the CTF data in it's decompressed
>      * form.
>      */
>     if ((ctftab = malloc(sz, M_LINKER, M_WAITOK)) == NULL) {
>         error = ENOMEM;
>         goto out;
>     }
>
>     /*
>      * Read the CTF data into the raw buffer if compressed, or
>      * directly into the CTF buffer otherwise.
>      */
>     if ((error = vn_rdwr(UIO_READ, nd.ni_vp, raw == NULL ? ctftab : raw,
>         shdr[i].sh_size, shdr[i].sh_offset, UIO_SYSSPACE, IO_NODELOCKED,
>         td->td_ucred, NOCRED, &resid, td)) != 0)
>         goto out;
>
>     /* Check if decompression is required. */
>     if (raw != NULL) {
>         z_stream zs;
>         int ret;
>
>         /*
>          * The header isn't compressed, so copy that into the
>          * CTF buffer first.
>          */
>         bcopy(ctf_hdr, ctftab, sizeof(ctf_hdr));
>
>         /* Initialise the zlib structure. */
>         bzero(&zs, sizeof(zs));
>         zs.zalloc = z_alloc;
>         zs.zfree = z_free;
>
>         if (inflateInit(&zs) != Z_OK) {
>             error = EIO;
>             goto out;
>         }
>
>         zs.avail_in = shdr[i].sh_size - sizeof(ctf_hdr);
>         zs.next_in = ((uint8_t *) raw) + sizeof(ctf_hdr);
>         zs.avail_out = sz - sizeof(ctf_hdr);
>         zs.next_out = ((uint8_t *) ctftab) + sizeof(ctf_hdr);
>         if ((ret = inflate(&zs, Z_FINISH)) != Z_STREAM_END) {
>             printf("%s(%d): zlib inflate returned %d\n", __func__,
> __LINE__, ret);
>             error = EIO;
>             goto out;
>         }
>     }
>
>     /* Got the CTF data! */
>     ef->ctftab = ctftab;
>     ef->ctfcnt = shdr[i].sh_size;
>
>     /* We'll retain the memory allocated for the CTF data. */
>     ctftab = NULL;
>
>     /* Let the caller use the CTF data read. */
>     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;
>
> out:                                                                     //
> here error is 0, but we encounter an ERROR and
>
> // lc->ctfoffp is NULL
>     VOP_UNLOCK(nd.ni_vp, 0);
>     vn_close(nd.ni_vp, FREAD, td->td_ucred, td);
>     VFS_UNLOCK_GIANT(vfslocked);
>
>     if (hdr != NULL)
>         free(hdr, M_LINKER);
>     if (shdr != NULL)
>         free(shdr, M_LINKER);
>     if (shstrtab != NULL)
>         free(shstrtab, M_LINKER);
>     if (ctftab != NULL)
>         free(ctftab, M_LINKER);
>     if (raw != NULL)
>         free(raw, M_LINKER);
> #else
>     error = EOPNOTSUPP;
> #endif
>
>     return (error);
> }
>
> Here is patch to fix the missing check
>
> ..............................................................................................................................
> diff --git a/sys/kern/kern_ctf.c b/sys/kern/kern_ctf.c
> index 758ad81..b78e474 100644
> --- a/sys/kern/kern_ctf.c
> +++ b/sys/kern/kern_ctf.c
> _at__at_ -164,8 +164,12 _at__at_ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
>       * section names aren't present, then we can't locate the
>       * .SUNW_ctf section containing the CTF data.
>       */
> -    if (hdr->e_shstrndx == 0 || shdr[hdr->e_shstrndx].sh_type !=
> SHT_STRTAB)
> +    if (hdr->e_shstrndx == 0 || shdr[hdr->e_shstrndx].sh_type !=
> SHT_STRTAB) {
> +
> +        printf("%s(%d):e_shstrndx is %d, sh_type is %d\n", lf->pathname,
> __LINE__,
> +            hdr->e_shstrndx, shdr[hdr->e_shstrndx].sh_type);
>          goto out;
> +    }
>
>      /* Allocate memory to buffer the section header strings. */
>      if ((shstrtab = malloc(shdr[hdr->e_shstrndx].sh_size, M_LINKER,
> _at__at_ -187,8 +191,12 _at__at_ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
>              break;
>
>      /* Check if the CTF section wasn't found. */
> -    if (i >= hdr->e_shnum)
> +    if (i >= hdr->e_shnum) {
> +        error = EFTYPE;
> +        printf("%s(%d): module %s has no .SUNW_ctf section\n", __func__,
> +                __LINE__, lf->pathname);
>          goto out;
> +    }
>
>      /* Read the CTF header. */
>      if ((error = vn_rdwr(UIO_READ, nd.ni_vp, ctf_hdr, sizeof(ctf_hdr),
> _at__at_ -197,12 +205,20 _at__at_ link_elf_ctf_get(linker_file_t lf, linker_ctf_t *lc)
>          goto out;
>
>      /* Check the CTF magic number. (XXX check for big endian!) */
> -    if (ctf_hdr[0] != 0xf1 || ctf_hdr[1] != 0xcf)
> +    if (ctf_hdr[0] != 0xf1 || ctf_hdr[1] != 0xcf) {
> +        error = EFTYPE;
> +        printf("%s(%d): module %s has wrong format\n", __func__, __LINE__,
>
> +                lf->pathname);
>          goto out;
> +    }
>
>      /* Check if version 2. */
> -    if (ctf_hdr[2] != 2)
> +    if (ctf_hdr[2] != 2) {
> +        error = EFTYPE;
> +        printf("%s(%d): module %s CTF format version is %d\n", __func__,
> __LINE__,
> +                lf->pathname, ctf_hdr[2]);
>          goto out;
> +    }
>
>      /* Check if the data is compressed. */
>      if ((ctf_hdr[3] & 0x1) != 0) {
>
> .........................................................................................................................
>
> 2011/9/26 Paul Ambrose <ambrosehua_at_gmail.com>
>
>> Hi, Ryan, I came across the similar problem on 8-stable when I run
>> # dtrace -lv
>> the panic message says:
>> page fault just happened at fbt.c
>> ........................................................................
>>     if (*lc.ctfoffp == NULL) {       // page fault
>>         /*
>>          * Initialise the CTF object and function symindx to
>>          * byte offset array.
>>          */
>>         if (fbt_ctfoff_init(ctl, &lc) != 0)
>>             return;
>>
>>         /* Initialise the CTF type to byte offset array. */
>>         if (fbt_typoff_init(&lc) != 0)
>>             return;
>>     }
>> ........................................................................
>>
>> And I came across the similar problem on 9-current only once, but when I
>> recompile the kernel,
>> it does not reproduce. I will try your patch on 8-stable, but could you
>> tell me  where did you meet
>> the problem, and what is your module without CTF data?
>>
>
>
Received on Thu Sep 29 2011 - 12:04:41 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:40:18 UTC