Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1415976 > unrolled thread

[ldv-project] [net] libertas: potential race condition

Back to article view | Back to linux.kernel

Contents

  [ldv-project] [net] libertas: potential race condition Pavel Andrianov <andrianov@ispras.ru> - 2016-06-07 12:40 +0200
    Re: [ldv-project] [net] libertas: potential race condition Dan Williams <dcbw@redhat.com> - 2016-06-07 16:50 +0200
      Re: [ldv-project] [net] libertas: potential race condition Pavel Andrianov <andrianov@ispras.ru> - 2016-06-07 18:00 +0200
      Re: [ldv-project] [net] libertas: potential race condition James Cameron <quozl@laptop.org> - 2016-06-08 01:00 +0200

#1415976 — [ldv-project] [net] libertas: potential race condition

Hi!

There is a potential race condition in 
drivers/net/wireless/libertas/libertas.ko.
In the function lbs_hard_start_xmit(..), line 159, a socket buffer is 
written to priv->current_skb with a spin_lock protection.
In the function lbs_mac_event_disconnected(..), lines 50-51, the field 
current_skb is cleaned. There is no protection used. The corresponding 
handlers are activated at the same time in lbs_start_card(..) and then 
may be executed simultaneously. Note, there are two structures 
lbs_netdev_ops and mesh_netdev_ops, which have the target handler 
lbs_hard_start_xmit.
Is it a real race or I have missed something?

-- 
Pavel Andrianov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: andrianov@ispras.ru

[toc] | [next] | [standalone]

#1416271

On Tue, 2016-06-07 at 13:30 +0400, Pavel Andrianov wrote:
> Hi!
> 
> There is a potential race condition in 
> drivers/net/wireless/libertas/libertas.ko.
> In the function lbs_hard_start_xmit(..), line 159, a socket buffer
> is 
> written to priv->current_skb with a spin_lock protection.
> In the function lbs_mac_event_disconnected(..), lines 50-51, the
> field 
> current_skb is cleaned. There is no protection used. The
> corresponding 
> handlers are activated at the same time in lbs_start_card(..) and
> then 
> may be executed simultaneously. Note, there are two structures 
> lbs_netdev_ops and mesh_netdev_ops, which have the target handler 
> lbs_hard_start_xmit.
> Is it a real race or I have missed something?

Yeah, it looks like it should be grabbing priv->driver_lock before
clearing priv->currenttxskb in lbs_mac_event_disconnected().  Care to
submit a patch after testing?  Do you have any of that hardware?

Dan

[toc] | [prev] | [next] | [standalone]

#1416341

07.06.2016 18:39, Dan Williams пишет:
> On Tue, 2016-06-07 at 13:30 +0400, Pavel Andrianov wrote:
>> Hi!
>>
>> There is a potential race condition in
>> drivers/net/wireless/libertas/libertas.ko.
>> In the function lbs_hard_start_xmit(..), line 159, a socket buffer
>> is
>> written to priv->current_skb with a spin_lock protection.
>> In the function lbs_mac_event_disconnected(..), lines 50-51, the
>> field
>> current_skb is cleaned. There is no protection used. The
>> corresponding
>> handlers are activated at the same time in lbs_start_card(..) and
>> then
>> may be executed simultaneously. Note, there are two structures
>> lbs_netdev_ops and mesh_netdev_ops, which have the target handler
>> lbs_hard_start_xmit.
>> Is it a real race or I have missed something?
> Yeah, it looks like it should be grabbing priv->driver_lock before
> clearing priv->currenttxskb in lbs_mac_event_disconnected().  Care to
> submit a patch after testing?  Do you have any of that hardware?
>
> Dan
I have no that hardware and I have some doubts about the simple fix, 
you've suggested. For instance, in lbs_hard_start_xmit the lock is 
acquired twice and the priv->tx_pending_len can be modified also by 
lbs_mac_event_disconnected (even if spin_lock will be added to 
lbs_mac_event_disconnected). Moreover, the function lbs_send_tx_feedback 
also cleaned priv->currenttxskb, but it happens also without any 
protection. Thus, the fix has to be more complicated, and I have no 
ideas about it.

-- 
Pavel Andrianov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: andrianov@ispras.ru

[toc] | [prev] | [next] | [standalone]

#1416710

On Tue, Jun 07, 2016 at 09:39:55AM -0500, Dan Williams wrote:
> On Tue, 2016-06-07 at 13:30 +0400, Pavel Andrianov wrote:
> > Hi!
> > 
> > There is a potential race condition in 
> > drivers/net/wireless/libertas/libertas.ko.
> > In the function lbs_hard_start_xmit(..), line 159, a socket buffer
> > is 
> > written to priv->current_skb with a spin_lock protection.
> > In the function lbs_mac_event_disconnected(..), lines 50-51, the
> > field 
> > current_skb is cleaned. There is no protection used. The
> > corresponding 
> > handlers are activated at the same time in lbs_start_card(..) and
> > then 
> > may be executed simultaneously. Note, there are two structures 
> > lbs_netdev_ops and mesh_netdev_ops, which have the target handler 
> > lbs_hard_start_xmit.
> > Is it a real race or I have missed something?
> 
> Yeah, it looks like it should be grabbing priv->driver_lock before
> clearing priv->currenttxskb in lbs_mac_event_disconnected().  Care to
> submit a patch after testing?  Do you have any of that hardware?

I've hardware, with serial console.

Can test any patch, on USB (8388) or SDIO (8686).

-- 
James Cameron
http://quozl.netrek.org/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web