Commit ad21d542 authored by Thomas Haller's avatar Thomas Haller
Browse files

iface-helper: fix non-reentrant call to platform for failed IPv6 DAD

Platform invokes change events while reading netlink events. However,
platform code is not re-entrant and calling into platform again is not
allowed (aside operations that do not process the netlink socket, like
lookup of the platform cache).

That basically means, we have to always process events in an idle
handler. That is not a too strong limitation, because we anyway don't
know the call context in which the platform event is emitted and we
should avoid unguarded recursive calls into platform.

Otherwise, we get hit an assertion/crash in nm-iface-helper:

     1  raise()
     2  abort()
     3  g_assertion_message()
     4  g_assertion_message_expr()
     5  do_delete_object()
     6  ip6_address_delete()
 >>> 7  nm_platform_ip6_address_delete()
     8  nm_platform_ip6_address_sync()
     9  nm_ip6_config_commit()
     10 ndisc_config_changed()
     11 ffi_call_unix64()
     12 ffi_call()
     13 g_cclosure_marshal_generic_va()
     14 _g_closure_invoke_va()
     15 g_signal_emit_valist()
     16 g_signal_emit()
 >>> 17 nm_ndisc_dad_failed()
     18 ffi_call_unix64()
     19 ffi_call()
     20 g_cclosure_marshal_generic()
     21 g_closure_invoke()
     22 signal_emit_unlocked_R()
     23 g_signal_emit_valist()
     24 g_signal_emit()
 >>> 25 nm_platform_cache_update_emit_signal()
     26 event_handler_recvmsgs()
     27 event_handler_read_netlink()
     28 delayed_action_handle_one()
     29 delayed_action_handle_all()
     30 do_delete_object()
     31 ip6_address_delete()
     32 nm_platform_ip6_address_delete()
     33 nm_platform_ip6_address_sync()
 >>> 34 nm_ip6_config_commit()
     35 ndisc_config_changed()
     36 ffi_call_unix64()
     37 ffi_call()
     38 g_cclosure_marshal_generic_va()
     39 _g_closure_invoke_va()
     40 g_signal_emit_valist()
     41 g_signal_emit()
     42 check_timestamps()
     43 receive_ra()
     44 ndp_call_eventfd_handler()
     45 ndp_callall_eventfd_handler()
     46 event_ready()
     47 g_main_context_dispatch()
     48 g_main_context_iterate.isra.22()
     49 g_main_loop_run()
 >>> 50 main()

NMPlatform already has a check to assert against recursive calls
in delayed_action_handle_all():

    g_return_val_if_fail (priv->delayed_action.is_handling == 0, FALSE);

    priv->delayed_action.is_handling++;
    ...
    priv->delayed_action.is_handling--;

Fixes: f85728ec

https://bugzilla.redhat.com/show_bug.cgi?id=1546656
parent cf796151
......@@ -11638,7 +11638,6 @@ queued_ip6_config_change (gpointer user_data)
{
NMDevice *self = user_data;
NMDevicePrivate *priv;
GSList *iter;
gboolean need_ipv6ll = FALSE;
NMPlatform *platform;
......@@ -11671,20 +11670,17 @@ queued_ip6_config_change (gpointer user_data)
&& (platform = nm_device_get_platform (self))
&& nm_platform_link_get (platform, priv->ifindex)) {
/* Handle DAD failures */
for (iter = priv->dad6_failed_addrs; iter; iter = iter->next) {
const NMPObject *obj = iter->data;
const NMPlatformIP6Address *addr = NMP_OBJECT_CAST_IP6_ADDRESS (obj);
const NMPlatformIP6Address *addr2;
addr2 = NMP_OBJECT_CAST_IP6_ADDRESS (nm_platform_lookup_obj (platform,
NMP_CACHE_ID_TYPE_OBJECT_TYPE,
obj));
if ( addr2
&& ( NM_FLAGS_HAS (addr2->n_ifa_flags, IFA_F_TEMPORARY)
|| !NM_FLAGS_HAS (addr2->n_ifa_flags, IFA_F_DADFAILED))) {
/* the address still/again exists and is not in DADFAILED state. Skip it. */
while (priv->dad6_failed_addrs) {
nm_auto_nmpobj const NMPObject *obj = NULL;
const NMPlatformIP6Address *addr;
obj = priv->dad6_failed_addrs->data;
priv->dad6_failed_addrs = g_slist_delete_link (priv->dad6_failed_addrs, priv->dad6_failed_addrs);
if (!nm_ndisc_dad_addr_is_fail_candidate (platform, obj))
continue;
}
addr = NMP_OBJECT_CAST_IP6_ADDRESS (obj);
_LOGI (LOGD_IP6, "ipv6: duplicate address check failed for the %s address",
nm_platform_ip6_address_to_string (addr, NULL, 0));
......@@ -11705,11 +11701,11 @@ queued_ip6_config_change (gpointer user_data)
if (need_ipv6ll)
check_and_add_ipv6ll_addr (self);
} else {
g_slist_free_full (priv->dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
priv->dad6_failed_addrs = NULL;
}
g_slist_free_full (priv->dad6_failed_addrs, (GDestroyNotify) nmp_object_unref);
priv->dad6_failed_addrs = NULL;
/* Check if DAD is still pending */
if ( priv->ip6_state == IP_CONF
&& priv->dad6_ip6_config
......@@ -11763,11 +11759,9 @@ device_ipx_changed (NMPlatform *platform,
case NMP_OBJECT_TYPE_IP6_ADDRESS:
addr = platform_object;
if ( !NM_FLAGS_HAS (addr->n_ifa_flags, IFA_F_TEMPORARY)
&& priv->state > NM_DEVICE_STATE_DISCONNECTED
if ( priv->state > NM_DEVICE_STATE_DISCONNECTED
&& priv->state < NM_DEVICE_STATE_DEACTIVATING
&& ( (change_type == NM_PLATFORM_SIGNAL_CHANGED && addr->n_ifa_flags & IFA_F_DADFAILED)
|| (change_type == NM_PLATFORM_SIGNAL_REMOVED && addr->n_ifa_flags & IFA_F_TENTATIVE))) {
&& nm_ndisc_dad_addr_is_fail_candidate_event (change_type, addr)) {
priv->dad6_failed_addrs = g_slist_prepend (priv->dad6_failed_addrs,
(gpointer) nmp_object_ref (NMP_OBJECT_UP_CAST (addr)));
}
......
......@@ -27,6 +27,9 @@
#include "nm-setting-ip6-config.h"
#include "NetworkManagerUtils.h"
#include "platform/nm-platform.h"
#include "platform/nmp-object.h"
#define NM_TYPE_NDISC (nm_ndisc_get_type ())
#define NM_NDISC(obj) (G_TYPE_CHECK_INSTANCE_CAST ((obj), NM_TYPE_NDISC, NMNDisc))
#define NM_NDISC_CLASS(klass) (G_TYPE_CHECK_CLASS_CAST ((klass), NM_TYPE_NDISC, NMNDiscClass))
......@@ -184,4 +187,32 @@ NMPlatform *nm_ndisc_get_platform (NMNDisc *self);
NMPNetns *nm_ndisc_netns_get (NMNDisc *self);
gboolean nm_ndisc_netns_push (NMNDisc *self, NMPNetns **netns);
static inline gboolean
nm_ndisc_dad_addr_is_fail_candidate_event (NMPlatformSignalChangeType change_type,
const NMPlatformIP6Address *addr)
{
return !NM_FLAGS_HAS (addr->n_ifa_flags, IFA_F_TEMPORARY)
&& ( (change_type == NM_PLATFORM_SIGNAL_CHANGED && addr->n_ifa_flags & IFA_F_DADFAILED)
|| (change_type == NM_PLATFORM_SIGNAL_REMOVED && addr->n_ifa_flags & IFA_F_TENTATIVE));
}
static inline gboolean
nm_ndisc_dad_addr_is_fail_candidate (NMPlatform *platform,
const NMPObject *obj)
{
const NMPlatformIP6Address *addr;
addr = NMP_OBJECT_CAST_IP6_ADDRESS (nm_platform_lookup_obj (platform,
NMP_CACHE_ID_TYPE_OBJECT_TYPE,
obj));
if ( addr
&& ( NM_FLAGS_HAS (addr->n_ifa_flags, IFA_F_TEMPORARY)
|| !NM_FLAGS_HAS (addr->n_ifa_flags, IFA_F_DADFAILED))) {
/* the address still/again exists and is not in DADFAILED state. Skip it. */
return FALSE;
}
return TRUE;
}
#endif /* __NETWORKMANAGER_NDISC_H__ */
......@@ -33,6 +33,8 @@
#include <signal.h>
#include <linux/rtnetlink.h>
#include "nm-utils/nm-c-list.h"
#include "main-utils.h"
#include "NetworkManagerUtils.h"
#include "platform/nm-linux-platform.h"
......@@ -55,6 +57,9 @@
static struct {
GMainLoop *main_loop;
int ifindex;
guint dad_failed_id;
CList dad_failed_lst_head;
} gl/*obal*/ = {
.ifindex = -1,
};
......@@ -316,19 +321,58 @@ do_early_setup (int *argc, char **argv[])
return TRUE;
}
typedef struct {
NMPlatform *platform;
NMNDisc *ndisc;
} DadFailedHandleData;
static gboolean
dad_failed_handle_idle (gpointer user_data)
{
DadFailedHandleData *data = user_data;
NMCListElem *elem;
while ((elem = c_list_first_entry (&gl.dad_failed_lst_head, NMCListElem, lst))) {
nm_auto_nmpobj const NMPObject *obj = elem->data;
nm_c_list_elem_free (elem);
if (nm_ndisc_dad_addr_is_fail_candidate (data->platform, obj)) {
nm_ndisc_dad_failed (data->ndisc,
&NMP_OBJECT_CAST_IP6_ADDRESS (obj)->address);
}
}
gl.dad_failed_id = 0;
return G_SOURCE_REMOVE;
}
static void
ip6_address_changed (NMPlatform *platform,
int obj_type_i,
int iface,
NMPlatformIP6Address *addr,
const NMPlatformIP6Address *addr,
int change_type_i,
NMNDisc *ndisc)
{
const NMPlatformSignalChangeType change_type = change_type_i;
if ( (change_type == NM_PLATFORM_SIGNAL_CHANGED && addr->n_ifa_flags & IFA_F_DADFAILED)
|| (change_type == NM_PLATFORM_SIGNAL_REMOVED && addr->n_ifa_flags & IFA_F_TENTATIVE))
nm_ndisc_dad_failed (ndisc, &addr->address);
DadFailedHandleData *data;
if (!nm_ndisc_dad_addr_is_fail_candidate_event (change_type, addr))
return;
c_list_link_tail (&gl.dad_failed_lst_head,
&nm_c_list_elem_new_stale ((gpointer) nmp_object_ref (NMP_OBJECT_UP_CAST (addr)))->lst);
if (gl.dad_failed_id)
return;
data = g_slice_new (DadFailedHandleData);
data->platform = platform;
data->ndisc = ndisc;
gl.dad_failed_id = g_idle_add_full (G_PRIORITY_DEFAULT_IDLE,
dad_failed_handle_idle,
data,
nm_g_slice_free_fcn (DadFailedHandleData));
}
int
......@@ -346,6 +390,8 @@ main (int argc, char *argv[])
guint sd_id;
char sysctl_path_buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE];
c_list_init (&gl.dad_failed_lst_head);
setpgid (getpid (), getpid ());
if (!do_early_setup (&argc, &argv))
......@@ -526,6 +572,9 @@ main (int argc, char *argv[])
g_main_loop_run (gl.main_loop);
nm_clear_g_source (&gl.dad_failed_id);
nm_c_list_elem_free_all (&gl.dad_failed_lst_head, (GDestroyNotify) nmp_object_unref);
if (pidfile && wrote_pidfile)
unlink (pidfile);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment