Re: [PATCH] hw/acpi/erst.c: Fix memset argument order

[Alexander Bulekov]

On 221021 1505, Alexander Bulekov wrote:
> On 221019 2115, Christian A. Ehrhardt wrote:
> > Fix memset argument order: The second argument is
> > the value, the length goes last.
> > 
> > Cc: Eric DeVolder <eric.devolder@oracle.com>
> > Cc: qemu-stable@nongnu.org
> > Fixes: f7e26ffa590 ("ACPI ERST: support for ACPI ERST feature")
> > Signed-off-by: Christian A. Ehrhardt <lk@c--e.de>
> > ---
> >  hw/acpi/erst.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/acpi/erst.c b/hw/acpi/erst.c
> > index df856b2669..26391f93ca 100644
> > --- a/hw/acpi/erst.c
> > +++ b/hw/acpi/erst.c
> > @@ -716,7 +716,7 @@ static unsigned write_erst_record(ERSTDeviceState *s)
> >      if (nvram) {
> >          /* Write the record into the slot */
> >          memcpy(nvram, exchange, record_length);
> > -        memset(nvram + record_length, exchange_length - record_length, 
> > 0xFF);
> > +        memset(nvram + record_length, 0xFF, exchange_length - 
> > record_length);
> 
> Hi, 
> I'm running the fuzzer over this code. So far, it hasn't complained
> about this particular memset call, but it has crashed on the memcpy,
> directly preceding it. I think the record_length checks might be
> insufficient. I made an issue/reproducer:
> https://gitlab.com/qemu-project/qemu/-/issues/1268
> 
> In that particular case, record_length is ffffff00 and passes the
> checks. I'll keep an eye on the fuzzer to see if it will be able to
> crash the memset.

Here's a testcase for the memset issue:

cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
512M -object memory-backend-ram,id=erstnvram,size=0x10000 -device \
acpi-erst,memdev=erstnvram -nodefaults -qtest stdio
outl 0xcf8 0x80001010
outl 0xcfc 0xe0000000
outl 0xcf8 0x80001014
outl 0xcfc 0xe0002000
outl 0xcf8 0x80001004
outw 0xcfc 0x02
write 0xe0000008 0x1 0x9c
write 0xe0002015 0x1 0x01
write 0xe0002066 0x1 0x02
write 0xe0000000 0x1 0x05
write 0xe0002066 0x1 0x04
write 0xe0000000 0x1 0x05
write 0xe0002066 0x1 0x01
write 0xe0000000 0x1 0x05
write 0xe0002065 0x1 0x01
write 0xe0000000 0x1 0x05
write 0xe0002066 0x1 0x02
write 0xe0000000 0x1 0x05
write 0xe0002066 0x1 0x03
write 0xe0000000 0x1 0x05
write 0xe0002015 0x1 0x20
write 0xe0002066 0x1 0x00
write 0xe0000000 0x1 0x05
EOF

> 
> For this patch:
> Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
> 
> >          /* If a new record, increment the record_count */
> >          if (!record_found) {
> >              uint32_t record_count;
> > -- 
> > 2.34.1
> > 
> >