[PATCH 0/7] Address some Coverity found issues

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The following Coverity issues are flagged in Coverity 7.6.1 compared
to not being seen in 7.5.1 - this series just addresses those issues
before 7.6.1 is installed in our upstream Jenkins environment:

http://jenkins-virt.usersys.redhat.com/job/libvirt-coverity/

It seems the bulk are essentially false positives, but we can do something
in order to work around them.

For the first 3 patches, Coverity complains that virVasprintfInternal
can set "*strp = NULL;" on return (in the error path), but the callers
don't associate the error path (< 0) with the 'buffer' and thus believe
there could be some path where a return 0 would occur. This would result
in the callers of the virPCIFile, virPCIDriverFile, and virPCIDriverDir
having an error. Rather than try to add sa_assert()'s for each, adjust
the calls and callers to return the buffer

For the fourth patch, the strtok_r call expects that the first call will
have a non-NULL value for the first parameter and subsequent calls over
the same string would pass a NULL value in the first parameter and a
non-NULL parameter in the third parameter to be used as the "point" in
the string to continue looking for the token.  However, Coverity doesn't
realize this and thus flags the strtok_r call with a possible forward
NULL dereference because we initialize the third parameter to NULL.
By adding an 'sa_assert' for what is the first argument lets Coverity
know we know what we're doing.  Additionally, VIR_STRDUP could return a
NULL string and a zero return, the assertion in openvz_conf.c is on
the passed value to be scanned.

The fifth patch is perhaps the only one fixing a real issue; however,
it's been around since 1.2.11, so perhaps it's a less traveled path.

The sixth patch seems to be a false positive, but since Coverity doesn't
make the connection between nstack and stack, it's just as safe to
add the protection in the Free routine against a strange path being hit.

For the last patch, this is a false positive, but it also shows up in
the 7.5.1 coverity build:

http://jenkins-virt.usersys.redhat.com/job/libvirt-coverity/128/

Essentially the issue is that virResizeN can "return 0;" prior to
the virExpandN and that causes Coverity some angst since it would
then be possible to not have anything returned in "*ret" which would
cause a NULL dereference. Stepping through the logic has no path
in which that could happen. In order to handle this, rather than
use VIR_RESIZE_N for 'n' times through the loop, let's first count
the matches, use VIR_ALLOC_N, and then rerun the same loop filling
in each address as before.  The difference being 'n' VIR_RESIZE_N
versus 2*'n' STREQ calls - perhaps a small wash performance wise.

John Ferlan (7):
  util: Resolve Coverity FORWARD_NULL
  util: Resolve Coverity FORWARD_NULL
  util: Resolve Coverity FORWARD_NULL
  Avoid Coverity FORWARD_NULL prior to strtok_r calls
  phyp: Resolve Coverity FORWARD_NULL
  util: Resolve Coverity FORWARD_NULL
  util: Avoid Coverity FORWARD_NULL

 src/esx/esx_vi.c          |  1 +
 src/libxl/libxl_conf.c    |  1 +
 src/openvz/openvz_conf.c  |  2 ++
 src/phyp/phyp_driver.c    |  3 +--
 src/util/virdbus.c        |  4 +++
 src/util/virpci.c         | 67 ++++++++++++++++++++++++-----------------------
 src/util/virtypedparam.c  | 14 ++++++----
 src/xenapi/xenapi_utils.c |  1 +
 8 files changed, 53 insertions(+), 40 deletions(-)

-- 
2.1.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]