08.06.2016 02:51, James Cameron пишет:
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).
Hi!
I've prepare the patch for this issue. Could you test it?
Thank you.
--
Pavel Andrianov
Linux Verification Center, ISPRAS
web: http://linuxtesting.org
e-mail: andria...@ispras.ru
>From b1a9e157ccdf8650da93844fd2dbdb6fca509b59 Mon Sep 17 00:00:00 2001
From: Pavel <andria...@ispras.ru>
Date: Tue, 14 Jun 2016 16:59:00 +0400
Subject: [PATCH] libertas: Add spinlock to avoid race condition
lbs_mac_event_disconnected may free priv->currenttxskb
while lbs_hard_start_xmit accesses to it.
The patch adds a spinlock for mutual exclusion.
Signed-off-by: Pavel <andria...@ispras.ru>
---
drivers/net/wireless/marvell/libertas/cmdresp.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/wireless/marvell/libertas/cmdresp.c b/drivers/net/wireless/marvell/libertas/cmdresp.c
index c95bf6d..c67ae07 100644
--- a/drivers/net/wireless/marvell/libertas/cmdresp.c
+++ b/drivers/net/wireless/marvell/libertas/cmdresp.c
@@ -27,6 +27,8 @@
void lbs_mac_event_disconnected(struct lbs_private *priv,
bool locally_generated)
{
+ unsigned long flags;
+
if (priv->connect_status != LBS_CONNECTED)
return;
@@ -46,9 +48,11 @@ void lbs_mac_event_disconnected(struct lbs_private *priv,
netif_carrier_off(priv->dev);
/* Free Tx and Rx packets */
+ spin_lock_irqsave(&priv->driver_lock, flags);
kfree_skb(priv->currenttxskb);
priv->currenttxskb = NULL;
priv->tx_pending_len = 0;
+ spin_unlock_irqrestore(&priv->driver_lock, flags);
priv->connect_status = LBS_DISCONNECTED;
--
1.7.11.7
