d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix acpidb CIDs 1011279 (Buffer not null terminated) and 978405 and

Browse Source 
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
This commit is contained in:
Don Lewis 2016-05-24 23:41:36 +00:00
parent 7e2cc014bd
commit 668bf37a0e
1 changed files with 21 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
usr.sbin/acpi/acpidb/acpidb.c
View File

@ -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) {
perror("open");
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);
}
write(fd2, &dummy_dsdt_table, sizeof(ACPI_TABLE_HEADER));
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(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);
}