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.devol...@oracle.com> > > Cc: qemu-sta...@nongnu.org > > Fixes: f7e26ffa590 ("ACPI ERST: support for ACPI ERST feature") > > Signed-off-by: Christian A. Ehrhardt <l...@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 <alx...@bu.edu> > > > /* If a new record, increment the record_count */ > > if (!record_found) { > > uint32_t record_count; > > -- > > 2.34.1 > > > >