Bug 717926

Since RHEL 6.2 External Beta has begun, and this bug remains
unresolved, it has been rejected as it is not proposed as
exception or blocker.

Red Hat invites you to ask your support representative to
propose this request, if appropriate and relevant, in the
next release of Red Hat Enterprise Linux.

Created attachment 574855 [details]
New defects in NetworkManager

Hello,

the new Coverity scan revealed some new potential bugs in
NetworkManager-0.8.1-27.el6 against NetworkManager-0.8.1-24 (list of added
defects is attached).  I have tried to filter out false positives and the
following bugs seems to be real:

    1. Unchecked return value (low prio)

       NetworkManager-0.8.1/src/nm-system.c:2009

       I'm really not sure here.  This potential bug is added by
       rh712302-vlan.patch and I don't know if it may cause real problems.

    2. Interchange of '~' and '!' operators!

       NetworkManager-0.8.1/libnm-util/nm-setting-vlan.c:486

    3. Missing else branch.

       NetworkManager-0.8.1/system-settings/plugins/ifcfg-rh/reader.c:3699

       If the call 'g_str_has_prefix (iface_name, "vlan")' returns false
       variable 'p' stays NULL and on line 3702 is assigned to 'w' which is
       immediately dereferenced.

    4. Signed / unsigned int problem.

       NetworkManager-0.8.1/src/nm-device-vlan.c:640

       .. on line 638 is done assignment from signed => unsigned -- that may
       cause problems when the variable 'vlan_id' was really negative.  The
       check for negative value on line 642 may be never satisfied..

    5. 'strcpy' from rtnl_link_get_name() return value into
       vlan_ioctl_args.device1.  I'm just not sure if this is completely safe..
       Could somebody more involved look at this bunch of warnings?

       Occurrences:
             NetworkManager-0.8.1/src/nm-netlink-compat.c:193
             NetworkManager-0.8.1/src/nm-netlink-compat.c:124
             NetworkManager-0.8.1/src/nm-netlink-compat.c:161
             NetworkManager-0.8.1/src/nm-netlink-compat.c:226
             NetworkManager-0.8.1/src/nm-system.c:1931
             NetworkManager-0.8.1/src/nm-system.c:1931
             NetworkManager-0.8.1/src/nm-system.c:1905

This report is related to Michal's warning -- this bug is still in
NetworkManager.

Devel:
 Those errors are mentioned just as a warning and it depends on you whether
 they will be fixed.  They may be not so dangerous and not so prioritized to
 fix in  6.3 so feel free to move it to 6.4 or close it as a NOTABUG if we
 don't need to fix these at all.

Quality engineering:
 This issues were found by static analysis tool and we can't provide any
 reproducer for these.  We will verify the fix once available.  Please check
 these tests as SanityOnly (just check that patches for the issues and nothing
 unexpected is added by the commit).  If you want to check the new package with
 Coverity yourself, feel free to use covscan tool
 (https://engineering.redhat.com/trac/CoverityScan/wiki/covscan).

>     1. Unchecked return value (low prio)
> 
>        NetworkManager-0.8.1/src/nm-system.c:2009

I'm not sure what it's complaining about. Or maybe the patch has already been updated since the version you ran coverity against? Right now that line is:

    for (i = 0; i < num; i++) {
-->     if (nm_setting_vlan_get_priority (s_vlan, NM_VLAN_INGRESS_MAP, i, &from, &to))
            if (rtnl_link_vlan_set_ingress_map (new_link, from, to))
                goto err_out_delete_vlan_with_new_name;
    }

so I don't see what it thinks we're not checking...

>>     1. Unchecked return value (low prio)
>>
>>        NetworkManager-0.8.1/src/nm-system.c:2009
>
> I'm not sure what it's complaining about. Or maybe the patch has already been
> updated since the version you ran coverity against? Right now that line is:
>
>     for (i = 0; i < num; i++) {
> -->     if (nm_setting_vlan_get_priority (s_vlan, NM_VLAN_INGRESS_MAP, i,
>                   &from, &to))
>             if (rtnl_link_vlan_set_ingress_map (new_link, from, to))
>                 goto err_out_delete_vlan_with_new_name;
>     }
>
> so I don't see what it thinks we're not checking...

Hi Dan, I have accidentally copied&pasted bad source filename and the line -->
the correct one is this one:

    NetworkManager-0.8.1/cli/src/settings.c:572

Sorry for my mistake,
Pavel

These are mostly pretty simple fixes, and clearly correct, and could easily be fixed in 6.3.

(In reply to comment #0)
> /network-manager-applet-0.8.1/src/applet-dialogs.c:546 - Potential
> uninitialized value using. 

was already fixed upstream

(In reply to comment #3)

>     1. Unchecked return value (low prio)
> 
>        NetworkManager-0.8.1/cli/src/settings.c:572

Now fixed upstream

>     2. Interchange of '~' and '!' operators!
> 
>        NetworkManager-0.8.1/libnm-util/nm-setting-vlan.c:486

Oof. Fixed upstream

>     3. Missing else branch.
> 
>        NetworkManager-0.8.1/system-settings/plugins/ifcfg-rh/reader.c:3699

There are a few other bugs nearby in this code (eg, "DEVICE=vlanx" will make it spin forever because it's not incrementing w). I'm not totally sure what sorts of inputs it intends to handle vs what it intends to consider errors, so I'll leave this until dcbw gets back.

>     4. Signed / unsigned int problem.
> 
>        NetworkManager-0.8.1/src/nm-device-vlan.c:640

Fixed upstream.

>     5. 'strcpy' from rtnl_link_get_name() return value into
>        vlan_ioctl_args.device1.  I'm just not sure if this is completely safe..

It is safe, although using strcpy() makes me uncomfortable anyway. But I'll leave these to dcbw too.

This request was evaluated by Red Hat Product Management for inclusion
in a Red Hat Enterprise Linux maintenance release. Product Management has 
requested further review of this request by Red Hat Engineering, for potential
inclusion in a Red Hat Enterprise Linux Update release for currently deployed 
products. This request is not yet committed for inclusion in an Update release.

I've got a patch for this, but we need exception+ now.

The patch is small, and fixes some clear bugs in the vlan code.

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-2012-0832.html