On 29.10.18 11:51, Peter Maydell wrote: > Hi; could somebody who understands the block layer refcounting have > a look at Coverity issues CID 1395870 and 1395871, please?
Don't forget 1395869. > In both > cases, Coverity reports a use-after-free because it thinks that a > sequence where a code path might (conditionally) end up calling > blk_deref() twice could be freeing the memory in the first call > and using it after. I'm not sure whether these are false positives > because the refcounting has confused Coverity, or genuine issues where > we have got refcounting logic wrong, so I don't know if we need a > fix or if we should squash the coverity bug as a false-positive... It looks basically reasonable to me (just like 1395869). All of these block devices have two refcounts, one from the device state, and one from the monitor. These three places drop both refcounts after one another. On first glance I found the order in qdev-properties-system.c a bit weird because it unrefs the monitor reference first (which is definitely there), and the device state reference only afterwards (which piix.c implies may or may not be there). However, @dev cannot be NULL (otherwise "*ptr" would have segfaulted), so the device state reference is guaranteed to be there. OTOH, it appears that in this case the monitor reference may be missing, so it's correct to try to drop that reference first here, in case there is none. So all looks good to me, I'll mark them as false positives (like Paolo has done for 1395869 already). Max
signature.asc
Description: OpenPGP digital signature