netdev - Re: [RFC PATCH] rtl8187: Fix led support for rfkill

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <AANLkTi==LeJvEgmnYaphO1aPeS7MSn3P4v-O_ynb-yNb@mail.gmail.com>
Date:	Fri, 1 Apr 2011 11:12:11 +0800
From:	wu zhangjin <wuzhangjin@...il.com>
To:	Herton Ronaldo Krzesinski <herton.krzesinski@...onical.com>
Cc:	Larry Finger <Larry.Finger@...inger.net>,
	Hin-Tak Leung <htl10@...rs.sourceforge.net>,
	Herton Ronaldo Krzesinski <herton@...onical.com>,
	linux-wireless@...r.kernel.org, netdev@...r.kernel.org,
	Roman Mamedov <rm@...anrm.ru>
Subject: Re: [RFC PATCH] rtl8187: Fix led support for rfkill

On 4/1/11, Herton Ronaldo Krzesinski <herton.krzesinski@...onical.com> wrote:
> On Fri, Apr 01, 2011 at 12:22:51AM +0800, wu zhangjin wrote:
>> On Mon, Mar 28, 2011 at 11:40 PM, Herton Ronaldo Krzesinski
>> <herton.krzesinski@...onical.com> wrote:
>> [...]
>> >>
>> >> My rtl8187 devices are both external USB sticks, thus they have no
>> >> interaction with a radio-kill switch. I will test your patch to make
>> >> sure it does no harm to my system.
>> >>
>> >> I think the commit message should be revised. A simple statement
>> >> like "the LED does not turn off when the rfkill switch is off"
>> >> should be sufficient.
>> >
>> > The patch should work, but I wonder if we should be fiddling with
>> > priv->vif
>> > for this, perhaps we should not assume vif to be valid after
>> > rtl8187_remove_interface (I don't see problems with current
>> > rtl8187/mac80211 code on a quick look, but...)
>> >
>>
>> Yes, but seems only two places have touched the vif pointer.
>>
>> $ grep "vif =" -ur drivers/net/wireless/rtl818x/rtl8187/
>> drivers/net/wireless/rtl818x/rtl8187/dev.c:	priv->vif = NULL;
>> drivers/net/wireless/rtl818x/rtl8187/dev.c:	priv->vif = vif;
>>
>> > I cleaner solution may be to use a priv->mode like p54.
>>
>> Yep, then, we may need to add a 'mode' member to rtl8187_priv and
>> update it properly like p54.
>>
>> So, What do you prefer? I will prepare a new patch asap.
>
> In the led code we dereference priv->vif, checking for priv->vif->type.
> vif is a pointer from mac80211, and after rtl8187_remove_interface is
> called by it, at least from my POV can do anything it wants with vif,
> like freeing the sdata vif points to. From what I checked, it doesn't
> free currently and your patch don't have issues, but may be it's better
> to be safe and add a new "future proof" mode field.
>
> And as your bug shows, the led code access priv->vif after
> rtl8187_remove_interface, so we hit the case where vif could be
> considered invalid.
>
> So because of this and being cleaner I would go adding 'mode'.

Ok, thanks ;-)

Best Regards,
Wu Zhangjin

>
>>
>> Best Regards,
>> Wu Zhangjin
>>
>> >
>> >>
>> >> Larry
>> >>
>> >
>> > --
>> > []'s
>> > Herton
>> >
>>
>
> --
> []'s
> Herton
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html