From owner-svn-src-stable@freebsd.org Thu Oct 24 02:36:43 2019 Return-Path: Delivered-To: svn-src-stable@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 036F8173961; Thu, 24 Oct 2019 02:36:43 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 46zBG267TVz498f; Thu, 24 Oct 2019 02:36:42 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id B2BC37590; Thu, 24 Oct 2019 02:36:42 +0000 (UTC) (envelope-from kevans@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x9O2agQA087739; Thu, 24 Oct 2019 02:36:42 GMT (envelope-from kevans@FreeBSD.org) Received: (from kevans@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x9O2agZH087738; Thu, 24 Oct 2019 02:36:42 GMT (envelope-from kevans@FreeBSD.org) Message-Id: <201910240236.x9O2agZH087738@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kevans set sender to kevans@FreeBSD.org using -f From: Kyle Evans Date: Thu, 24 Oct 2019 02:36:42 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-12@freebsd.org Subject: svn commit: r353978 - in stable/12/stand: common efi/loader X-SVN-Group: stable-12 X-SVN-Commit-Author: kevans X-SVN-Commit-Paths: in stable/12/stand: common efi/loader X-SVN-Commit-Revision: 353978 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for all the -stable branches of the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Oct 2019 02:36:43 -0000 Author: kevans Date: Thu Oct 24 02:36:42 2019 New Revision: 353978 URL: https://svnweb.freebsd.org/changeset/base/353978 Log: MFC r345998-r346002, r346007-r346008: various loader improvements r345998: loader: malloc+bzero is calloc Replace malloc+bzero in module.c with calloc. r345999: loader: file_addmodule should check for memory allocation strdup() can return NULL. r346000: loader: remove pointer checks before free() in module.c free() does check for NULL argument, remove duplicate checks. r346001: loader: file_addmetadata() should check for memory allocation malloc() can return NULL. r346002: loader: mod_loadkld() error: we previously assumed 'last_file' could be null The last_file variable is used to reset the loadaddr variable back to original value; however, it is possible the last_file is NULL, so we can not blindly trust it. But then again, we can just save the original loadaddr and use the saved value for recovery. r346007: loader: add file_remove() function to undo file_insert_tail(). 346002 did miss the fact that we do not only undo the loadaddr, but also we need to remove the inserted module. Implement file_remove() to do the job. r346008: loader: command_lsefi: ret can be used uninitialized Modified: stable/12/stand/common/module.c stable/12/stand/efi/loader/main.c Directory Properties: stable/12/ (props changed) Modified: stable/12/stand/common/module.c ============================================================================== --- stable/12/stand/common/module.c Thu Oct 24 02:34:48 2019 (r353977) +++ stable/12/stand/common/module.c Thu Oct 24 02:36:42 2019 (r353978) @@ -52,16 +52,18 @@ struct moduledir { STAILQ_ENTRY(moduledir) d_link; }; -static int file_load(char *filename, vm_offset_t dest, struct preloaded_file **result); -static int file_load_dependencies(struct preloaded_file *base_mod); -static char * file_search(const char *name, char **extlist); -static struct kernel_module * file_findmodule(struct preloaded_file *fp, char *modname, struct mod_depend *verinfo); -static int file_havepath(const char *name); -static char *mod_searchmodule(char *name, struct mod_depend *verinfo); -static void file_insert_tail(struct preloaded_file *mp); -struct file_metadata* metadata_next(struct file_metadata *base_mp, int type); -static void moduledir_readhints(struct moduledir *mdp); -static void moduledir_rebuild(void); +static int file_load(char *, vm_offset_t, struct preloaded_file **); +static int file_load_dependencies(struct preloaded_file *); +static char * file_search(const char *, char **); +static struct kernel_module *file_findmodule(struct preloaded_file *, char *, + struct mod_depend *); +static int file_havepath(const char *); +static char *mod_searchmodule(char *, struct mod_depend *); +static void file_insert_tail(struct preloaded_file *); +static void file_remove(struct preloaded_file *); +struct file_metadata *metadata_next(struct file_metadata *, int); +static void moduledir_readhints(struct moduledir *); +static void moduledir_rebuild(void); /* load address should be tweaked by first module loaded (kernel) */ static vm_offset_t loadaddr = 0; @@ -70,10 +72,11 @@ static vm_offset_t loadaddr = 0; static const char *default_searchpath = "/boot/kernel;/boot/modules;/boot/dtb"; #else -static const char *default_searchpath ="/boot/kernel;/boot/modules"; +static const char *default_searchpath = "/boot/kernel;/boot/modules"; #endif -static STAILQ_HEAD(, moduledir) moduledir_list = STAILQ_HEAD_INITIALIZER(moduledir_list); +static STAILQ_HEAD(, moduledir) moduledir_list = + STAILQ_HEAD_INITIALIZER(moduledir_list); struct preloaded_file *preloaded_files = NULL; @@ -534,8 +537,7 @@ mod_load(char *modname, struct mod_depend *verinfo, in mp = file_findmodule(NULL, modname, verinfo); if (mp) { #ifdef moduleargs - if (mp->m_args) - free(mp->m_args); + free(mp->m_args); mp->m_args = unargv(argc, argv); #endif snprintf(command_errbuf, sizeof(command_errbuf), @@ -560,9 +562,10 @@ mod_load(char *modname, struct mod_depend *verinfo, in int mod_loadkld(const char *kldname, int argc, char *argv[]) { - struct preloaded_file *fp, *last_file; - int err; + struct preloaded_file *fp; + int err; char *filename; + vm_offset_t loadaddr_saved; /* * Get fully qualified KLD name @@ -583,22 +586,19 @@ mod_loadkld(const char *kldname, int argc, char *argv[ free(filename); return (0); } - for (last_file = preloaded_files; - last_file != NULL && last_file->f_next != NULL; - last_file = last_file->f_next) - ; do { err = file_load(filename, loadaddr, &fp); if (err) break; fp->f_args = unargv(argc, argv); + loadaddr_saved = loadaddr; loadaddr = fp->f_addr + fp->f_size; - file_insert_tail(fp); /* Add to the list of loaded files */ + file_insert_tail(fp); /* Add to the list of loaded files */ if (file_load_dependencies(fp) != 0) { err = ENOENT; - last_file->f_next = NULL; - loadaddr = last_file->f_addr + last_file->f_size; + file_remove(fp); + loadaddr = loadaddr_saved; fp = NULL; break; } @@ -678,10 +678,12 @@ file_addmetadata(struct preloaded_file *fp, int type, struct file_metadata *md; md = malloc(sizeof(struct file_metadata) - sizeof(md->md_data) + size); - md->md_size = size; - md->md_type = type; - bcopy(p, md->md_data, size); - md->md_next = fp->f_metadata; + if (md != NULL) { + md->md_size = size; + md->md_type = type; + bcopy(p, md->md_data, size); + md->md_next = fp->f_metadata; + } fp->f_metadata = md; } @@ -926,11 +928,14 @@ file_addmodule(struct preloaded_file *fp, char *modnam mp = file_findmodule(fp, modname, &mdepend); if (mp) return (EEXIST); - mp = malloc(sizeof(struct kernel_module)); + mp = calloc(1, sizeof(struct kernel_module)); if (mp == NULL) return (ENOMEM); - bzero(mp, sizeof(struct kernel_module)); mp->m_name = strdup(modname); + if (mp->m_name == NULL) { + free(mp); + return (ENOMEM); + } mp->m_version = version; mp->m_fp = fp; mp->m_next = fp->f_modules; @@ -958,18 +963,14 @@ file_discard(struct preloaded_file *fp) } mp = fp->f_modules; while (mp) { - if (mp->m_name) - free(mp->m_name); + free(mp->m_name); mp1 = mp; mp = mp->m_next; free(mp1); } - if (fp->f_name != NULL) - free(fp->f_name); - if (fp->f_type != NULL) - free(fp->f_type); - if (fp->f_args != NULL) - free(fp->f_args); + free(fp->f_name); + free(fp->f_type); + free(fp->f_args); free(fp); } @@ -980,12 +981,8 @@ file_discard(struct preloaded_file *fp) struct preloaded_file * file_alloc(void) { - struct preloaded_file *fp; - if ((fp = malloc(sizeof(struct preloaded_file))) != NULL) { - bzero(fp, sizeof(struct preloaded_file)); - } - return (fp); + return (calloc(1, sizeof(struct preloaded_file))); } /* @@ -1007,6 +1004,29 @@ file_insert_tail(struct preloaded_file *fp) } } +/* + * Remove module from the chain + */ +static void +file_remove(struct preloaded_file *fp) +{ + struct preloaded_file *cm; + + if (preloaded_files == NULL) + return; + + if (preloaded_files == fp) { + preloaded_files = fp->f_next; + return; + } + for (cm = preloaded_files; cm->f_next != NULL; cm = cm->f_next) { + if (cm->f_next == fp) { + cm->f_next = fp->f_next; + return; + } + } +} + static char * moduledir_fullpath(struct moduledir *mdp, const char *fname) { @@ -1056,10 +1076,8 @@ moduledir_readhints(struct moduledir *mdp) return; bad: close(fd); - if (mdp->d_hints) { - free(mdp->d_hints); - mdp->d_hints = NULL; - } + free(mdp->d_hints); + mdp->d_hints = NULL; mdp->d_flags |= MDIR_NOHINTS; return; } @@ -1120,8 +1138,7 @@ moduledir_rebuild(void) if ((mdp->d_flags & MDIR_REMOVED) == 0) { mdp = STAILQ_NEXT(mdp, d_link); } else { - if (mdp->d_hints) - free(mdp->d_hints); + free(mdp->d_hints); mtmp = mdp; mdp = STAILQ_NEXT(mdp, d_link); STAILQ_REMOVE(&moduledir_list, mtmp, moduledir, d_link); Modified: stable/12/stand/efi/loader/main.c ============================================================================== --- stable/12/stand/efi/loader/main.c Thu Oct 24 02:34:48 2019 (r353977) +++ stable/12/stand/efi/loader/main.c Thu Oct 24 02:36:42 2019 (r353978) @@ -1217,7 +1217,7 @@ command_lsefi(int argc __unused, char *argv[] __unused EFI_HANDLE handle; UINTN bufsz = 0, i, j; EFI_STATUS status; - int ret; + int ret = 0; status = BS->LocateHandle(AllHandles, NULL, NULL, &bufsz, buffer); if (status != EFI_BUFFER_TOO_SMALL) {