2008-10-30 15:03:53

by Jouni Malinen

[permalink] [raw]
Subject: [PATCHv2 5/5] mac80211_hwsim: Make sure beacon_timer gets deleted

It was possible to trigger a kernel panic because beacon_timer may not
have been deleted in all cases when the kernel module was removed while
hostapd was still running.

Signed-off-by: Jouni Malinen <[email protected]>

Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
===================================================================
--- wireless-testing.orig/drivers/net/wireless/mac80211_hwsim.c 2008-10-30 16:11:33.000000000 +0200
+++ wireless-testing/drivers/net/wireless/mac80211_hwsim.c 2008-10-30 16:26:49.000000000 +0200
@@ -290,6 +290,7 @@
{
struct mac80211_hwsim_data *data = hw->priv;
data->started = 0;
+ del_timer(&data->beacon_timer);
printk(KERN_DEBUG "%s:%s\n", wiphy_name(hw->wiphy), __func__);
}


--

--
Jouni Malinen PGP id EFC895FA


2008-10-30 15:42:23

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] mac80211_hwsim: Make sure beacon_timer gets deleted

On Thu, Oct 30, 2008 at 04:08:40PM +0100, Johannes Berg wrote:
> On Thu, 2008-10-30 at 16:59 +0200, Jouni Malinen wrote:
> > Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
> > @@ -290,6 +290,7 @@
> > {
> > struct mac80211_hwsim_data *data = hw->priv;
> > data->started = 0;
> > + del_timer(&data->beacon_timer);
>
> should that be del_timer_sync, just in case? Not sure how that works
> with the rearming thing, I never really understood rearming timers.

No idea.. I have to admit I don't care that much in this particular
case (hwsim is a test driver), but if someone can explain how this is
supposed to be done properly, I would be happy to fix this ;-).

That started=0 will prevent future timer handlers from using add_timer,
but a currently running handler may miss that and as such,
del_timer_sync() might not be enough. I would assume calling
del_timer_sync() here twice would take care of that corner case, but
that looks somewhat funny.

John, can you at least apply the first four patches from this series? I
would be fine with getting the fifth one in as-is, too, since it does at
least make it quite a bit less likely to hit the kernel panic. We can
then update this if someone can tell what the recommended way of killing
this type of timer is.

--
Jouni Malinen PGP id EFC895FA

2008-10-30 15:08:46

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCHv2 5/5] mac80211_hwsim: Make sure beacon_timer gets deleted

On Thu, 2008-10-30 at 16:59 +0200, Jouni Malinen wrote:
> plain text document attachment (hwsim_del_timer.patch)
> It was possible to trigger a kernel panic because beacon_timer may not
> have been deleted in all cases when the kernel module was removed while
> hostapd was still running.
>
> Signed-off-by: Jouni Malinen <[email protected]>
>
> Index: wireless-testing/drivers/net/wireless/mac80211_hwsim.c
> ===================================================================
> --- wireless-testing.orig/drivers/net/wireless/mac80211_hwsim.c 2008-10-30 16:11:33.000000000 +0200
> +++ wireless-testing/drivers/net/wireless/mac80211_hwsim.c 2008-10-30 16:26:49.000000000 +0200
> @@ -290,6 +290,7 @@
> {
> struct mac80211_hwsim_data *data = hw->priv;
> data->started = 0;
> + del_timer(&data->beacon_timer);

should that be del_timer_sync, just in case? Not sure how that works
with the rearming thing, I never really understood rearming timers.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part