Fix sched registration race when checking for name collision
Subsystems (2)
PATCH 1/1 Fix sched registration race when checking for name collision
The register_ip_vs_scheduler() checks for the scheduler with the same name under the read-locked __ip_vs_sched_lock, then drops, takes it for writing and puts the scheduler in list.
This is racy, since we can have a race window between the lock being re-locked for writing.
The fix is to search the scheduler with the given name right under the write-locked __ip_vs_sched_lock.
---
net/ipv4/ipvs/ip_vs_sched.c +14 -13
@@ -183,19 +183,6 @@ int register_ip_vs_scheduler(struct ip_vs_scheduler *scheduler)/* increase the module use count */ ip_vs_use_count_inc (); -/* -* Make sure that the scheduler with this name doesn't exist -* in the scheduler list. -*/ - sched =ip_vs_sched_getbyname (scheduler->name); -if (sched) { -ip_vs_scheduler_put (sched); -ip_vs_use_count_dec (); -IP_VS_ERR ("register_ip_vs_scheduler(): [%s] scheduler " -"already existed in the system\n" , scheduler->name); -return -EINVAL ; - } -write_lock_bh (&__ip_vs_sched_lock);if (scheduler->n_list.next != &scheduler->n_list) {
@@ -207,6 +194,20 @@ int register_ip_vs_scheduler(struct ip_vs_scheduler *scheduler) }/* +* Make sure that the scheduler with this name doesn't exist +* in the scheduler list. +*/ +list_for_each_entry (sched, &ip_vs_schedulers, n_list) { +if (strcmp (scheduler->name, sched->name) ==0 ) { +write_unlock_bh (&__ip_vs_sched_lock); +ip_vs_use_count_dec (); +IP_VS_ERR ("register_ip_vs_scheduler(): [%s] scheduler " +"already existed in the system\n" , + scheduler->name); +return -EINVAL ; + } + } +/* * Add it into the d-linked scheduler list */ list_add (&scheduler->n_list, &ip_vs_schedulers);
Quoted 9 lines · Pavel Emelyanov
The
register_ip_vs_scheduler()checks for the scheduler with the same name under the read-locked__ip_vs_sched_lock, then drops, takes it for writing and puts the scheduler in list.This is racy, since we can have a race window between the lock being re-locked for writing.
The fix is to search the scheduler with the given name right under the write-locked
__ip_vs_sched_lock.
This looks correct to me.
Quoted 1 line
Signed-off-by Pavel Emelyanov <[email protected]>
Quoted 47 lines
---
net/ipv4/ipvs/ip_vs_sched.c +14
-13@@ -183,19 +183,6 @@ int register_ip_vs_scheduler(struct ip_vs_scheduler *scheduler)/* increase the module use count */ ip_vs_use_count_inc (); -/* -* Make sure that the scheduler with this name doesn't exist -* in the scheduler list. -*/ - sched =ip_vs_sched_getbyname (scheduler->name); -if (sched) { -ip_vs_scheduler_put (sched); -ip_vs_use_count_dec (); -IP_VS_ERR ("register_ip_vs_scheduler(): [%s] scheduler " -"already existed in the system\n" , scheduler->name); -return -EINVAL ; - } -write_lock_bh (&__ip_vs_sched_lock);if (scheduler->n_list.next != &scheduler->n_list) {@@ -207,6 +194,20 @@ int register_ip_vs_scheduler(struct ip_vs_scheduler *scheduler) }/* +* Make sure that the scheduler with this name doesn't exist +* in the scheduler list. +*/ +list_for_each_entry (sched, &ip_vs_schedulers, n_list) { +if (strcmp (scheduler->name, sched->name) ==0 ) { +write_unlock_bh (&__ip_vs_sched_lock); +ip_vs_use_count_dec (); +IP_VS_ERR ("register_ip_vs_scheduler(): [%s] scheduler " +"already existed in the system\n" , + scheduler->name); +return -EINVAL ; + } + } +/* * Add it into the d-linked scheduler list */ list_add (&scheduler->n_list, &ip_vs_schedulers);
Signature · 1 line
Horms
David Miller
maintainerFrom: Simon Horman <[email protected]>
Date: Tue, 4 Dec 2007 10:41:43 +0900
Quoted 8 lines · Pavel Emelyanov
Quoted 7 lines
The
register_ip_vs_scheduler()checks for the scheduler with the same name under the read-locked__ip_vs_sched_lock, then drops, takes it for writing and puts the scheduler in list.This is racy, since we can have a race window between the lock being re-locked for writing.
Quoted 8 lines
Quoted 2 lines
The fix is to search the scheduler with the given name right under the write-locked
__ip_vs_sched_lock.This looks correct to me.
Quoted 1 line
Signed-off-by Pavel Emelyanov <[email protected]>Acked-by Simon Horman <[email protected]>
Also applied, thanks a lot.