Search Linux Wireless

Re: [PATCH v2] iwlwifi: fix oops in iwl3945_led_brightness_set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Jun 8, 2008 at 2:13 PM, Marcin Slusarz <marcin.slusarz@xxxxxxxxx> wrote:
> On Sun, Jun 08, 2008 at 01:18:36PM +0300, Tomas Winkler wrote:
>> On Sun, Jun 8, 2008 at 11:48 AM, Marcin Slusarz
>> <marcin.slusarz@xxxxxxxxx> wrote:
>> > fix race between:
>> > ieee80211_open->ieee80211_led_radio->led_trigger_event->led_set_brightness->iwl3945_led_brightness_set
>> > (which assumes that "led->priv" is not NULL)
>> > and
>> > iwl3945_pci_probe->iwl3945_setup_deferred_work->(...)->iwl3945_bg_alive_start->iwl3945_alive_start->iwl3945_led_register->iwl3945_led_register_led
>> > which sets priv field in struct iwl3945_led
>> > after
>> > led->led_dev.brightness_set = iwl3945_led_brightness_set;
>> > (...)
>> > led_classdev_register(device, &led->led_dev);
>> >
>> > http://kerneloops.org/guilty.php?guilty=iwl3945_led_brightness_set&version=2.6.25-release&start=1671168&end=1703935&class=oops
>> >
>> > Signed-off-by: Marcin Slusarz <marcin.slusarz@xxxxxxxxx>
>> > Cc: Zhu Yi <yi.zhu@xxxxxxxxx>
>> > Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
>> > Cc: Tomas Winkler <tomas.winkler@xxxxxxxxx>
>> > Cc: John W. Linville <linville@xxxxxxxxxxxxx>
>> > Cc: linux-wireless@xxxxxxxxxxxxxxx
>> > Cc: ipw3945-devel@xxxxxxxxxxxxxxxxxxxxx
>> > ---
>> >
>> > Compile tested only. Please review and test on real hardware.
>> >
>> > ---
>> >  drivers/net/wireless/iwlwifi/iwl-3945-led.c |    2 ++
>> >  1 files changed, 2 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-led.c b/drivers/net/wireless/iwlwifi/iwl-3945-led.c
>> > index d200d08..9f08568 100644
>> > --- a/drivers/net/wireless/iwlwifi/iwl-3945-led.c
>> > +++ b/drivers/net/wireless/iwlwifi/iwl-3945-led.c
>> > @@ -185,6 +185,8 @@ static void iwl3945_led_brightness_set(struct led_classdev *led_cdev,
>> >                                               struct iwl3945_led, led_dev);
>> >        struct iwl3945_priv *priv = led->priv;
>> >
>> > +       if (!led->registered)
>> > +               return;
>> >        if (test_bit(STATUS_EXIT_PENDING, &priv->status))
>> >                return;
>> >
>> > --
>> > 1.5.4.5
>> >
>> NACK
>> Just need to revert the order in registration command as done in
>> iwl_leds_register_led.
>
> Thanks for a review!
> Updated patch below.
>
> ---
> fix race between:
> ieee80211_open->ieee80211_led_radio->led_trigger_event->led_set_brightness->iwl3945_led_brightness_set
> (which assumes that "led->priv" is not NULL)
> and
> iwl3945_pci_probe->iwl3945_setup_deferred_work->(...)->iwl3945_bg_alive_start->iwl3945_alive_start->iwl3945_led_register->iwl3945_led_register_led
> which sets priv field in struct iwl3945_led
> after
> led->led_dev.brightness_set = iwl3945_led_brightness_set;
> (...)
> led_classdev_register(device, &led->led_dev);
>
> http://kerneloops.org/guilty.php?guilty=iwl3945_led_brightness_set&version=2.6.25-release&start=1671168&end=1703935&class=oops
>
> Signed-off-by: Marcin Slusarz <marcin.slusarz@xxxxxxxxx>
> Cc: Zhu Yi <yi.zhu@xxxxxxxxx>
> Cc: Reinette Chatre <reinette.chatre@xxxxxxxxx>
> Cc: Tomas Winkler <tomas.winkler@xxxxxxxxx>
> Cc: John W. Linville <linville@xxxxxxxxxxxxx>
> Cc: linux-wireless@xxxxxxxxxxxxxxx
> Cc: ipw3945-devel@xxxxxxxxxxxxxxxxxxxxx
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945-led.c |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945-led.c b/drivers/net/wireless/iwlwifi/iwl-3945-led.c
> index d200d08..8b1528e 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945-led.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945-led.c
> @@ -229,14 +229,15 @@ static int iwl3945_led_register_led(struct iwl3945_priv *priv,
>        led->led_dev.brightness_set = iwl3945_led_brightness_set;
>        led->led_dev.default_trigger = trigger;
>
> +       led->priv = priv;
> +       led->type = type;
> +
>        ret = led_classdev_register(device, &led->led_dev);
>        if (ret) {
>                IWL_ERROR("Error: failed to register led handler.\n");
>                return ret;
>        }
>
> -       led->priv = priv;
> -       led->type = type;
>        led->registered = 1;
>
>        if (set_led && led->led_on)
> --
> 1.5.4.5
ACK

Tomas
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]
  Powered by Linux