Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev/acpi apply FreeBSD revs r214848 and r214849:
details: https://anonhg.NetBSD.org/src/rev/a57e91a19fb0
branches: trunk
changeset: 467085:a57e91a19fb0
user: chs <chs%NetBSD.org@localhost>
date: Mon Jan 13 00:19:43 2020 +0000
description:
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.
diffstat:
sys/dev/acpi/acpi_pci_link.c | 106 +++++++++++++++++-------------------------
1 files changed, 42 insertions(+), 64 deletions(-)
diffs (214 lines):
diff -r d8961cc2d15a -r a57e91a19fb0 sys/dev/acpi/acpi_pci_link.c
--- a/sys/dev/acpi/acpi_pci_link.c Mon Jan 13 00:09:28 2020 +0000
+++ b/sys/dev/acpi/acpi_pci_link.c Mon Jan 13 00:19:43 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: acpi_pci_link.c,v 1.24 2019/12/06 07:27:06 maxv Exp $ */
+/* $NetBSD: acpi_pci_link.c,v 1.25 2020/01/13 00:19:43 chs Exp $ */
/*-
* Copyright (c) 2002 Mitsuru IWASAKI <iwasaki%jp.freebsd.org@localhost>
@@ -27,7 +27,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: acpi_pci_link.c,v 1.24 2019/12/06 07:27:06 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: acpi_pci_link.c,v 1.25 2020/01/13 00:19:43 chs Exp $");
#include <sys/param.h>
#include <sys/malloc.h>
@@ -255,6 +255,7 @@
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 @@
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 @@
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 @@
}
/* 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 @@
__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 @@
/* 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 @@
} 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] =
Home |
Main Index |
Thread Index |
Old Index