On 6.2.2014 16:48, Eric Blake wrote:
On 02/06/2014 08:18 AM, Pavel Hrdina wrote:
The coverity server complains about the dev->stubDriver that it was freed by the virPCIDeviceSetStubDriver function, but it somehow don't get the fact that into the variable is immediately assigned new string.
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- tests/virpcitest.c | 2 ++ 1 file changed, 2 insertions(+)
What's the exact complaint?
The first one that coverity don't get that after free the new string is assigned into the dev->stubDriver: -------------------------------------------------------------------- Event freed_arg: "virPCIDeviceSetStubDriver" frees "dev->stubDriver". -------------------------------------------------------------------- This is the root cause for another complaint that the dev->stubDriver is dereferenced after free: -------------------------------------------------------------------- Event deref_arg: Calling "virPCIDeviceDetach" dereferences freed pointer "dev->stubDriver". --------------------------------------------------------------------
diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 994b300..8ff3b1d 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -248,6 +248,7 @@ testVirPCIDeviceDetachSingle(const void *opaque) if (!dev) goto cleanup;
+ /* coverity[freed_arg] */ if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0 || virPCIDeviceDetach(dev, NULL, NULL) < 0) goto cleanup;
Is this something where an sa_assert() could do the trick in a more tool-agnostic manner?
I'm looking at the code again and probably the issue found by coverity could happen if the STRDUP fails inside of the virPCIDeviceSetStubDriver. This is a corner case where it could leads to creating a wrong modprobe command because the dev->stubDriver also as driver and then as module wouldn't be included in the command line. -------------------------------------------------------------------- virPCIDeviceDetach()->virPCIProbeStubDriver()->virKModLoad()-> doModprobe() -------------------------------------------------------------------- For the virKmodLoad() function the coverity found this issue: -------------------------------------------------------------------- Event deref_parm_in_call: Function "virKModLoad" dereferences "driver". (The dereference is assumed on the basis of the 'nonnull' parameter attribute.) -------------------------------------------------------------------- I'm not sure what to do with this. At first I've thought that it's only the coverity mistake. Thanks for the review, Pavel