From 856e309d7b18cabe18c845bdf57e92958343a9ec Mon Sep 17 00:00:00 2001 From: Michael Chapman Date: Tue, 16 Aug 2016 19:07:42 +1000 Subject: [PATCH 1/3] networkd: avoid NULL pointer dereference in route_add If no result parameter is provided, do not attempt to write the found/newly-created route to it. This is presently not an issue as all callers currently provide a non-NULL result parameter, however we should do this for symmetry with address_add and future code robustness. --- src/network/networkd-route.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index cedaf47cf8e..82f9047ff61 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -322,7 +322,8 @@ int route_add( } else return r; - *ret = route; + if (ret) + *ret = route; return 0; } From 3bdccf69ca626d6c2f6653044f1f68d9c9da0e42 Mon Sep 17 00:00:00 2001 From: Michael Chapman Date: Thu, 18 Aug 2016 17:46:21 +1000 Subject: [PATCH 2/3] networkd: do not touch link_messages when expiring routes link_messages is used during link configuration to advance the link state machine through SETTING_ADDRESSES -> SETTING_ROUTES -> CONFIGURED. If a route expires in the middle of this, it is possible for link_messages to hit zero inside route_expire_callback, rather than in route_handler or address_handler where it would trigger the next step in configuration. Should this happen, the link will not complete configuration, and it may not have its static routes configured. Since route_expire_callback does not need to do anything once the expired route has been removed from the kernel, it is safe to simply not account for the netlink request. --- src/network/networkd-route.c | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 82f9047ff61..5335df53c7e 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -441,20 +441,14 @@ static int route_expire_callback(sd_netlink *rtnl, sd_netlink_message *m, void * assert(m); assert(link); assert(link->ifname); - assert(link->link_messages > 0); if (IN_SET(link->state, LINK_STATE_FAILED, LINK_STATE_LINGER)) return 1; - link->link_messages--; - r = sd_netlink_message_get_errno(m); if (r < 0 && r != -EEXIST) log_link_warning_errno(link, r, "could not remove route: %m"); - if (link->link_messages == 0) - log_link_debug(link, "route removed"); - return 1; } @@ -467,11 +461,8 @@ int route_expire_handler(sd_event_source *s, uint64_t usec, void *userdata) { r = route_remove(route, route->link, route_expire_callback); if (r < 0) log_warning_errno(r, "Could not remove route: %m"); - else { - /* route may not be exist in kernel. If we fail still remove it */ - route->link->link_messages++; + else route_free(route); - } return 1; } From a0d95bbc38e60b1ac0b6b57d7a0ac3246d1576be Mon Sep 17 00:00:00 2001 From: Michael Chapman Date: Thu, 18 Aug 2016 17:54:12 +1000 Subject: [PATCH 3/3] networkd: use RT_TABLE_MAIN by default The default route table used by sd-netlink (and iproute2) is RT_TABLE_MAIN, not RT_TABLE_DEFAULT. Ensure networkd has the same idea. --- src/network/networkd-route.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/network/networkd-route.c b/src/network/networkd-route.c index 5335df53c7e..b197a5da6b3 100644 --- a/src/network/networkd-route.c +++ b/src/network/networkd-route.c @@ -41,7 +41,7 @@ int route_new(Route **ret) { route->family = AF_UNSPEC; route->scope = RT_SCOPE_UNIVERSE; route->protocol = RTPROT_UNSPEC; - route->table = RT_TABLE_DEFAULT; + route->table = RT_TABLE_MAIN; route->lifetime = USEC_INFINITY; *ret = route; @@ -549,14 +549,12 @@ int route_configure( if (r < 0) return log_error_errno(r, "Could not set flags: %m"); - if (route->table != RT_TABLE_DEFAULT) { - + if (route->table != RT_TABLE_MAIN) { if (route->table < 256) { r = sd_rtnl_message_route_set_table(req, route->table); if (r < 0) return log_error_errno(r, "Could not set route table: %m"); } else { - r = sd_rtnl_message_route_set_table(req, RT_TABLE_UNSPEC); if (r < 0) return log_error_errno(r, "Could not set route table: %m");