From owner-freebsd-bugs@FreeBSD.ORG Sun Jul 8 12:00:25 2012 Return-Path: Delivered-To: freebsd-bugs@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 432471065679 for ; Sun, 8 Jul 2012 12:00:25 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 197008FC14 for ; Sun, 8 Jul 2012 12:00:25 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.5/8.14.5) with ESMTP id q68C0O56081375 for ; Sun, 8 Jul 2012 12:00:24 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.5/8.14.5/Submit) id q68C0OET081372; Sun, 8 Jul 2012 12:00:24 GMT (envelope-from gnats) Resent-Date: Sun, 8 Jul 2012 12:00:24 GMT Resent-Message-Id: <201207081200.q68C0OET081372@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-bugs@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Dan Lukes Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E26C9106564A; Sun, 8 Jul 2012 11:58:08 +0000 (UTC) (envelope-from dan@obluda.cz) Received: from kgw.obluda.cz (kgw.obluda.cz [193.179.199.50]) by mx1.freebsd.org (Postfix) with ESMTP id 48E828FC18; Sun, 8 Jul 2012 11:58:08 +0000 (UTC) Received: from localhost (localhost [127.0.0.1]) by localhost (8.14.5/8.14.5) with ESMTP id q68BseAD002032; Sun, 8 Jul 2012 13:54:40 +0200 (CEST) (envelope-from dan@obluda.cz) Received: (from root@localhost) by localhost (8.14.5/8.14.5/Submit) id q68BseNo002031; Sun, 8 Jul 2012 13:54:40 +0200 (CEST) (envelope-from dan@obluda.cz) Message-Id: <201207081154.q68BseNo002031@nb.obluda.cz> Date: Sun, 8 Jul 2012 13:54:40 +0200 (CEST) From: Dan Lukes To: FreeBSD-gnats-submit@FreeBSD.org X-Send-Pr-Version: 3.113 Cc: freebsd-acpi@FreeBSD.org, Hans-Joerg_Hoexer@genua.de Subject: bin/169707: [ patch ] improper handling of ACPI TCPA table, acpidump abend imminent X-BeenThere: freebsd-bugs@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Dan Lukes List-Id: Bug reports List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 08 Jul 2012 12:00:25 -0000 >Number: 169707 >Category: bin >Synopsis: [ patch ] improper handling of ACPI TCPA table, acpidump abend imminent >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sun Jul 08 12:00:24 UTC 2012 >Closed-Date: >Last-Modified: >Originator: Dan Lukes >Release: FreeBSD 9.0 i386 >Organization: Obludarium >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 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 #include #include +#include #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, "", event->event_type); + asprintf(&eventname, "", 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 --- >Release-Note: >Audit-Trail: >Unformatted: