Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 24 May 2016 23:41:36 +0000 (UTC)
From:      Don Lewis <truckman@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r300633 - head/usr.sbin/acpi/acpidb
Message-ID:  <201605242341.u4ONfabj083997@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
Author: truckman
Date: Tue May 24 23:41:36 2016
New Revision: 300633
URL: https://svnweb.freebsd.org/changeset/base/300633

Log:
  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
  MFC after:	1 week
  Differential Revision:	https://reviews.freebsd.org/D6368

Modified:
  head/usr.sbin/acpi/acpidb/acpidb.c

Modified: head/usr.sbin/acpi/acpidb/acpidb.c
==============================================================================
--- head/usr.sbin/acpi/acpidb/acpidb.c	Tue May 24 23:36:43 2016	(r300632)
+++ head/usr.sbin/acpi/acpidb/acpidb.c	Tue May 24 23:41:36 2016	(r300633)
@@ -385,8 +385,7 @@ load_dsdt(const char *dsdtfile)
 	ACPI_NEW_TABLE_DESC	*list;
 	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) {
@@ -399,11 +398,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);
 	}
 
@@ -411,21 +412,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.
@@ -487,7 +497,7 @@ load_dsdt(const char *dsdtfile)
 	AcpiGbl_DebuggerConfiguration = 0;
 	AcpiDbUserCommands(':', NULL);
 
-	if (strcmp(dsdtfile, filetmp) != 0) {
+	if (dounlink) {
 		unlink(filetmp);
 	}

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201605242341.u4ONfabj083997>