From owner-freebsd-current@FreeBSD.ORG Thu Sep 29 14:04:41 2011 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id B0978106564A for ; Thu, 29 Sep 2011 14:04:41 +0000 (UTC) (envelope-from ambrosehua@gmail.com) Received: from mail-fx0-f54.google.com (mail-fx0-f54.google.com [209.85.161.54]) by mx1.freebsd.org (Postfix) with ESMTP id 00E3C8FC0A for ; Thu, 29 Sep 2011 14:04:40 +0000 (UTC) Received: by fxg9 with SMTP id 9so2433131fxg.13 for ; Thu, 29 Sep 2011 07:04:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :content-type; bh=vJa5zU13JfdI7F5sIpD5CumFNNgc6J3MG7fAprisBPw=; b=pB0FmsSnXmT9l6kCy07s76t5rzlHELgTy4bE2uPcobq4wh2H8WrfeqOzzAGevEJEp0 U9bk8ISh7yajul/DS3DnAScBiPqvTLeQudvYSff88p121uFDRYUkpNDQJ5YFPzTU3kMu 8VJKih1+ZB2llhOTdP/ryABNXZ3fJu3sp0ttE= MIME-Version: 1.0 Received: by 10.223.47.207 with SMTP id o15mr5797492faf.88.1317305079801; Thu, 29 Sep 2011 07:04:39 -0700 (PDT) Received: by 10.223.69.132 with HTTP; Thu, 29 Sep 2011 07:04:39 -0700 (PDT) In-Reply-To: References: Date: Thu, 29 Sep 2011 22:04:39 +0800 Message-ID: From: Paul Ambrose To: rysto32@gmail.com, freebsd-current@freebsd.org Content-Type: text/plain; charset=ISO-8859-1 X-Content-Filtered-By: Mailman/MimeDel 2.1.5 Cc: Subject: Re: [PATCH] dtrace crashes when trying to trace fbt probes X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Sep 2011 14:04:41 -0000 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 @@ -164,8 +164,14 @@ 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, @@ -187,8 +193,12 @@ 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), @@ -197,12 +207,20 @@ 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 > 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 > @@ -164,8 +164,12 @@ 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, > @@ -187,8 +191,12 @@ 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), > @@ -197,12 +205,20 @@ 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 > >> 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? >> > >