[libvirt] [PATCH] virNetworkDefUpdateIPDHCPHost: Don't crash when updating network
https://bugzilla.redhat.com/show_bug.cgi?id=1182486 When updating a network and adding new ip-dhcp-host entry, the deamon may crash. The problem is, we iterate over existing <host/> entries trying to compare MAC addresses to see if there's already an existing rule. However, not all entries are required to have MAC address. For instance, the following is perfectly valid entry: <host id='00:04:58:fd:e4:15:1b:09:4c:0e:09:af:e4:d3:8c:b8:ca:1e' name='redhatipv6.redhat.com' ip='2001:db8:ca2:2::119'/> When the checking loop iterates over this, the entry's MAC address is accessed directly. Well, the fix is obvious - check if the address is defined before trying to compare it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/network_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 26fe18d..3b719de 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3601,7 +3601,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def, /* log error if an entry with same name/address/ip already exists */ for (i = 0; i < ipdef->nhosts; i++) { - if ((host.mac && + if ((host.mac && ipdef->hosts[i].mac && !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) || (host.name && STREQ_NULLABLE(host.name, ipdef->hosts[i].name)) || -- 2.0.5
On 01/15/2015 09:48 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1182486
When updating a network and adding new ip-dhcp-host entry, the deamon may crash. The problem is, we iterate over existing <host/> entries trying to compare MAC addresses to see if there's already an existing rule. However, not all entries are required to have MAC address. For instance, the following is perfectly valid entry:
<host id='00:04:58:fd:e4:15:1b:09:4c:0e:09:af:e4:d3:8c:b8:ca:1e' name='redhatipv6.redhat.com' ip='2001:db8:ca2:2::119'/>
When the checking loop iterates over this, the entry's MAC address is accessed directly. Well, the fix is obvious - check if the address is defined before trying to compare it.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/network_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 26fe18d..3b719de 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3601,7 +3601,7 @@ virNetworkDefUpdateIPDHCPHost(virNetworkDefPtr def,
/* log error if an entry with same name/address/ip already exists */ for (i = 0; i < ipdef->nhosts; i++) { - if ((host.mac && + if ((host.mac && ipdef->hosts[i].mac && !virMacAddrCompare(host.mac, ipdef->hosts[i].mac)) || (host.name && STREQ_NULLABLE(host.name, ipdef->hosts[i].name)) ||
This is good as far as it goes, but there are 2 other places in the function that do the same thing (call virMacAddrCompare() without validating both mac pointers first) - one for MODIFY and one for DELETE in addition to the one you're fixing. ACK with that fix. (Beyond that, the test used in the bug report points out that we are allowing users to add an IPv6 static host definition to the host table for an IPv4 address. We should probably not allow that (although doing so at least doesn't cause a crash). We should do a separate patch to check that the family of the desired ipdef in the network matches the family of the ip in the provided host definition, and log an error if it doesn't.)
participants (2)
-
Laine Stump -
Michal Privoznik