search

Xilinx / qemu

Xilinx's fork of Quick EMUlator (QEMU) with improved support and modelling for the Xilinx platforms.
https://xilinx-wiki.atlassian.net/wiki/spaces/A/pages/821395464/QEMU+User+Documentation
Other
238 stars 152 forks source link

Missing Terminator (Null Character) in MDIO Slave Interface #37

Closed poisedmilk closed 4 years ago

poisedmilk commented 4 years ago

When the qom is initialized, it seems to go through each type (in this case the MDIO Slave Interface) and in qom/object.c::type_new() it assigns each info->interfaces[i].type to ti->interfaces[i].typename , so long as the interfaces and type for the info object are not NULL.

Without this NULL character included in the interfaces for the MDIO Slave Interface, the source code will compile without any errors but will segmentation fault when executed on an ARM processor. I used GDB to run the aarch64-softmmu binary after compiling it and traced the source of the seg fault back to the creation of the mdio slave interface type.

After passing in the expected TYPE_FDT_GENERIC_MMAP it did not return from type_new() but instead passed in unexpected values leading to a memory access violation when the strdup in type_new() was attempting to assign the value of info->interfaces[i].type to ti->interfaces[i].typename.

Ending interfaces with a null character seems to be the general convention of other types within the qemu source code, and after adding and building the binary did not seg fault on ARM. It is worth noting that I attempted this on an x86 processor and never saw any seg faults related to this, and I assume it must be some nuance of the ARMv8 architecture.

edgarigl commented 4 years ago

Thanks Dylan,

This patch looks good, I've queued it for merge after it passes our testsuite. Next time you submit a patch, it would be great if you could add two things to the commit message.

  1. If you commit with the -s, git will automatically add a Signed-off-by line to the commit. You can fix up commits after the fact by doing git commit --amend -s, for example.
  2. The first line of the commit message is a short summary of the patch. It's useful for example when doing things like git log --oneline. It's very useful to include a prefix in the summary, something along the lines of "mdio: Add NULL terminator to interface array".

I've added both of these to the commit, so you don't need to resubmit a new pull req. Just something to keep in mind for next time.

Thanks, Edgar

edgarigl commented 4 years ago

Merged.