Bug 749531 - Coverity scan revealed defects
Summary: Coverity scan revealed defects
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: libvirt
Version: 6.1
Hardware: x86_64
OS: Linux
urgent
urgent
Target Milestone: rc
: ---
Assignee: Eric Blake
QA Contact: Virtualization Bugs
URL:
Whiteboard:
Depends On: 747516
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-10-27 11:29 UTC by RHEL Program Management
Modified: 2011-11-09 10:14 UTC (History)
10 users (show)

Fixed In Version: libvirt-0.8.7-18.el6_1.4
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-11-07 13:48:56 UTC
Target Upstream Version:


Attachments (Terms of Use)
CoverityScan defect reports (33.23 KB, application/octet-stream)
2011-11-03 08:57 UTC, Alex Jia
no flags Details
CoverityScan defect reports (33.23 KB, text/plain)
2011-11-03 09:07 UTC, Alex Jia
no flags Details


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2011:1431 0 normal SHIPPED_LIVE libvirt bug fix update 2011-11-07 18:48:41 UTC

Description RHEL Program Management 2011-10-27 11:29:32 UTC
This bug has been copied from bug #747516 and has been proposed
to be backported to 6.1 z-stream (EUS).

Comment 4 Eric Blake 2011-10-27 14:21:10 UTC
Per bug 747516, I need to backport the following:

c24c07f
2ea9409
9892f7b
79052a7
34b999b

Comment 8 Alex Jia 2011-11-03 08:55:39 UTC
libvirt-0.8.7-18.el6_1.2 built without patches(run1):
Analysis summary report:
------------------------
Files analyzed                 : 172
Total LoC input to cov-analyze : 232552
Functions analyzed             : 5732
Paths analyzed                 : 597586
New defects found              : 53 Total
                                  8 CHECKED_RETURN
                                  2 CONSTANT_EXPRESSION_RESULT
                                  2 DEADCODE
                                  9 FORWARD_NULL
                                  7 MISSING_BREAK
                                  1 MISSING_RETURN
                                  1 NEGATIVE_RETURNS
                                  1 NULL_RETURNS
                                  1 OVERRUN_STATIC
                                  9 RESOURCE_LEAK
                                  1 RETURN_LOCAL
                                  2 REVERSE_INULL
                                  2 SIGN_EXTENSION
                                  2 UNINIT
                                  2 UNUSED_VALUE
                                  3 USE_AFTER_FREE

libvirt-0.8.7-18.el6_1.4 built with patches(run1):

Analysis summary report:
------------------------
Files analyzed                 : 172
Total LoC input to cov-analyze : 232227
Functions analyzed             : 5731
Paths analyzed                 : 597462
New defects found              : 48 Total
                                  8 CHECKED_RETURN
                                  2 CONSTANT_EXPRESSION_RESULT
                                  2 DEADCODE
                                  8 FORWARD_NULL
                                  7 MISSING_BREAK
                                  1 MISSING_RETURN
                                  1 NEGATIVE_RETURNS
                                  1 NULL_RETURNS
                                  1 OVERRUN_STATIC
                                  6 RESOURCE_LEAK
                                  1 RETURN_LOCAL
                                  2 REVERSE_INULL
                                  2 SIGN_EXTENSION
                                  2 UNINIT
                                  2 UNUSED_VALUE
                                  2 USE_AFTER_FREE

There are 5 issues have been fixed by comparing el6_1.2 with el6_1.4:
1 FORWARD_NULL
3 RESOURCE_LEAK
1 USE_AFTER_FREE


The rest of issues:

8 CHECKED_RETURN             (False positive)
2 CONSTANT_EXPRESSION_RESULT (No point backporting)
2 DEADCODE         (Harmless dead code)
8 FORWARD_NULL     (False positive, Not worth backporting 98cd17b)
7 MISSING_BREAK    (False positives)
1 MISSING_RETURN   (Not libvirt's fault)
1 NEGATIVE_RETURNS (Not worth backporting 89e651f)
1 NULL_RETURNS   (Not worth a backport)
1 OVERRUN_STATIC (Not worth backporting 59953c3)
6 RESOURCE_LEAK  (VMX is not part of RHEL, Not worth backporting bb88952...?)
1 RETURN_LOCAL   (False positive)
2 REVERSE_INULL  (Not worth backporting 1518042, d69b79a)
2 SIGN_EXTENSION (Not worth backporting f73198d, 54456cc)
2 UNINIT         (LXC is tech preview, Might be worth backporting)
2 UNUSED_VALUE   (Harmless dead assignment)
2 USE_AFTER_FREE (False positive)


Hi Eric, please confirm these, and is't okay for RESOURCE_LEAK? 

Thanks,
Alex

Comment 9 Alex Jia 2011-11-03 08:57:31 UTC
Created attachment 531516 [details]
CoverityScan defect reports

Comment 10 Alex Jia 2011-11-03 09:07:08 UTC
Created attachment 531518 [details]
CoverityScan defect reports

This one is correct, I can't change content type for previous attachment, it's a buzilla issue, so please ignore the first attachment, thanks.

Comment 11 Eric Blake 2011-11-03 14:17:37 UTC
From the original report:

> Error: RESOURCE_LEAK:
> /builddir/build/BUILD/libvirt-0.8.7/src/qemu/qemu_command.c:5637: alloc_arg:
> Calling allocation function "virAlloc" on "def".
> 
> Real bugs (two instances this function), can be triggered by
> domxml-from-native, although that is not called frequently.  Might be worth
> backporting 2ea9409, depending on the anticipated call rate.

From the rebuild:

Error: RESOURCE_LEAK:
/builddir/build/BUILD/libvirt-0.8.7/src/qemu/qemu_command.c:5638: alloc_arg: Calling allocation function "virAlloc" on "def".
/builddir/build/BUILD/libvirt-0.8.7/src/util/memory.c:102: alloc_fn: Storage is returned from allocation function "calloc".
/builddir/build/BUILD/libvirt-0.8.7/src/util/memory.c:102: var_assign: Assigning: "*((void **)ptrptr)" = "calloc(1UL, size)".
/builddir/build/BUILD/libvirt-0.8.7/src/qemu/qemu_command.c:5836: overwrite_var: Overwriting "def" in call "def = NULL" leaks the storage that "def" points to.

Aargh - the backport of 2ea9409, while fixing a real leak, was insufficient to fix the original detected problem.  Thankfully, though, the fix for the detected leak is upstream commit db3b32c, which documents that the leak is only possible on invalid input, and therefore unlikely to hit 6.1.z.  Which means:

Yes, I'm happy that we have solved everything that is necessary for 6.1.z, and this bug can be moved to verified; all remaining issues are not show-stoppers and therefore not worth fixing in z-stream.

Comment 12 Alex Jia 2011-11-03 14:44:31 UTC
(In reply to comment #8)
> libvirt-0.8.7-18.el6_1.2 built without patches(run1):
s/without/with/.

Comment 13 Alex Jia 2011-11-03 14:46:07 UTC
(In reply to comment #11)
> Aargh - the backport of 2ea9409, while fixing a real leak, was insufficient to
> fix the original detected problem.  Thankfully, though, the fix for the
> detected leak is upstream commit db3b32c, which documents that the leak is only
> possible on invalid input, and therefore unlikely to hit 6.1.z.  Which means:
> 
> Yes, I'm happy that we have solved everything that is necessary for 6.1.z, and
> this bug can be moved to verified; all remaining issues are not show-stoppers
> and therefore not worth fixing in z-stream.

Eric, thanks a lot, move the bug to VERIFIED status based on latest test result and Comment 12.

Comment 14 Alex Jia 2011-11-03 14:48:02 UTC
(In reply to comment #13)
> Eric, thanks a lot, move the bug to VERIFIED status based on latest test result
> and Comment 12.

Sorry, it should be Comment 11.

Comment 15 errata-xmlrpc 2011-11-07 13:48:56 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2011-1431.html


Note You need to log in before you can comment on or make changes to this bug.