Re: [patch 20/60] ipvs: Fix possible deadlock in estimator code

From: Simon Horman
Date: Mon Aug 18 2008 - 20:31:48 EST


On Mon, Aug 18, 2008 at 11:43:10AM -0700, Greg KH wrote:
> 2.6.26-stable review patch. If anyone has any objections, please let us know.
>
> ------------------
> From: Sven Wegener <sven.wegener@xxxxxxxxxxx>
>
> commit 8ab19ea36c5c5340ff598e4d15fc084eb65671dc upstream
>
> There is a slight chance for a deadlock in the estimator code. We can't call
> del_timer_sync() while holding our lock, as the timer might be active and
> spinning for the lock on another cpu. Work around this issue by using
> try_to_del_timer_sync() and releasing the lock. We could actually delete the
> timer outside of our lock, as the add and kill functions are only every called
> from userspace via [gs]etsockopt() and are serialized by a mutex, but better
> make this explicit.
>
> Signed-off-by: Sven Wegener <sven.wegener@xxxxxxxxxxx>
> Acked-by: Simon Horman <horms@xxxxxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
>
> ---
> net/ipv4/ipvs/ip_vs_est.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> --- a/net/ipv4/ipvs/ip_vs_est.c
> +++ b/net/ipv4/ipvs/ip_vs_est.c
> @@ -172,8 +172,11 @@ void ip_vs_kill_estimator(struct ip_vs_s
> kfree(est);
> killed++;
> }
> - if (killed && est_list == NULL)
> - del_timer_sync(&est_timer);
> + while (killed && !est_list && try_to_del_timer_sync(&est_timer) < 0) {
> + write_unlock_bh(&est_lock);
> + cpu_relax();
> + write_lock_bh(&est_lock);
> + }
> write_unlock_bh(&est_lock);
> }
>

Hi,

I am comfortable with this change for both 2.6.25-stable and 2.6.26-stable.

An arguably cleaner though more invasive fix, which will likely go
into 2.6.28 is to use init and cleanup fuctions for the estimators
as the empty case can actually only occur during the unload of IPVS.

See "ipvs: Create init functions for estimator code", which was posted by
Sven to netdev & lvs-devel, and is currently living in horms/lvs-2.6.git on
git.kernel.org.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/