> Date: Sat, 27 Feb 2021 19:43:31 +0900 (JST)
> From: YASUOKA Masahiko <yasu...@openbsd.org>
> Content-Type: Text/Plain; charset=us-ascii
>
> 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 #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?
I'm not 100% confident about this one. The check seems correct to me,
but there is too much broken AML out there...
Maybe Theo can put this one in snaps for a bit?
> 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;
>
>
