From owner-svn-src-stable@freebsd.org Wed Jun 1 17:16:36 2016 Return-Path: Delivered-To: svn-src-stable@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id F1890B617D0; Wed, 1 Jun 2016 17:16:36 +0000 (UTC) (envelope-from truckman@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 mx1.freebsd.org (Postfix) with ESMTPS id CD8D110B0; Wed, 1 Jun 2016 17:16:36 +0000 (UTC) (envelope-from truckman@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id u51HGa5o048342; Wed, 1 Jun 2016 17:16:36 GMT (envelope-from truckman@FreeBSD.org) Received: (from truckman@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id u51HGaqK048341; Wed, 1 Jun 2016 17:16:36 GMT (envelope-from truckman@FreeBSD.org) Message-Id: <201606011716.u51HGaqK048341@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: truckman set sender to truckman@FreeBSD.org using -f From: Don Lewis Date: Wed, 1 Jun 2016 17:16:36 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org Subject: svn commit: r301142 - stable/10/usr.sbin/acpi/acpidb X-SVN-Group: stable-10 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.22 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: Wed, 01 Jun 2016 17:16:37 -0000 Author: truckman Date: Wed Jun 1 17:16:35 2016 New Revision: 301142 URL: https://svnweb.freebsd.org/changeset/base/301142 Log: MFC r300633 Fix acpidb CIDs 1011279 (Buffer not null terminated) and 978405 and 1199380 (Resource leak). load_dsdt() calls strncpy() to copy a filename and Coverity warns that the destination buffer may not be NUL terminated. Fix this by using strlcpy() instead. If silent truncation occurs, then the filename was not valid anyway. load_dsdt() leaks an fd (CID 978405) and a memory region allocated using mmap() (CID 1199380) when it returns. Fix these by calling close() and munmap() as appropriate. Don't bother fixing the minor memory leak "list", allocated by AcGetAllTablesFromFile() (CID 1355191). Check for truncation when creating the temp file name. Set a flag to indicate that the temp file should be unlinked. Relying on a strcmp() test could delete the input file in contrived cases. Reported by: Coverity CID: 1011279, 978405, 1199380 Reviewed by: jkim Differential Revision: https://reviews.freebsd.org/D6368 Modified: stable/10/usr.sbin/acpi/acpidb/acpidb.c Directory Properties: stable/10/ (props changed) Modified: stable/10/usr.sbin/acpi/acpidb/acpidb.c ============================================================================== --- stable/10/usr.sbin/acpi/acpidb/acpidb.c Wed Jun 1 17:13:43 2016 (r301141) +++ stable/10/usr.sbin/acpi/acpidb/acpidb.c Wed Jun 1 17:16:35 2016 (r301142) @@ -383,8 +383,7 @@ load_dsdt(const char *dsdtfile) char filetmp[PATH_MAX]; u_int8_t *code; struct stat sb; - int fd, fd2; - int error; + int dounlink, error, fd; fd = open(dsdtfile, O_RDONLY, 0); if (fd == -1) { @@ -397,11 +396,13 @@ load_dsdt(const char *dsdtfile) return (-1); } code = mmap(NULL, (size_t)sb.st_size, PROT_READ, MAP_PRIVATE, fd, (off_t)0); + close(fd); if (code == NULL) { perror("mmap"); return (-1); } if ((error = AcpiInitializeSubsystem()) != AE_OK) { + munmap(code, (size_t)sb.st_size); return (-1); } @@ -409,21 +410,30 @@ load_dsdt(const char *dsdtfile) * make sure DSDT data contains table header or not. */ if (strncmp((char *)code, "DSDT", 4) == 0) { - strncpy(filetmp, dsdtfile, sizeof(filetmp)); + dounlink = 0; + strlcpy(filetmp, dsdtfile, sizeof(filetmp)); } else { + dounlink = 1; mode_t mode = (S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); dummy_dsdt_table.Length = sizeof(ACPI_TABLE_HEADER) + sb.st_size; - snprintf(filetmp, sizeof(filetmp), "%s.tmp", dsdtfile); - fd2 = open(filetmp, O_WRONLY | O_CREAT | O_TRUNC, mode); - if (fd2 == -1) { + if ((size_t)snprintf(filetmp, sizeof(filetmp), "%s.tmp", + dsdtfile) > sizeof(filetmp) - 1) { + fprintf(stderr, "file name too long\n"); + munmap(code, (size_t)sb.st_size); + return (-1); + } + fd = open(filetmp, O_WRONLY | O_CREAT | O_TRUNC, mode); + if (fd == -1) { perror("open"); + munmap(code, (size_t)sb.st_size); return (-1); } - write(fd2, &dummy_dsdt_table, sizeof(ACPI_TABLE_HEADER)); + write(fd, &dummy_dsdt_table, sizeof(ACPI_TABLE_HEADER)); - write(fd2, code, sb.st_size); - close(fd2); + write(fd, code, sb.st_size); + close(fd); } + munmap(code, (size_t)sb.st_size); /* * Install the virtual machine version of address space handlers. @@ -484,7 +494,7 @@ load_dsdt(const char *dsdtfile) AcpiGbl_DebuggerConfiguration = 0; AcpiDbUserCommands(':', NULL); - if (strcmp(dsdtfile, filetmp) != 0) { + if (dounlink) { unlink(filetmp); }