From owner-svn-src-all@FreeBSD.ORG Tue Jan 13 23:02:55 2009 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1B99F10657D2; Tue, 13 Jan 2009 23:02:55 +0000 (UTC) (envelope-from luigi@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 012488FC16; Tue, 13 Jan 2009 23:02:55 +0000 (UTC) (envelope-from luigi@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n0DN2sEZ019019; Tue, 13 Jan 2009 23:02:54 GMT (envelope-from luigi@svn.freebsd.org) Received: (from luigi@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n0DN2sUX019014; Tue, 13 Jan 2009 23:02:54 GMT (envelope-from luigi@svn.freebsd.org) Message-Id: <200901132302.n0DN2sUX019014@svn.freebsd.org> From: Luigi Rizzo Date: Tue, 13 Jan 2009 23:02:54 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-7@freebsd.org X-SVN-Group: stable-7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r187201 - stable/7/usr.sbin/kldxref X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 13 Jan 2009 23:02:59 -0000 Author: luigi Date: Tue Jan 13 23:02:54 2009 New Revision: 187201 URL: http://svn.freebsd.org/changeset/base/187201 Log: MFC: rev. 185475, 186824, 186826, 186827 various cleanups including: + check a possible buffer overflow when creating a temp file, submitted by Christoph Mallon + remove stale struct definitions + clarify the use of dflag and remove useless checks + Make the linker.hints file have mode 644 instead of 600. + fix a couple of innocuous compiler warnings + correct description of how a string is stored, fix a few typos and reference the kernel file which processes this info. Modified: stable/7/usr.sbin/kldxref/Makefile stable/7/usr.sbin/kldxref/ef_i386.c stable/7/usr.sbin/kldxref/ef_obj.c stable/7/usr.sbin/kldxref/fileformat stable/7/usr.sbin/kldxref/kldxref.c Modified: stable/7/usr.sbin/kldxref/Makefile ============================================================================== --- stable/7/usr.sbin/kldxref/Makefile Tue Jan 13 22:46:24 2009 (r187200) +++ stable/7/usr.sbin/kldxref/Makefile Tue Jan 13 23:02:54 2009 (r187201) @@ -5,6 +5,7 @@ MAN= kldxref.8 SRCS= kldxref.c ef.c ef_obj.c WARNS?= 2 +CFLAGS+=-fno-strict-aliasing .if exists(ef_${MACHINE_ARCH}.c) SRCS+= ef_${MACHINE_ARCH}.c Modified: stable/7/usr.sbin/kldxref/ef_i386.c ============================================================================== --- stable/7/usr.sbin/kldxref/ef_i386.c Tue Jan 13 22:46:24 2009 (r187200) +++ stable/7/usr.sbin/kldxref/ef_i386.c Tue Jan 13 23:02:54 2009 (r187201) @@ -43,12 +43,13 @@ */ int ef_reloc(struct elf_file *ef, const void *reldata, int reltype, Elf_Off relbase, - Elf_Off dataoff, size_t len, void *dest) + Elf_Off dataoff, size_t len, void *_dest) { Elf_Addr *where, addr, addend; Elf_Size rtype, symidx; const Elf_Rel *rel; const Elf_Rela *rela; + char *dest = _dest; switch (reltype) { case EF_RELOC_REL: Modified: stable/7/usr.sbin/kldxref/ef_obj.c ============================================================================== --- stable/7/usr.sbin/kldxref/ef_obj.c Tue Jan 13 22:46:24 2009 (r187200) +++ stable/7/usr.sbin/kldxref/ef_obj.c Tue Jan 13 23:02:54 2009 (r187201) @@ -133,7 +133,7 @@ static struct elf_file_ops ef_obj_file_o }; static int -ef_obj_get_type(elf_file_t ef) +ef_obj_get_type(elf_file_t __unused ef) { return (EFT_KLD); @@ -180,7 +180,7 @@ ef_obj_symaddr(elf_file_t ef, Elf_Size s { const Elf_Sym *sym; - if (symidx >= ef->ddbsymcnt) + if (symidx >= (size_t) ef->ddbsymcnt) return (0); sym = ef->ddbsymtab + symidx; Modified: stable/7/usr.sbin/kldxref/fileformat ============================================================================== --- stable/7/usr.sbin/kldxref/fileformat Tue Jan 13 22:46:24 2009 (r187200) +++ stable/7/usr.sbin/kldxref/fileformat Tue Jan 13 23:02:54 2009 (r187201) @@ -1,7 +1,9 @@ $FreeBSD$ - linker.hints file consists from the one or more records. First record of -file is special and determines its version: +linker.hints file consists from the one or more records, +and is processed by sys/kern/kern_linker.c::linker_hints_lookup() + +First record of file is special and determines its version: int version; @@ -16,25 +18,28 @@ struct record { 'data' determines its type: struct data { - int type; /* type of data. currently MTD_* values */ + int type; /* type of data. currently MDT_* values */ }; The rest of record depends on the type. struct string { - int length; /* length of string */ + uint8_t length; /* length of string */ char val[]; /* string itself (no terminating zero) */ }; struct data_mdt_version { int type = MDT_VERSION; struct string modname; + /* padding */ int version; struct string kldname; + /* padding */ }; struct data_mdt_module { - int type = MDT_VERSION; + int type = MDT_MODULE; struct string modname; struct string kldname; + /* padding */ }; Modified: stable/7/usr.sbin/kldxref/kldxref.c ============================================================================== --- stable/7/usr.sbin/kldxref/kldxref.c Tue Jan 13 22:46:24 2009 (r187200) +++ stable/7/usr.sbin/kldxref/kldxref.c Tue Jan 13 23:02:54 2009 (r187201) @@ -57,37 +57,19 @@ #define MAXRECSIZE 1024 #define check(val) if ((error = (val)) != 0) break -#ifndef min -#define min(a,b) (((a)<(b)) ? (a) : (b)) -#endif - -struct mod_info { - char* mi_name; - int mi_ver; - SLIST_ENTRY(mod_info) mi_next; -}; - -#ifdef notnow -struct kld_info { - char* k_filename; - SLIST_HEAD(mod_list_head, mod_info) k_modules; - SLIST_ENTRY(kld_info) k_next; -}; +static int dflag; /* do not create a hint file, only write on stdout */ +static int verbose; -SLIST_HEAD(kld_list_head, kld_info) kldlist; -#endif - -static int dflag, verbose; - -FILE *fxref; +static FILE *fxref; /* current hints file */ static const char *xref_file = "linker.hints"; +/* + * A record is stored in the static buffer recbuf before going to disk. + */ static char recbuf[MAXRECSIZE]; -static int recpos, reccnt; - -FILE *maketempfile(char *, const char *); -static void usage(void); +static int recpos; /* current write position */ +static int reccnt; /* total record written to this file so far */ static void intalign(void) @@ -105,7 +87,7 @@ record_start(void) static int record_end(void) { - if (dflag || recpos == 0) + if (recpos == 0) return 0; reccnt++; intalign(); @@ -123,6 +105,9 @@ record_buf(const void *buf, int size) return 0; } +/* + * An int is stored in host order and aligned + */ static int record_int(int val) { @@ -130,21 +115,21 @@ record_int(int val) return record_buf(&val, sizeof(val)); } -static int -record_byte(u_char val) -{ - return record_buf(&val, sizeof(val)); -} - +/* + * A string is stored as 1-byte length plus data, no padding + */ static int record_string(const char *str) { - int len = strlen(str); - int error; - + int len, error; + u_char val; + if (dflag) return 0; - error = record_byte(len); + val = len = strlen(str); + if (len > 255) + errx(1, "string %s too long", str); + error = record_buf(&val, sizeof(val)); if (error) return error; return record_buf(str, len); @@ -170,21 +155,23 @@ parse_entry(struct mod_metadata *md, con break; case MDT_VERSION: check(EF_SEG_READ(ef, data, sizeof(mdv), &mdv)); - record_int(MDT_VERSION); - record_string(cval); - record_int(mdv.mv_version); - record_string(kldname); - if (!dflag) - break; - printf(" interface %s.%d\n", cval, mdv.mv_version); + if (dflag) { + printf(" interface %s.%d\n", cval, mdv.mv_version); + } else { + record_int(MDT_VERSION); + record_string(cval); + record_int(mdv.mv_version); + record_string(kldname); + } break; case MDT_MODULE: - record_int(MDT_MODULE); - record_string(cval); - record_string(kldname); - if (!dflag) - break; - printf(" module %s\n", cval); + if (dflag) { + printf(" module %s\n", cval); + } else { + record_int(MDT_MODULE); + record_string(cval); + record_string(kldname); + } break; default: warnx("unknown metadata record %d in file %s", md->md_type, kldname); @@ -199,8 +186,6 @@ read_kld(char *filename, char *kldname) { struct mod_metadata md; struct elf_file ef; -/* struct kld_info *kip; - struct mod_info *mip;*/ void **p, **orgp; int error, eftype, nmlen; long start, finish, entries; @@ -224,8 +209,9 @@ read_kld(char *filename, char *kldname) } if (!dflag) { cp = strrchr(kldname, '.'); - nmlen = cp ? min(MAXMODNAME, cp - kldname) : - min(MAXMODNAME, strlen(kldname)); + nmlen = (cp != NULL) ? cp - kldname : (int)strlen(kldname); + if (nmlen > MAXMODNAME) + nmlen = MAXMODNAME; strlcpy(kldmodname, kldname, nmlen); /* fprintf(fxref, "%s:%s:%d\n", kldmodname, kldname, 0);*/ } @@ -252,25 +238,43 @@ read_kld(char *filename, char *kldname) return error; } -FILE * +/* + * Create a temp file in directory root, make sure we don't + * overflow the buffer for the destination name + */ +static FILE * maketempfile(char *dest, const char *root) { char *p; - int fd; + int n, fd; - strlcpy(dest, root, MAXPATHLEN); + p = strrchr(root, '/'); + n = p != NULL ? p - root + 1 : 0; + if (snprintf(dest, MAXPATHLEN, "%.*slhint.XXXXXX", n, root) >= + MAXPATHLEN) { + errno = ENAMETOOLONG; + return NULL; + } - if ((p = strrchr(dest, '/')) != 0) - p++; - else - p = dest; - strcpy(p, "lhint.XXXXXX"); fd = mkstemp(dest); - return ((fd == -1) ? NULL : fdopen(fd, "w+")); + if (fd < 0) + return NULL; + fchmod(fd, 0644); /* nothing secret in the file */ + return fdopen(fd, "w+"); } static char xrefname[MAXPATHLEN], tempname[MAXPATHLEN]; +static void +usage(void) +{ + + fprintf(stderr, "%s\n", + "usage: kldxref [-Rdv] [-f hintsfile] path ..." + ); + exit(1); +} + int main(int argc, char *argv[]) { @@ -280,20 +284,19 @@ main(int argc, char *argv[]) struct stat sb; fts_options = FTS_PHYSICAL; -/* SLIST_INIT(&kldlist);*/ while ((opt = getopt(argc, argv, "Rdf:v")) != -1) { switch (opt) { - case 'd': + case 'd': /* no hint file, only print on stdout */ dflag = 1; break; - case 'f': + case 'f': /* use this name instead of linker.hints */ xref_file = optarg; break; case 'v': verbose++; break; - case 'R': + case 'R': /* recurse on directories */ fts_options |= FTS_COMFOLLOW; break; default: @@ -319,19 +322,22 @@ main(int argc, char *argv[]) for (;;) { p = fts_read(ftsp); - if ((p == NULL || p->fts_info == FTS_D) && !dflag && fxref) { + if ((p == NULL || p->fts_info == FTS_D) && fxref) { + /* close and rename the current hint file */ fclose(fxref); fxref = NULL; if (reccnt) { rename(tempname, xrefname); } else { + /* didn't find any entry, ignore this file */ unlink(tempname); unlink(xrefname); } } if (p == NULL) break; - if (p && p->fts_info == FTS_D && !dflag) { + if (p->fts_info == FTS_D && !dflag) { + /* visiting a new directory, create a new hint file */ snprintf(xrefname, sizeof(xrefname), "%s/%s", ftsp->fts_path, xref_file); fxref = maketempfile(tempname, ftsp->fts_path); @@ -341,6 +347,7 @@ main(int argc, char *argv[]) fwrite(&ival, sizeof(ival), 1, fxref); reccnt = 0; } + /* skip non-files or .symbols entries */ if (p->fts_info != FTS_F) continue; if (p->fts_namelen >= 8 && @@ -351,13 +358,3 @@ main(int argc, char *argv[]) fts_close(ftsp); return 0; } - -static void -usage(void) -{ - - fprintf(stderr, "%s\n", - "usage: kldxref [-Rdv] [-f hintsfile] path ..." - ); - exit(1); -}