ipvs: clear the svc scheduler ptr early on edit

ip_vs_edit_service() while unbinding the old scheduler clears
the svc->scheduler ptr after the scheduler module initiates
RCU callbacks. This can cause packets to use the old
scheduler at the time when svc->sched_data is already freed
after RCU grace period.

Fix it by clearing the ptr early in ip_vs_unbind_scheduler(),
before the done_service method schedules any RCU callbacks.

Also, if the new scheduler fails to initialize when replacing
the old scheduler, try to restore the old scheduler while still
returning the error code.

Link: https://sashiko.dev/#/patchset/20260519015506.634185-1-rosenp%40gmail.com
Fixes: 05f00505a89a ("ipvs: fix crash if scheduler is changed")
Signed-off-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

diff --git a/include/net/ip_vs.h b/include/net/ip_vs.h
index a02e569813d2b6195455943c995b1cc6ab40b5ff..e517eaaa177b02179e833d829a82799ff0479951 100644 (file)
--- a/include/net/ip_vs.h
+++ b/include/net/ip_vs.h
@@ -1824,8 +1824,7 @@ int register_ip_vs_scheduler(struct ip_vs_scheduler *scheduler);
 int unregister_ip_vs_scheduler(struct ip_vs_scheduler *scheduler);
 int ip_vs_bind_scheduler(struct ip_vs_service *svc,
                         struct ip_vs_scheduler *scheduler);
-void ip_vs_unbind_scheduler(struct ip_vs_service *svc,
-                           struct ip_vs_scheduler *sched);
+void ip_vs_unbind_scheduler(struct ip_vs_service *svc);
 struct ip_vs_scheduler *ip_vs_scheduler_get(const char *sched_name);
 void ip_vs_scheduler_put(struct ip_vs_scheduler *scheduler);
 struct ip_vs_conn *

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index bd9cae44d2149e66613c2aae987c04066ee92782..16daba8cac83c551f1fbbfe73c86d82ed95af0ea 100644 (file)
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1898,7 +1898,7 @@ ip_vs_add_service(struct netns_ipvs *ipvs, struct ip_vs_service_user_kern *u,
        if (ret_hooks >= 0)
                ip_vs_unregister_hooks(ipvs, u->af);
        if (svc != NULL) {
-               ip_vs_unbind_scheduler(svc, sched);
+               ip_vs_unbind_scheduler(svc);
                ip_vs_service_free(svc);
        }
        ip_vs_scheduler_put(sched);
@@ -1962,9 +1962,8 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
        old_sched = rcu_dereference_protected(svc->scheduler, 1);
        if (sched != old_sched) {
                if (old_sched) {
-                       ip_vs_unbind_scheduler(svc, old_sched);
-                       RCU_INIT_POINTER(svc->scheduler, NULL);
-                       /* Wait all svc->sched_data users */
+                       ip_vs_unbind_scheduler(svc);
+                       /* Wait all svc->scheduler/sched_data users */
                        synchronize_rcu();
                }
                /* Bind the new scheduler */
@@ -1972,6 +1971,10 @@ ip_vs_edit_service(struct ip_vs_service *svc, struct ip_vs_service_user_kern *u)
                        ret = ip_vs_bind_scheduler(svc, sched);
                        if (ret) {
                                ip_vs_scheduler_put(sched);
+                               /* Try to restore the old_sched */
+                               if (old_sched &&
+                                   !ip_vs_bind_scheduler(svc, old_sched))
+                                       old_sched = NULL;
                                goto out;
                        }
                }
@@ -2027,7 +2030,7 @@ static void __ip_vs_del_service(struct ip_vs_service *svc, bool cleanup)
 
        /* Unbind scheduler */
        old_sched = rcu_dereference_protected(svc->scheduler, 1);
-       ip_vs_unbind_scheduler(svc, old_sched);
+       ip_vs_unbind_scheduler(svc);
        ip_vs_scheduler_put(old_sched);
 
        /* Unbind persistence engine, keep svc->pe */

diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c
index c6e421c4e2991363769085555d3e9b0a05ade5d4..24adc38942a0d185e91145c49a38a79769f9cebf 100644 (file)
--- a/net/netfilter/ipvs/ip_vs_sched.c
+++ b/net/netfilter/ipvs/ip_vs_sched.c
@@ -56,19 +56,19 @@ int ip_vs_bind_scheduler(struct ip_vs_service *svc,
 /*
  *  Unbind a service with its scheduler
  */
-void ip_vs_unbind_scheduler(struct ip_vs_service *svc,
-                           struct ip_vs_scheduler *sched)
+void ip_vs_unbind_scheduler(struct ip_vs_service *svc)
 {
-       struct ip_vs_scheduler *cur_sched;
+       struct ip_vs_scheduler *sched;
 
-       cur_sched = rcu_dereference_protected(svc->scheduler, 1);
-       /* This check proves that old 'sched' was installed */
-       if (!cur_sched)
+       sched = rcu_dereference_protected(svc->scheduler, 1);
+       if (!sched)
                return;
 
+       /* Reset the scheduler before initiating any RCU callbacks */
+       rcu_assign_pointer(svc->scheduler, NULL);
+       smp_wmb();      /* paired with smp_rmb() in ip_vs_schedule() */
        if (sched->done_service)
                sched->done_service(svc);
-       /* svc->scheduler can be set to NULL only by caller */
 }