Hi,
Let me update "diff #2".
On Fri, 26 Feb 2021 13:42:32 +0900 (JST)
YASUOKA Masahiko <yasu...@openbsd.org> wrote:
> My vaio repeatedly crashed by "Data modified on freelist"(*1) or other
> memory corruptions. After my long time debug, I found the route cause
> is a handling of references of LocalX, like the following:
>
> If ((SMRW (0x0B, 0x16, 0x21, RefOf (Local0)) == Zero))
>
> In the called control method, "RefOf (Local1)" is referred as Arg3, is
> stored a value like the following:
>
> Arg3 = \_SB.PCI0.LPCB.EC0.SMD0
>
> In aml_store(), lvalue is reset if lvalue is a LocalX. But since that
> was done before resolving the reference, lvalue was not reset if
> lvalue is a reference of LocalX.
>
> diff #1 fixes that problem. It resets lvalue after resolving
> references.
>
> ok?
>
> diff #2 adds aml_die() if any memory corruption occurs when creating
> field in a buffer. This actually happens on my vaio (pro pk 14) if
> diff #1 is not applied.
>
> ok?
>
> diff #1
>
> Index: sys/dev/acpi/dsdt.c
> ===================================================================
> RCS file: /var/cvs/openbsd/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.257
> diff -u -p -r1.257 dsdt.c
> --- sys/dev/acpi/dsdt.c 17 Dec 2020 17:57:19 -0000 1.257
> +++ sys/dev/acpi/dsdt.c 26 Feb 2021 04:12:03 -0000
> @@ -2961,11 +2961,11 @@ aml_store(struct aml_scope *scope, struc
> aml_rwfield(rhs, 0, rhs->v_field.bitlen, &tmp, ACPI_IOREAD);
> rhs = &tmp;
> }
> +
> + lhs = aml_gettgt(lhs, AMLOP_STORE);
> /* Store to LocalX: free value */
> if (lhs->stack >= AMLOP_LOCAL0 && lhs->stack <= AMLOP_LOCAL7)
> aml_freevalue(lhs);
> -
> - lhs = aml_gettgt(lhs, AMLOP_STORE);
> switch (lhs->type) {
> case AML_OBJTYPE_UNINITIALIZED:
> aml_copyvalue(lhs, rhs);
>
> diff #2
>
> Index: sys/dev/acpi/dsdt.c
> ===================================================================
> RCS file: /var/cvs/openbsd/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.257
> diff -u -p -r1.257 dsdt.c
> --- sys/dev/acpi/dsdt.c 17 Dec 2020 17:57:19 -0000 1.257
> +++ sys/dev/acpi/dsdt.c 26 Feb 2021 04:33:21 -0000
> @@ -2742,11 +2742,17 @@ aml_rwfield(struct aml_value *fld, int b
> } else if (mode == ACPI_IOREAD) {
> /* bufferfield:read */
> _aml_setvalue(val, AML_OBJTYPE_INTEGER, 0, 0);
> + if (ref1->length < aml_bytepos(fld->v_field.bitpos) +
> + aml_bytelen(fld->v_field.bitlen))
> + aml_die("bufferfield:read out of range");
> aml_bufcpy(&val->v_integer, 0, ref1->v_buffer,
> fld->v_field.bitpos, fld->v_field.bitlen);
> } else {
> /* bufferfield:write */
> val = aml_convert(val, AML_OBJTYPE_INTEGER, -1);
> + if (ref1->length < aml_bytepos(fld->v_field.bitpos) +
> + aml_bytelen(fld->v_field.bitlen))
> + aml_die("bufferfield:write out of range");
> aml_bufcpy(ref1->v_buffer, fld->v_field.bitpos, &val->v_integer,
> 0, fld->v_field.bitlen);
> aml_delref(&val, "wrbuffld");
It's better to die when creating a field which refers out of range
memory.
ok?
Index: sys/dev/acpi/dsdt.c
===================================================================
RCS file: /disk/cvs/openbsd/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.257
diff -u -p -r1.257 dsdt.c
--- sys/dev/acpi/dsdt.c 17 Dec 2020 17:57:19 -0000 1.257
+++ sys/dev/acpi/dsdt.c 27 Feb 2021 09:58:31 -0000
@@ -2790,6 +2790,11 @@ aml_createfield(struct aml_value *field,
data->type != AML_OBJTYPE_BUFFER)
data = aml_convert(data, AML_OBJTYPE_BUFFER, -1);
+ if (field->type == AML_OBJTYPE_BUFFERFIELD &&
+ data->length < aml_bytepos(bpos) + aml_bytelen(blen))
+ aml_die("%s(%s) out of range\n", aml_mnem(opcode, 0),
+ aml_nodename(field->node));
+
field->v_field.type = opcode;
field->v_field.bitpos = bpos;
field->v_field.bitlen = blen;
