Commit 193989cc authored by Julian Anastasov's avatar Julian Anastasov Committed by Pablo Neira Ayuso
Browse files

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: 05f00505 ("ipvs: fix crash if scheduler is changed")
Signed-off-by: default avatarJulian Anastasov <ja@ssi.bg>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent c6c5327d

include/net/ip_vs.h

+1 −2
Original line number Diff line number Diff line
@@ -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 *

net/netfilter/ipvs/ip_vs_ctl.c

+8 −5
Original line number Diff line number Diff line
@@ -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 */

net/netfilter/ipvs/ip_vs_sched.c

+7 −7
Original line number Diff line number Diff line
@@ -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 */

}