Merge pull request #19924 from yuwata/sd-event-fix-assertion

sd-event: fix assrtion in sleep_between()

src/libsystemd/sd-event/sd-event.c

@@ -168,10 +168,9 @@ static int pending_prioq_compare(const void *a, const void *b) {
         assert(y->pending);
 
         /* Enabled ones first */
-        if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)
-                return -1;
-        if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
-                return 1;
+        r = CMP(x->enabled == SD_EVENT_OFF, y->enabled == SD_EVENT_OFF);
+        if (r != 0)
+                return r;
 
         /* Non rate-limited ones first. */
         r = CMP(!!x->ratelimited, !!y->ratelimited);
@@ -195,10 +194,9 @@ static int prepare_prioq_compare(const void *a, const void *b) {
         assert(y->prepare);
 
         /* Enabled ones first */
-        if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)
-                return -1;
-        if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
-                return 1;
+        r = CMP(x->enabled == SD_EVENT_OFF, y->enabled == SD_EVENT_OFF);
+        if (r != 0)
+                return r;
 
         /* Non rate-limited ones first. */
         r = CMP(!!x->ratelimited, !!y->ratelimited);
@@ -265,18 +263,17 @@ static bool event_source_timer_candidate(const sd_event_source *s) {
 
 static int time_prioq_compare(const void *a, const void *b, usec_t (*time_func)(const sd_event_source *s)) {
         const sd_event_source *x = a, *y = b;
+        int r;
 
         /* Enabled ones first */
-        if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)
-                return -1;
-        if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
-                return 1;
+        r = CMP(x->enabled == SD_EVENT_OFF, y->enabled == SD_EVENT_OFF);
+        if (r != 0)
+                return r;
 
         /* Order "non-pending OR ratelimited" before "pending AND not-ratelimited" */
-        if (event_source_timer_candidate(x) && !event_source_timer_candidate(y))
-                return -1;
-        if (!event_source_timer_candidate(x) && event_source_timer_candidate(y))
-                return 1;
+        r = CMP(!event_source_timer_candidate(x), !event_source_timer_candidate(y));
+        if (r != 0)
+                return r;
 
         /* Order by time */
         return CMP(time_func(x), time_func(y));
@@ -292,15 +289,15 @@ static int latest_time_prioq_compare(const void *a, const void *b) {
 
 static int exit_prioq_compare(const void *a, const void *b) {
         const sd_event_source *x = a, *y = b;
+        int r;
 
         assert(x->type == SOURCE_EXIT);
         assert(y->type == SOURCE_EXIT);
 
         /* Enabled ones first */
-        if (x->enabled != SD_EVENT_OFF && y->enabled == SD_EVENT_OFF)
-                return -1;
-        if (x->enabled == SD_EVENT_OFF && y->enabled != SD_EVENT_OFF)
-                return 1;
+        r = CMP(x->enabled == SD_EVENT_OFF, y->enabled == SD_EVENT_OFF);
+        if (r != 0)
+                return r;
 
         /* Lower priority values first */
         return CMP(x->priority, y->priority);
@@ -774,14 +771,15 @@ static void event_source_time_prioq_reshuffle(sd_event_source *s) {
         assert(s);
 
         /* Called whenever the event source's timer ordering properties changed, i.e. time, accuracy,
-         * pending, enable state. Makes sure the two prioq's are ordered properly again. */
+         * pending, enable state, and ratelimiting state. Makes sure the two prioq's are ordered
+         * properly again. */
 
         if (s->ratelimited)
                 d = &s->event->monotonic;
-        else {
-                assert(EVENT_SOURCE_IS_TIME(s->type));
+        else if (EVENT_SOURCE_IS_TIME(s->type))
                 assert_se(d = event_get_clock_data(s->event, s->type));
-        }
+        else
+                return; /* no-op for an event source which is neither a timer nor ratelimited. */
 
         prioq_reshuffle(d->earliest, s, &s->earliest_index);
         prioq_reshuffle(d->latest, s, &s->latest_index);
@@ -2372,14 +2370,6 @@ static int event_source_offline(
                 source_io_unregister(s);
                 break;
 
-        case SOURCE_TIME_REALTIME:
-        case SOURCE_TIME_BOOTTIME:
-        case SOURCE_TIME_MONOTONIC:
-        case SOURCE_TIME_REALTIME_ALARM:
-        case SOURCE_TIME_BOOTTIME_ALARM:
-                event_source_time_prioq_reshuffle(s);
-                break;
-
         case SOURCE_SIGNAL:
                 event_gc_signal_data(s->event, &s->priority, s->signal.sig);
                 break;
@@ -2400,6 +2390,11 @@ static int event_source_offline(
                 prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
                 break;
 
+        case SOURCE_TIME_REALTIME:
+        case SOURCE_TIME_BOOTTIME:
+        case SOURCE_TIME_MONOTONIC:
+        case SOURCE_TIME_REALTIME_ALARM:
+        case SOURCE_TIME_BOOTTIME_ALARM:
         case SOURCE_DEFER:
         case SOURCE_POST:
         case SOURCE_INOTIFY:
@@ -2409,6 +2404,9 @@ static int event_source_offline(
                 assert_not_reached("Wut? I shouldn't exist.");
         }
 
+        /* Always reshuffle time prioq, as the ratelimited flag may be changed. */
+        event_source_time_prioq_reshuffle(s);
+
         return 1;
 }
 
@@ -2498,22 +2496,11 @@ static int event_source_online(
         s->ratelimited = ratelimited;
 
         /* Non-failing operations below */
-        switch (s->type) {
-        case SOURCE_TIME_REALTIME:
-        case SOURCE_TIME_BOOTTIME:
-        case SOURCE_TIME_MONOTONIC:
-        case SOURCE_TIME_REALTIME_ALARM:
-        case SOURCE_TIME_BOOTTIME_ALARM:
-                event_source_time_prioq_reshuffle(s);
-                break;
-
-        case SOURCE_EXIT:
+        if (s->type == SOURCE_EXIT)
                 prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
-                break;
 
-        default:
-                break;
-        }
+        /* Always reshuffle time prioq, as the ratelimited flag may be changed. */
+        event_source_time_prioq_reshuffle(s);
 
         return 1;
 }
@@ -2983,8 +2970,8 @@ static int event_arm_timer(
 
         if (!d->needs_rearm)
                 return 0;
-        else
-                d->needs_rearm = false;
+
+        d->needs_rearm = false;
 
         a = prioq_peek(d->earliest);
         if (!a || a->enabled == SD_EVENT_OFF || time_event_source_next(a) == USEC_INFINITY) {
@@ -3680,8 +3667,8 @@ static int arm_watchdog(sd_event *e) {
         assert(e->watchdog_fd >= 0);
 
         t = sleep_between(e,
-                          e->watchdog_last + (e->watchdog_period / 2),
-                          e->watchdog_last + (e->watchdog_period * 3 / 4));
+                          usec_add(e->watchdog_last, (e->watchdog_period / 2)),
+                          usec_add(e->watchdog_last, (e->watchdog_period * 3 / 4)));
 
         timespec_store(&its.it_value, t);
 

src/network/networkd-link-bus.c

@@ -667,7 +667,7 @@ int bus_link_method_reconfigure(sd_bus_message *message, void *userdata, sd_bus_
         if (r == 0)
                 return 1; /* Polkit will call us back */
 
-        r = link_reconfigure(l, true);
+        r = link_reconfigure(l, /* force = */ true);
         if (r < 0)
                 return r;
         if (r > 0) {

src/network/networkd-link.c

@@ -1199,7 +1199,7 @@ static int link_get_network(Link *link, Network **ret) {
         return -ENOENT;
 }
 
-static int link_reconfigure_internal(Link *link, bool force) {
+static int link_reconfigure_impl(Link *link, bool force) {
         Network *network;
         int r;
 
@@ -1267,7 +1267,7 @@ static int link_reconfigure_handler_internal(sd_netlink *rtnl, sd_netlink_messag
         if (r <= 0)
                 return r;
 
-        r = link_reconfigure_internal(link, force);
+        r = link_reconfigure_impl(link, force);
         if (r < 0)
                 link_enter_failed(link);
 
@@ -1497,15 +1497,6 @@ static int link_carrier_gained(Link *link) {
                 /* let's shortcut things for CAN which doesn't need most of what's done below. */
                 return 0;
 
-        r = wifi_get_info(link);
-        if (r < 0)
-                return r;
-        if (r > 0) {
-                r = link_reconfigure_internal(link, false);
-                if (r != 0)
-                        return r;
-        }
-
         if (IN_SET(link->state, LINK_STATE_CONFIGURING, LINK_STATE_CONFIGURED)) {
                 r = link_acquire_dynamic_conf(link);
                 if (r < 0)
@@ -1830,7 +1821,7 @@ void link_update_operstate(Link *link, bool also_update_master) {
          : "")
 
 static int link_update_flags(Link *link, sd_netlink_message *message) {
-        bool link_was_admin_up, had_carrier;
+        bool link_was_lower_up, link_was_admin_up, had_carrier;
         uint8_t operstate;
         unsigned flags;
         int r;
@@ -1892,6 +1883,7 @@ static int link_update_flags(Link *link, sd_netlink_message *message) {
                         log_link_debug(link, "Unknown link flags lost, ignoring: %#.5x", unknown_flags_removed);
         }
 
+        link_was_lower_up = link->flags & IFF_LOWER_UP;
         link_was_admin_up = link->flags & IFF_UP;
         had_carrier = link_has_carrier(link);
 
@@ -1900,6 +1892,19 @@ static int link_update_flags(Link *link, sd_netlink_message *message) {
 
         link_update_operstate(link, true);
 
+        if (!link_was_lower_up && (link->flags & IFF_LOWER_UP)) {
+                r = wifi_get_info(link);
+                if (r < 0)
+                        return r;
+                if (r > 0) {
+                        /* All link information is up-to-date. So, it is not necessary to call
+                         * RTM_GETLINK netlink method again. */
+                        r = link_reconfigure_impl(link, /* force = */ false);
+                        if (r < 0)
+                                return r;
+                }
+        }
+
         if (!link_was_admin_up && (link->flags & IFF_UP)) {
                 log_link_info(link, "Link UP");
 

src/network/networkd-manager-bus.c

@@ -216,7 +216,7 @@ static int bus_method_reload(sd_bus_message *message, void *userdata, sd_bus_err
                 return r;
 
         HASHMAP_FOREACH(link, manager->links) {
-                r = link_reconfigure(link, false);
+                r = link_reconfigure(link, /* force = */ false);
                 if (r < 0)
                         return r;
         }

src/network/networkd-setlink.c

@@ -920,7 +920,16 @@ static int link_up_or_down(Link *link, bool up, link_netlink_message_handler_t c
 }
 
 int link_down(Link *link) {
-        return link_up_or_down(link, false, link_down_handler);
+        int r;
+
+        assert(link);
+
+        r = link_up_or_down(link, false, link_down_handler);
+        if (r < 0)
+                return log_link_error_errno(link, r, "Failed to bring down interface: %m");
+
+        link->set_flags_messages++;
+        return 0;
 }
 
 static bool link_is_ready_to_activate(Link *link) {

src/network/networkd-wifi.c

@@ -18,8 +18,9 @@
 int wifi_get_info(Link *link) {
         _cleanup_free_ char *ssid = NULL;
         enum nl80211_iftype iftype;
+        bool updated = false;
         const char *type;
-        int r, s = 0;
+        int r;
 
         assert(link);
 
@@ -38,23 +39,26 @@ int wifi_get_info(Link *link) {
         r = wifi_get_interface(link->manager->genl, link->ifindex, &iftype, &ssid);
         if (r < 0)
                 return r;
-        if (r > 0 && link->wlan_iftype == iftype && streq_ptr(link->ssid, ssid))
-                r = 0;
+        if (r == 0)
+                iftype = link->wlan_iftype; /* Assume iftype is not changed. */
 
-        link->wlan_iftype = iftype;
-        free_and_replace(link->ssid, ssid);
+        if (iftype == NL80211_IFTYPE_STATION) {
+                struct ether_addr bssid;
 
-        if (link->wlan_iftype == NL80211_IFTYPE_STATION) {
-                struct ether_addr old_bssid = link->bssid;
+                r = wifi_get_station(link->manager->genl, link->ifindex, &bssid);
+                if (r < 0)
+                        return r;
 
-                s = wifi_get_station(link->manager->genl, link->ifindex, &link->bssid);
-                if (s < 0)
-                        return s;
-                if (s > 0 && memcmp(&old_bssid, &link->bssid, sizeof old_bssid) == 0)
-                        s = 0;
+                updated = !ether_addr_equal(&link->bssid, &bssid);
+                link->bssid = bssid;
         }
 
-        if (r > 0 || s > 0) {
+        updated = updated || link->wlan_iftype != iftype;
+        link->wlan_iftype = iftype;
+        updated = updated || !streq_ptr(link->ssid, ssid);
+        free_and_replace(link->ssid, ssid);
+
+        if (updated) {
                 if (link->wlan_iftype == NL80211_IFTYPE_STATION && link->ssid)
                         log_link_info(link, "Connected WiFi access point: %s (%s)",
                                       link->ssid, ETHER_ADDR_TO_STR(&link->bssid));

src/test/test-seccomp.c

@@ -837,7 +837,7 @@ static void test_load_syscall_filter_set_raw(void) {
                 assert_se(s = hashmap_new(NULL));
 #if defined __NR_poll && __NR_poll >= 0
                 assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_poll + 1), INT_TO_PTR(-1)) >= 0);
-#else
+#elif defined __NR_ppoll
                 assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_ppoll + 1), INT_TO_PTR(-1)) >= 0);
 #endif
 
@@ -854,7 +854,8 @@ static void test_load_syscall_filter_set_raw(void) {
                 assert_se(s = hashmap_new(NULL));
 #if defined __NR_poll && __NR_poll >= 0
                 assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_poll + 1), INT_TO_PTR(EILSEQ)) >= 0);
-#else
+#elif defined __NR_ppoll
+                assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_ppoll + 1), INT_TO_PTR(-1)) >= 0);
                 assert_se(hashmap_put(s, UINT32_TO_PTR(__NR_ppoll + 1), INT_TO_PTR(EILSEQ)) >= 0);
 #endif