Module Name: src Committed By: martin Date: Sun Mar 8 11:03:19 UTC 2020
Modified Files: src/sys/dev/acpi [netbsd-9]: acpi_pci_link.c Log Message: Pull up following revision(s) (requested by chs in ticket #765): sys/dev/acpi/acpi_pci_link.c: revision 1.25 apply FreeBSD revs r214848 and r214849: r214849 | jkim | 2010-11-05 13:24:26 -0700 (Fri, 05 Nov 2010) | 2 lines Add a forgotten change from the previous commit. r214848 | jkim | 2010-11-05 12:50:09 -0700 (Fri, 05 Nov 2010) | 13 lines Fix a use-after-free bug for extended IRQ resource[1]. When _PRS buffer is copied as a template for _SRS, a string pointer for descriptor name is also copied and it becomes stale as soon as it gets de-allocated[2]. Now _CRS is used as a template for _SRS as ACPI specification suggests if it is usable. The template from _PRS is still utilized but only when _CRS is not available or broken. To avoid use-after-free the problem in this case, however, only mandatory fields are copied, optional data is removed, and structure length is adjusted accordingly. Reported by: hps[1] Analyzed by: avg[2] Tested by: hps This also fixes reading past the end of a structure as detected by KASAN. To generate a diff of this commit: cvs rdiff -u -r1.22 -r1.22.26.1 src/sys/dev/acpi/acpi_pci_link.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Modified files: Index: src/sys/dev/acpi/acpi_pci_link.c diff -u src/sys/dev/acpi/acpi_pci_link.c:1.22 src/sys/dev/acpi/acpi_pci_link.c:1.22.26.1 --- src/sys/dev/acpi/acpi_pci_link.c:1.22 Sun Sep 14 19:54:05 2014 +++ src/sys/dev/acpi/acpi_pci_link.c Sun Mar 8 11:03:19 2020 @@ -1,4 +1,4 @@ -/* $NetBSD: acpi_pci_link.c,v 1.22 2014/09/14 19:54:05 mrg Exp $ */ +/* $NetBSD: acpi_pci_link.c,v 1.22.26.1 2020/03/08 11:03:19 martin Exp $ */ /*- * Copyright (c) 2002 Mitsuru IWASAKI <iwas...@jp.freebsd.org> @@ -27,7 +27,7 @@ */ #include <sys/cdefs.h> -__KERNEL_RCSID(0, "$NetBSD: acpi_pci_link.c,v 1.22 2014/09/14 19:54:05 mrg Exp $"); +__KERNEL_RCSID(0, "$NetBSD: acpi_pci_link.c,v 1.22.26.1 2020/03/08 11:03:19 martin Exp $"); #include <sys/param.h> #include <sys/malloc.h> @@ -255,6 +255,7 @@ link_add_crs(ACPI_RESOURCE *res, void *c static ACPI_STATUS link_add_prs(ACPI_RESOURCE *res, void *context) { + ACPI_RESOURCE *tmp; struct link_res_request *req; struct link *link; uint8_t *irqs = NULL; @@ -301,32 +302,28 @@ link_add_prs(ACPI_RESOURCE *res, void *c req->res_index++; /* - * Stash a copy of the resource for later use when - * doing _SRS. - * - * Note that in theory res->Length may exceed the size - * of ACPI_RESOURCE, due to variable length lists in - * subtypes. However, all uses of l_prs_template only - * rely on lists lengths of zero or one, for which - * sizeof(ACPI_RESOURCE) is sufficient space anyway. - * We cannot read longer than Length bytes, in case we - * read off the end of mapped memory. So we read - * whichever length is shortest, Length or - * sizeof(ACPI_RESOURCE). + * Stash a copy of the resource for later use when doing + * _SRS. */ - KASSERT(res->Length >= ACPI_RS_SIZE_MIN); + tmp = &link->l_prs_template; + if (is_ext_irq) { + memcpy(tmp, res, ACPI_RS_SIZE(tmp->Data.ExtendedIrq)); - memset(&link->l_prs_template, 0, sizeof(link->l_prs_template)); - memcpy(&link->l_prs_template, res, - MIN(res->Length, sizeof(link->l_prs_template))); + /* + * XXX acpi_AppendBufferResource() cannot handle + * optional data. + */ + memset(&tmp->Data.ExtendedIrq.ResourceSource, 0, + sizeof(tmp->Data.ExtendedIrq.ResourceSource)); + tmp->Length = ACPI_RS_SIZE(tmp->Data.ExtendedIrq); - if (is_ext_irq) { link->l_num_irqs = res->Data.ExtendedIrq.InterruptCount; link->l_trig = res->Data.ExtendedIrq.Triggering; link->l_pol = res->Data.ExtendedIrq.Polarity; ext_irqs = res->Data.ExtendedIrq.Interrupts; } else { + memcpy(tmp, res, ACPI_RS_SIZE(tmp->Data.Irq)); link->l_num_irqs = res->Data.Irq.InterruptCount; link->l_trig = res->Data.Irq.Triggering; link->l_pol = res->Data.Irq.Polarity; @@ -737,17 +734,16 @@ acpi_pci_link_add_reference(void *v, int static ACPI_STATUS acpi_pci_link_srs_from_crs(struct acpi_pci_link_softc *sc, ACPI_BUFFER *srsbuf) { - ACPI_RESOURCE *resource, *end, newres, *resptr; - ACPI_BUFFER crsbuf; + ACPI_RESOURCE *end, *res; ACPI_STATUS status; struct link *link; int i, in_dpf; /* Fetch the _CRS. */ - crsbuf.Pointer = NULL; - crsbuf.Length = ACPI_ALLOCATE_LOCAL_BUFFER; - status = AcpiGetCurrentResources(sc->pl_handle, &crsbuf); - if (ACPI_SUCCESS(status) && crsbuf.Pointer == NULL) + srsbuf->Pointer = NULL; + srsbuf->Length = ACPI_ALLOCATE_BUFFER; + status = AcpiGetCurrentResources(sc->pl_handle, srsbuf); + if (ACPI_SUCCESS(status) && srsbuf->Pointer == NULL) status = AE_NO_MEMORY; if (ACPI_FAILURE(status)) { aprint_verbose("%s: Unable to fetch current resources: %s\n", @@ -756,14 +752,13 @@ acpi_pci_link_srs_from_crs(struct acpi_p } /* Fill in IRQ resources via link structures. */ - srsbuf->Pointer = NULL; link = sc->pl_links; i = 0; in_dpf = DPF_OUTSIDE; - resource = (ACPI_RESOURCE *)crsbuf.Pointer; - end = (ACPI_RESOURCE *)((char *)crsbuf.Pointer + crsbuf.Length); + res = (ACPI_RESOURCE *)srsbuf->Pointer; + end = (ACPI_RESOURCE *)((char *)srsbuf->Pointer + srsbuf->Length); for (;;) { - switch (resource->Type) { + switch (res->Type) { case ACPI_RESOURCE_TYPE_START_DEPENDENT: switch (in_dpf) { case DPF_OUTSIDE: @@ -777,64 +772,44 @@ acpi_pci_link_srs_from_crs(struct acpi_p __func__); break; } - resptr = NULL; break; case ACPI_RESOURCE_TYPE_END_DEPENDENT: /* We are finished with DPF parsing. */ KASSERT(in_dpf != DPF_OUTSIDE); in_dpf = DPF_OUTSIDE; - resptr = NULL; break; case ACPI_RESOURCE_TYPE_IRQ: - newres = link->l_prs_template; - resptr = &newres; - resptr->Data.Irq.InterruptCount = 1; + res->Data.Irq.InterruptCount = 1; if (PCI_INTERRUPT_VALID(link->l_irq)) { KASSERT(link->l_irq < NUM_ISA_INTERRUPTS); - resptr->Data.Irq.Interrupts[0] = link->l_irq; - resptr->Data.Irq.Triggering = link->l_trig; - resptr->Data.Irq.Polarity = link->l_pol; + res->Data.Irq.Interrupts[0] = link->l_irq; + res->Data.Irq.Triggering = link->l_trig; + res->Data.Irq.Polarity = link->l_pol; } else - resptr->Data.Irq.Interrupts[0] = 0; + res->Data.Irq.Interrupts[0] = 0; link++; i++; break; case ACPI_RESOURCE_TYPE_EXTENDED_IRQ: - newres = link->l_prs_template; - resptr = &newres; - resptr->Data.ExtendedIrq.InterruptCount = 1; + res->Data.ExtendedIrq.InterruptCount = 1; if (PCI_INTERRUPT_VALID(link->l_irq)) { - resptr->Data.ExtendedIrq.Interrupts[0] = + res->Data.ExtendedIrq.Interrupts[0] = link->l_irq; - resptr->Data.ExtendedIrq.Triggering = + res->Data.ExtendedIrq.Triggering = link->l_trig; - resptr->Data.ExtendedIrq.Polarity = link->l_pol; + res->Data.ExtendedIrq.Polarity = link->l_pol; } else - resptr->Data.ExtendedIrq.Interrupts[0] = 0; + res->Data.ExtendedIrq.Interrupts[0] = 0; link++; i++; break; - default: - resptr = resource; - } - if (resptr != NULL) { - status = acpi_AppendBufferResource(srsbuf, resptr); - if (ACPI_FAILURE(status)) { - printf("%s: Unable to build resources: %s\n", - sc->pl_name, AcpiFormatException(status)); - if (srsbuf->Pointer != NULL) - ACPI_FREE(srsbuf->Pointer); - ACPI_FREE(crsbuf.Pointer); - return (status); - } } - if (resource->Type == ACPI_RESOURCE_TYPE_END_TAG) + if (res->Type == ACPI_RESOURCE_TYPE_END_TAG) break; - resource = ACPI_NEXT_RESOURCE(resource); - if (resource >= end) + res = ACPI_NEXT_RESOURCE(res); + if (res >= end) break; } - ACPI_FREE(crsbuf.Pointer); return (AE_OK); } @@ -854,10 +829,11 @@ acpi_pci_link_srs_from_links(struct acpi /* Add a new IRQ resource from each link. */ link = &sc->pl_links[i]; - newres = link->l_prs_template; - if (newres.Type == ACPI_RESOURCE_TYPE_IRQ) { + if (link->l_prs_template.Type == ACPI_RESOURCE_TYPE_IRQ) { /* Build an IRQ resource. */ + bcopy(&link->l_prs_template, &newres, + ACPI_RS_SIZE(newres.Data.Irq)); newres.Data.Irq.InterruptCount = 1; if (PCI_INTERRUPT_VALID(link->l_irq)) { KASSERT(link->l_irq < NUM_ISA_INTERRUPTS); @@ -869,6 +845,8 @@ acpi_pci_link_srs_from_links(struct acpi } else { /* Build an ExtIRQ resuorce. */ + bcopy(&link->l_prs_template, &newres, + ACPI_RS_SIZE(newres.Data.ExtendedIrq)); newres.Data.ExtendedIrq.InterruptCount = 1; if (PCI_INTERRUPT_VALID(link->l_irq)) { newres.Data.ExtendedIrq.Interrupts[0] =