>Submitter-Id: current-users
>Originator: Dan Lukes
>Organization: Obludarium
>Confidential: no
>Synopsis: [ patch ] improper handling of ACPI TCPA table, acpidump abend
>imminent
>Severity: serious
>Priority: medium
>Category: bin
>Class: sw-bug
>Release: FreeBSD 9.0 i386
>Environment:
System: FreeBSD 9.0
src/usr.sbin/acpi/acpidump/acpi.c,v 1.42.2.1.2.1
but apply for all revisions past 1.38 (e.g. all RELENG_9 and HEAD)
>Description:
TCG ACPI (TPCA) support added as SVN rev 211196
1. event->event_type and event->event_size are big-endian (see TPCA PC Specific
Specification, paragraph 7.2.2.2). Current code use them directly. It cause
misinterpretation of values and may cause abend.
2. 'if (vaddr + event->event_size >= vend )' test is insufficient because:
2a) event->event_size is declared signed and may be negative (especialy when
big-endian value used without proper conversion)
2b) vaddr+event->event_size may overflow / wrap around even in the case the
event_size is positive
in both cases, memory outside of <vaddr,vend> range may be referenced. Abend is
imminent.
>How-To-Repeat:
Dump non-empty TCPA table. It will print events incorrectly, may abend.
>Fix:
1. use ntohl() to convert event->event_size and event->event_type before use
2. test vaddr + eventdatasize for wraparound/underflow case also
--- usr.sbin/acpi/acpidump/acpi.c.old 2012-07-08 13:14:21.000000000 +0200
+++ usr.sbin/acpi/acpidump/acpi.c 2012-07-08 13:06:46.000000000 +0200
@@ -40,6 +40,7 @@
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
+#include <netinet/in.h>
#include "acpidump.h"
@@ -538,10 +539,15 @@
{
struct TCPApc_event *pc_event;
char *eventname = NULL;
+ unsigned int eventid, eventdatasize;
+
+ // EventID and EventDataSize are big endian (TPCA PC Specific
Specification [7.2.2.2])
+ eventid = ntohl(event->event_type);
+ eventdatasize = ntohl(event->event_size);
pc_event = (struct TCPApc_event *)(event + 1);
- switch(event->event_type) {
+ switch(eventid) {
case PREBOOT:
case POST_CODE:
case UNUSED:
@@ -559,12 +565,12 @@
case NONHOST_CONFIG:
case NONHOST_INFO:
asprintf(&eventname, "%s",
- tcpa_event_type_strings[event->event_type]);
+ tcpa_event_type_strings[eventid]);
break;
case ACTION:
- eventname = calloc(event->event_size + 1, sizeof(char));
- memcpy(eventname, pc_event, event->event_size);
+ eventname = calloc(eventdatasize + 1, sizeof(char));
+ memcpy(eventname, pc_event, eventdatasize);
break;
case EVENT_TAG:
@@ -593,7 +599,7 @@
break;
default:
- asprintf(&eventname, "<unknown 0x%02x>", event->event_type);
+ asprintf(&eventname, "<unknown 0x%02x>", eventid);
break;
}
@@ -659,20 +665,27 @@
vend = vaddr + len;
while (vaddr != NULL) {
+ unsigned int eventid, eventdatasize;
+
if (vaddr + sizeof(struct TCPAevent) >= vend)
break;
event = (struct TCPAevent *)(void *)vaddr;
- if (vaddr + event->event_size >= vend)
+ // EventID and EventDataSize are big endian (TPCA PC Specific
Specification [7.2.2.2])
+ eventid = ntohl(event->event_type);
+ eventdatasize = ntohl(event->event_size);
+ if (vaddr + eventdatasize >= vend )
+ break;
+ if (vaddr + eventdatasize < vaddr )
break;
- if (event->event_type == 0 && event->event_size == 0)
+ if (eventid == 0 && eventdatasize == 0)
break;
#if 0
{
unsigned int i, j, k;
- printf("\n\tsize %d\n\t\t%p ", event->event_size, vaddr);
+ printf("\n\tsize %u\n\t\t%p ", eventdatasize, vaddr);
for (j = 0, i = 0; i <
- sizeof(struct TCPAevent) + event->event_size; i++) {
+ sizeof(struct TCPAevent) + eventdatasize; i++) {
printf("%02x ", vaddr[i]);
if ((i+1) % 8 == 0) {
for (k = 0; k < 8; k++)
@@ -686,7 +699,7 @@
#endif
acpi_print_tcpa(event);
- vaddr += sizeof(struct TCPAevent) + event->event_size;
+ vaddr += sizeof(struct TCPAevent) + eventdatasize;
}
printf(END_COMMENT);
--- patch ends here ---
_______________________________________________
freebsd-acpi@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"
