[PATCH net] ipvs: fix NULL deref in ip_vs_add_service error path

[PATCH net] ipvs: fix NULL deref in ip_vs_add_service error path

[Weiming Shi]

When ip_vs_bind_scheduler() succeeds in ip_vs_add_service(), the local
variable sched is set to NULL.  If ip_vs_start_estimator() subsequently
fails, the out_err cleanup calls ip_vs_unbind_scheduler(svc, sched)
with sched == NULL.  ip_vs_unbind_scheduler() passes the cur_sched NULL
check (because svc->scheduler was set by the successful bind) but then
dereferences the NULL sched parameter at sched->done_service, causing a
kernel panic at offset 0x30 from NULL.

 Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] PREEMPT SMP KASAN NOPTI
 KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
 RIP: 0010:ip_vs_unbind_scheduler (net/netfilter/ipvs/ip_vs_sched.c:69)
 Call Trace:
  <TASK>
  ip_vs_add_service.isra.0 (net/netfilter/ipvs/ip_vs_ctl.c:1500)
  do_ip_vs_set_ctl (net/netfilter/ipvs/ip_vs_ctl.c:2809)
  nf_setsockopt (net/netfilter/nf_sockopt.c:102)
  ip_setsockopt (net/ipv4/ip_sockglue.c:1427)
  raw_setsockopt (net/ipv4/raw.c:850)
  do_sock_setsockopt (net/socket.c:2322)
  __sys_setsockopt (net/socket.c:2339)
  __x64_sys_setsockopt (net/socket.c:2350)
  do_syscall_64 (arch/x86/entry/syscall_64.c:94)
  entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
  </TASK>

Fix by recovering the scheduler pointer from svc->scheduler before
cleanup when the local sched variable has been cleared.  This also
prevents a latent module refcount leak: without the recovery,
ip_vs_scheduler_put(sched) receives NULL and skips the module_put(),
so the scheduler module could never be unloaded if the kernel survived
past the dereference.

Fixes: 05f00505a89a ("ipvs: fix crash if scheduler is changed")
Reported-by: Xiang Mei <xmei5@asu.edu>
Signed-off-by: Weiming Shi <bestswngs@gmail.com>
---
 net/netfilter/ipvs/ip_vs_ctl.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index 35642de2a0fee..e0c978def9749 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -1497,6 +1497,8 @@ 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) {
+		if (!sched)
+			sched = rcu_dereference_protected(svc->scheduler, 1);
 		ip_vs_unbind_scheduler(svc, sched);
 		ip_vs_service_free(svc);
 	}
-- 
2.43.0

Re: [PATCH net] ipvs: fix NULL deref in ip_vs_add_service error path

[Julian Anastasov]

	Hello,

On Wed, 1 Apr 2026, Weiming Shi wrote:

> When ip_vs_bind_scheduler() succeeds in ip_vs_add_service(), the local
> variable sched is set to NULL.  If ip_vs_start_estimator() subsequently
> fails, the out_err cleanup calls ip_vs_unbind_scheduler(svc, sched)
> with sched == NULL.  ip_vs_unbind_scheduler() passes the cur_sched NULL
> check (because svc->scheduler was set by the successful bind) but then
> dereferences the NULL sched parameter at sched->done_service, causing a
> kernel panic at offset 0x30 from NULL.
> 
>  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] PREEMPT SMP KASAN NOPTI
>  KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
>  RIP: 0010:ip_vs_unbind_scheduler (net/netfilter/ipvs/ip_vs_sched.c:69)
>  Call Trace:
>   <TASK>
>   ip_vs_add_service.isra.0 (net/netfilter/ipvs/ip_vs_ctl.c:1500)
>   do_ip_vs_set_ctl (net/netfilter/ipvs/ip_vs_ctl.c:2809)
>   nf_setsockopt (net/netfilter/nf_sockopt.c:102)
>   ip_setsockopt (net/ipv4/ip_sockglue.c:1427)
>   raw_setsockopt (net/ipv4/raw.c:850)
>   do_sock_setsockopt (net/socket.c:2322)
>   __sys_setsockopt (net/socket.c:2339)
>   __x64_sys_setsockopt (net/socket.c:2350)
>   do_syscall_64 (arch/x86/entry/syscall_64.c:94)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>   </TASK>
> 
> Fix by recovering the scheduler pointer from svc->scheduler before
> cleanup when the local sched variable has been cleared.  This also
> prevents a latent module refcount leak: without the recovery,
> ip_vs_scheduler_put(sched) receives NULL and skips the module_put(),
> so the scheduler module could never be unloaded if the kernel survived
> past the dereference.
> 
> Fixes: 05f00505a89a ("ipvs: fix crash if scheduler is changed")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
>  net/netfilter/ipvs/ip_vs_ctl.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> index 35642de2a0fee..e0c978def9749 100644
> --- a/net/netfilter/ipvs/ip_vs_ctl.c
> +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> @@ -1497,6 +1497,8 @@ 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) {
> +		if (!sched)
> +			sched = rcu_dereference_protected(svc->scheduler, 1);

	Good catch. But may be it should be enough if
we just remove the sched = NULL after successful
ip_vs_bind_scheduler(), what do you think? ip_vs_unbind_scheduler()
already detects if the scheduler is installed.

>  		ip_vs_unbind_scheduler(svc, sched);
>  		ip_vs_service_free(svc);

Regards

--
Julian Anastasov <ja@ssi.bg>

Re: [PATCH net] ipvs: fix NULL deref in ip_vs_add_service error path

[Weiming Shi]

On 26-04-01 09:38, Julian Anastasov wrote:
> 
> 	Hello,
> 
> On Wed, 1 Apr 2026, Weiming Shi wrote:
> 
> > When ip_vs_bind_scheduler() succeeds in ip_vs_add_service(), the local
> > variable sched is set to NULL.  If ip_vs_start_estimator() subsequently
> > fails, the out_err cleanup calls ip_vs_unbind_scheduler(svc, sched)
> > with sched == NULL.  ip_vs_unbind_scheduler() passes the cur_sched NULL
> > check (because svc->scheduler was set by the successful bind) but then
> > dereferences the NULL sched parameter at sched->done_service, causing a
> > kernel panic at offset 0x30 from NULL.
> > 
> >  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000006: 0000 [#1] PREEMPT SMP KASAN NOPTI
> >  KASAN: null-ptr-deref in range [0x0000000000000030-0x0000000000000037]
> >  RIP: 0010:ip_vs_unbind_scheduler (net/netfilter/ipvs/ip_vs_sched.c:69)
> >  Call Trace:
> >   <TASK>
> >   ip_vs_add_service.isra.0 (net/netfilter/ipvs/ip_vs_ctl.c:1500)
> >   do_ip_vs_set_ctl (net/netfilter/ipvs/ip_vs_ctl.c:2809)
> >   nf_setsockopt (net/netfilter/nf_sockopt.c:102)
> >   ip_setsockopt (net/ipv4/ip_sockglue.c:1427)
> >   raw_setsockopt (net/ipv4/raw.c:850)
> >   do_sock_setsockopt (net/socket.c:2322)
> >   __sys_setsockopt (net/socket.c:2339)
> >   __x64_sys_setsockopt (net/socket.c:2350)
> >   do_syscall_64 (arch/x86/entry/syscall_64.c:94)
> >   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
> >   </TASK>
> > 
> > Fix by recovering the scheduler pointer from svc->scheduler before
> > cleanup when the local sched variable has been cleared.  This also
> > prevents a latent module refcount leak: without the recovery,
> > ip_vs_scheduler_put(sched) receives NULL and skips the module_put(),
> > so the scheduler module could never be unloaded if the kernel survived
> > past the dereference.
> > 
> > Fixes: 05f00505a89a ("ipvs: fix crash if scheduler is changed")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> >  net/netfilter/ipvs/ip_vs_ctl.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
> > index 35642de2a0fee..e0c978def9749 100644
> > --- a/net/netfilter/ipvs/ip_vs_ctl.c
> > +++ b/net/netfilter/ipvs/ip_vs_ctl.c
> > @@ -1497,6 +1497,8 @@ 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) {
> > +		if (!sched)
> > +			sched = rcu_dereference_protected(svc->scheduler, 1);
> 
> 	Good catch. But may be it should be enough if
> we just remove the sched = NULL after successful
> ip_vs_bind_scheduler(), what do you think? ip_vs_unbind_scheduler()
> already detects if the scheduler is installed.
> 
> >  		ip_vs_unbind_scheduler(svc, sched);
> >  		ip_vs_service_free(svc);
> 
> Regards
> 
> --
> Julian Anastasov <ja@ssi.bg>
> 

Hi Julian,

Thanks for the review. You're right, removing the sched = NULL is
simpler and sufficient

I'll send a v2 patch.

Best,
Weiming Shi