[PATCH] net: wireless: ath: ath9k: Fix a possible data race in ath_chanctx_set_next

2018-05-08 Thread Jia-Ju Bai 
The write operation to "sc->next_chan" is protected by
the lock on line 1287, but the read operation to
this data on line 1262 is not protected by the lock.
Thus, there may exist a data race for "sc->next_chan".

To fix this data race, the read operation to "sc->next_chan" 
should be also protected by the lock.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/ath/ath9k/channel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath9k/channel.c 
b/drivers/net/wireless/ath/ath9k/channel.c
index 1b05b5d7a038..ed3cd5523481 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -1257,12 +1257,12 @@ void ath_chanctx_set_next(struct ath_softc *sc, bool 
force)
"Stopping current chanctx: %d\n",
sc->cur_chan->chandef.center_freq1);
sc->cur_chan->stopped = true;
-   spin_unlock_bh(>chan_lock);
 
if (sc->next_chan == >offchannel.chan) {
getrawmonotonic();
measure_time = true;
}
+   spin_unlock_bh(>chan_lock);
 
ath9k_chanctx_stop_queues(sc, sc->cur_chan);
queues_stopped = true;
-- 
2.17.0

[PATCH] net: wireless: b43legacy: Replace GFP_ATOMIC with GFP_KERNEL in dma_tx_fragment

2018-04-10 Thread Jia-Ju Bai 
dma_tx_fragment() is never called in atomic context.

dma_tx_fragment() is only called by b43legacy_dma_tx(), which is 
only called by b43legacy_tx_work().
b43legacy_tx_work() is only set a parameter of INIT_WORK() in 
b43legacy_wireless_init().

Despite never getting called from atomic context,
dma_tx_fragment() calls alloc_skb() with GFP_ATOMIC,
which does not sleep for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
which can sleep and improve the possibility of sucessful allocation.

This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/broadcom/b43legacy/dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/dma.c 
b/drivers/net/wireless/broadcom/b43legacy/dma.c
index cfa617d..2f0c64c 100644
--- a/drivers/net/wireless/broadcom/b43legacy/dma.c
+++ b/drivers/net/wireless/broadcom/b43legacy/dma.c
@@ -1064,7 +1064,7 @@ static int dma_tx_fragment(struct b43legacy_dmaring *ring,
meta->dmaaddr = map_descbuffer(ring, skb->data, skb->len, 1);
/* create a bounce buffer in zone_dma on mapping failure. */
if (b43legacy_dma_mapping_error(ring, meta->dmaaddr, skb->len, 1)) {
-   bounce_skb = alloc_skb(skb->len, GFP_ATOMIC | GFP_DMA);
+   bounce_skb = alloc_skb(skb->len, GFP_KERNEL | GFP_DMA);
if (!bounce_skb) {
ring->current_slot = old_top_slot;
ring->used_slots = old_used_slots;
-- 
1.9.1

[PATCH] net: wireless: cisco: airo: Replace mdelay with usleep_range in flashgchar

2018-04-10 Thread Jia-Ju Bai 
flashgchar() is never called in atomic context.

flashgchar() is only called by flashcard().
flashcard() calls copy_from_user() and kmalloc(GFP_KERNEL), which 
indicates this function is not called in atomic context.

Despite never getting called from atomic context, flashgchar()
calls mdelay() to busily wait.
This is not necessary and can be replaced with usleep_range() to
avoid busy waiting.

This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/cisco/airo.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/cisco/airo.c 
b/drivers/net/wireless/cisco/airo.c
index 54201c0..45747b7 100644
--- a/drivers/net/wireless/cisco/airo.c
+++ b/drivers/net/wireless/cisco/airo.c
@@ -8106,7 +8106,7 @@ static int flashgchar(struct airo_info *ai,int 
matchbyte,int dwelltime){
 
if(dwelltime && !(0x8000 & rchar)){
dwelltime -= 10;
-   mdelay(10);
+   usleep_range(1, 11000);
continue;
}
rbyte = 0xff & rchar;
-- 
1.9.1

[PATCH 2/2] net: wireless: zydas: Replace GFP_ATOMIC with GFP_KERNEL in zd1201_fw_upload

2018-04-10 Thread Jia-Ju Bai 
zd1201_probe() is never called in atomic context.

zd1201_fw_upload() is only called by zd1201_probe(), 
which is only set as ".probe" in struct usb_driver.

Despite never getting called from atomic context,
zd1201_fw_upload() calls kmalloc() with GFP_ATOMIC,
which does not sleep for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
which can sleep and improve the possibility of sucessful allocation.

This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/zydas/zd1201.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/zydas/zd1201.c 
b/drivers/net/wireless/zydas/zd1201.c
index 581e857..cba2bed 100644
--- a/drivers/net/wireless/zydas/zd1201.c
+++ b/drivers/net/wireless/zydas/zd1201.c
@@ -74,7 +74,7 @@ static int zd1201_fw_upload(struct usb_device *dev, int apfw)
data = fw_entry->data;
 len = fw_entry->size;
 
-   buf = kmalloc(1024, GFP_ATOMIC);
+   buf = kmalloc(1024, GFP_KERNEL);
if (!buf) {
err = -ENOMEM;
goto exit;
-- 
1.9.1

[PATCH 1/2] net: wireless: zydas: Replace mdelay with msleep in zd1201_probe

2018-04-10 Thread Jia-Ju Bai 
zd1201_probe() is never called in atomic context.

zd1201_probe() is only set as ".probe" in struct usb_driver.

Despite never getting called from atomic context, zd1201_probe()
calls mdelay() to busily wait.
This is not necessary and can be replaced with msleep() to
avoid busy waiting.

This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/zydas/zd1201.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/zydas/zd1201.c 
b/drivers/net/wireless/zydas/zd1201.c
index 581e857..12774e9 100644
--- a/drivers/net/wireless/zydas/zd1201.c
+++ b/drivers/net/wireless/zydas/zd1201.c
@@ -1767,7 +1767,7 @@ static int zd1201_probe(struct usb_interface *interface,
goto err_zd;
}
 
-   mdelay(100);
+   msleep(100);
err = zd1201_drvr_start(zd);
if (err)
goto err_zd;
-- 
1.9.1

Re: [PATCH] bcma: Replace mdelay with usleep_range in bcma_pmu_resources_init

2018-01-26 Thread Jia-Ju Bai 



On 2018/1/27 0:26, Larry Finger wrote:

On 01/26/2018 03:13 AM, Jia-Ju Bai wrote:
After checking all possible call chains to bcma_pmu_resources_init() 
here,

my tool finds that this function is never called in atomic context,
namely never in an interrupt handler or holding a spinlock.
Thus mdelay can be replaced with usleep_range to avoid busy wait.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
  drivers/bcma/driver_chipcommon_pmu.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bcma/driver_chipcommon_pmu.c 
b/drivers/bcma/driver_chipcommon_pmu.c

index f1eb4d3..478948c 100644
--- a/drivers/bcma/driver_chipcommon_pmu.c
+++ b/drivers/bcma/driver_chipcommon_pmu.c
@@ -203,7 +203,7 @@ static void bcma_pmu_resources_init(struct 
bcma_drv_cc *cc)

   * Add some delay; allow resources to come up and settle.
   * Delay is required for SoC (early init).
   */
-mdelay(2);
+usleep_range(1500, 2000);


I have no idea how critical this delay might be, but it would be safer 
to never make the sleep be shorter than the original delay. Using 
(2000, 2500) would be a better choice of arguments for usleep_range().


Okay, I have used usleep_range(2000, 2500) and sent patch v2.


Thanks,
Jia-Ju Bai

[PATCH] bcma: Replace mdelay with usleep_range in bcma_pmu_resources_init

2018-01-26 Thread Jia-Ju Bai 
After checking all possible call chains to bcma_pmu_resources_init() here,
my tool finds that this function is never called in atomic context,
namely never in an interrupt handler or holding a spinlock.
Thus mdelay can be replaced with usleep_range to avoid busy wait.

This is found by a static analysis tool named DCNS written by myself.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/bcma/driver_chipcommon_pmu.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bcma/driver_chipcommon_pmu.c 
b/drivers/bcma/driver_chipcommon_pmu.c
index f1eb4d3..478948c 100644
--- a/drivers/bcma/driver_chipcommon_pmu.c
+++ b/drivers/bcma/driver_chipcommon_pmu.c
@@ -203,7 +203,7 @@ static void bcma_pmu_resources_init(struct bcma_drv_cc *cc)
 * Add some delay; allow resources to come up and settle.
 * Delay is required for SoC (early init).
 */
-   mdelay(2);
+   usleep_range(1500, 2000);
 }
 
 /* Disable to allow reading SPROM. Don't know the adventages of enabling it. */
-- 
1.7.9.5

Re: [PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init_post

2018-01-09 Thread Jia-Ju Bai 



On 2018/1/9 17:07, Arend van Spriel wrote:

On 1/9/2018 9:39 AM, Jia-Ju Bai wrote:



On 2018/1/9 16:35, Greg KH wrote:

On Tue, Jan 09, 2018 at 09:40:06AM +0800, Jia-Ju Bai wrote:

b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range,
to reduce busy wait.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
v2:
* Replace mdelay with usleep_range, instead of msleep in v1.
   Thank Larry for good advice.
---
  drivers/net/wireless/broadcom/b43/phy_n.c |2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct
b43_wldev *dev)
  b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
  b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
-mdelay(2);
+usleep_range(2000, 3000);

Where did 3000 come from?  Are you sure about that?


I am not very sure, and I use it according to Larry's message:


Hi Jia-Ju Bai,

The duration here is for settling the registers so hardware can pick 
it up. Right after this they are written again. Now this is during 
initialization of the radio so not time critical, but probably 
anything in the range of 2000..3000 would also have been fine.


Hi Arend,

Thanks for your detailed explanation :)
So I think usleep_range(2000, 3000) is okay.


Thanks,
Jia-Ju Bai

[PATCH v2] b43: Replace mdelay with usleep_range in b43_radio_2057_init_post

2018-01-08 Thread Jia-Ju Bai 
b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with usleep_range, 
to reduce busy wait.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
v2:
* Replace mdelay with usleep_range, instead of msleep in v1.
  Thank Larry for good advice.
---
 drivers/net/wireless/broadcom/b43/phy_n.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c 
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..f2a2f41 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev 
*dev)
 
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
-   mdelay(2);
+   usleep_range(2000, 3000);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);
 
-- 
1.7.9.5

Re: b43: Replace mdelay with msleep in b43_radio_2057_init_post

2018-01-08 Thread Jia-Ju Bai 



On 2018/1/9 0:31, Larry Finger wrote:

On 01/08/2018 10:21 AM, Kalle Valo wrote:

Jia-Ju Bai <baijiaju1...@gmail.com> wrote:


b43_radio_2057_init_post is not called in an interrupt handler
nor holding a spinlock.
The function mdelay in it can be replaced with msleep, to reduce 
busy wait.


Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>


You submitted an identical patch a week earlier:

https://patchwork.kernel.org/patch/10137671/

How is this different? Also always add version number to the patch so 
that the

maintainers can follow the changes easily:

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing 



https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#changelog_missing 



I had negative comments on one of those due to the possibility of 
msleep(2) extending as long as 20 msec. Until the author, or someone 
else, can test that this is OK, then the mdelay(2) can only be 
replaced with usleep_range(2000, 3000).


NACK for both.

Larry



Sorry for my mistake.
I have sent a patch v2 using usleep_range(2000, 3000), and you can have 
a look :)



Thanks,
Jia-Ju Bai

[PATCH] b43: Replace mdelay with msleep in b43_radio_2057_init_post

2017-12-30 Thread Jia-Ju Bai 
b43_radio_2057_init_post is not called in an interrupt handler 
nor holding a spinlock.
The function mdelay in it can be replaced with msleep, to reduce busy wait.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c 
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..5bc838e 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev 
*dev)
 
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
-   mdelay(2);
+   msleep(2);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);
 
-- 
1.7.9.5

[PATCH] b43: Replace mdelay with msleep in b43_radio_2057_init_post

2017-12-23 Thread Jia-Ju Bai 
b43_radio_2057_init_post is not called in an interrupt handler 
nor holding a spinlock.
The function mdelay in it can be replaced with msleep, to reduce busy wait.

Signed-off-by: Jia-Ju Bai <baijiaju1...@gmail.com>
---
 drivers/net/wireless/broadcom/b43/phy_n.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/b43/phy_n.c 
b/drivers/net/wireless/broadcom/b43/phy_n.c
index a5557d7..5bc838e 100644
--- a/drivers/net/wireless/broadcom/b43/phy_n.c
+++ b/drivers/net/wireless/broadcom/b43/phy_n.c
@@ -1031,7 +1031,7 @@ static void b43_radio_2057_init_post(struct b43_wldev 
*dev)
 
b43_radio_set(dev, R2057_RFPLL_MISC_CAL_RESETN, 0x78);
b43_radio_set(dev, R2057_XTAL_CONFIG2, 0x80);
-   mdelay(2);
+   msleep(2);
b43_radio_mask(dev, R2057_RFPLL_MISC_CAL_RESETN, ~0x78);
b43_radio_mask(dev, R2057_XTAL_CONFIG2, ~0x80);
 
-- 
1.7.9.5

[PATCH] mac80211_hwsim: Fix a possible sleep-in-atomic bug in hwsim_get_radio_nl

2017-12-12 Thread Jia-Ju Bai 
The driver may sleep under a spinlock.
The function call path is:
hwsim_get_radio_nl (acquire the spinlock)
  nlmsg_new(GFP_KERNEL) --> may sleep

To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.

This bug is found by my static analysis tool(DSAC) and checked by my code 
review.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/mac80211_hwsim.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mac80211_hwsim.c 
b/drivers/net/wireless/mac80211_hwsim.c
index 10b075a..f2ebf4a 100644
--- a/drivers/net/wireless/mac80211_hwsim.c
+++ b/drivers/net/wireless/mac80211_hwsim.c
@@ -3215,7 +3215,7 @@ static int hwsim_get_radio_nl(struct sk_buff *msg, struct 
genl_info *info)
if (!net_eq(wiphy_net(data->hw->wiphy), genl_info_net(info)))
continue;
 
-   skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+   skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
if (!skb) {
res = -ENOMEM;
goto out_err;
-- 
1.7.9.5

[BUG] wl3501: a possible sleep-in-atomic bug in wl3501_reset

2017-12-12 Thread Jia-Ju Bai 
According to drivers/net/wireless/wl3501_cs.c, the driver may sleep 
under a spinlock.

The function call path is:
wl3501_reset (acquire the spinlock)
  free_irq --> may sleep

I do not find a good way to fix it, so I only report.
This possible bug is found by my static analysis tool (DSAC) and checked 
by my code review.



Thanks,
Jia-Ju Bai

[BUG] ssb: Possible sleep-in-atomic bugs in ssb_pcmcia_read8

2017-10-08 Thread Jia-Ju Bai 

According to pcmcia.c, the driver may sleep under a spinlock.
The function call paths are:
ssb_pcmcia_read8 (acquire the spinlock)
  select_core_and_segment
ssb_pcmcia_switch_segment
  ssb_pcmcia_cfg_write
pcmcia_write_config_byte
  pcmcia_access_config (drivers/pcmcia/pcmcia_resource.c)
mutex_lock --> may sleep

ssb_pcmcia_read8 (acquire the spinlock)
  select_core_and_segment
ssb_pcmcia_switch_segment
  sssb_pcmcia_cfg_read
pcmcia_read_config_byte
  pcmcia_access_config (drivers/pcmcia/pcmcia_resource.c)
mutex_lock --> may sleep

A possible fix is to use spinlock instead of mutex lock in 
pcmcia_access_config in drivers/pcmcia/pcmcia_resource.c.


These bugs are found by my static analysis tool and my code review.


Thanks,
Jia-Ju Bai

[PATCH] wcn36xx: Remove unnecessary rcu_read_unlock in wcn36xx_bss_info_changed

2017-10-08 Thread Jia-Ju Bai 
No rcu_read_lock is called, but rcu_read_unlock is still called.
Thus rcu_read_unlock should be removed.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/ath/wcn36xx/main.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c 
b/drivers/net/wireless/ath/wcn36xx/main.c
index 35bd50b..b83f01d 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -812,7 +812,6 @@ static void wcn36xx_bss_info_changed(struct ieee80211_hw 
*hw,
if (!sta) {
wcn36xx_err("sta %pM is not found\n",
  bss_conf->bssid);
-   rcu_read_unlock();
goto out;
}
sta_priv = wcn36xx_sta_to_priv(sta);
-- 
1.7.9.5

[PATCH V2] Fix a sleep-in-atomic bug in shash_setkey_unaligned

2017-10-02 Thread Jia-Ju Bai 
The SCTP program may sleep under a spinlock, and the function call path is:
sctp_generate_t3_rtx_event (acquire the spinlock)
  sctp_do_sm
sctp_side_effects
  sctp_cmd_interpreter
sctp_make_init_ack
  sctp_pack_cookie
crypto_shash_setkey
  shash_setkey_unaligned
kmalloc(GFP_KERNEL)

For the same reason, the orinoco driver may sleep in interrupt handler, 
and the function call path is:
orinoco_rx_isr_tasklet
  orinoco_rx
orinoco_mic
  crypto_shash_setkey
shash_setkey_unaligned
  kmalloc(GFP_KERNEL)

To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 crypto/shash.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/shash.c b/crypto/shash.c
index 5e31c8d..8fcecc6 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -41,7 +41,7 @@ static int shash_setkey_unaligned(struct crypto_shash *tfm, 
const u8 *key,
int err;
 
absize = keylen + (alignmask & ~(crypto_tfm_ctx_alignment() - 1));
-   buffer = kmalloc(absize, GFP_KERNEL);
+   buffer = kmalloc(absize, GFP_ATOMIC);
if (!buffer)
return -ENOMEM;
 
-- 
1.7.9.5

[PATCH] Fix a sleep-in-atomic bug in shash_setkey_unaligned

2017-10-02 Thread Jia-Ju Bai 
For the same reason, the orinoco driver may sleep in interrupt handler, 
and the function call path is:
orinoco_rx_isr_tasklet
  orinoco_rx
orinoco_mic
  crypto_shash_setkey
shash_setkey_unaligned
  kmalloc(GFP_KERNEL)

To fix it, GFP_KERNEL is replaced with GFP_ATOMIC.
This bug is found by my static analysis tool and my code review.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 crypto/shash.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/shash.c b/crypto/shash.c
index 5e31c8d..8fcecc6 100644
--- a/crypto/shash.c
+++ b/crypto/shash.c
@@ -41,7 +41,7 @@ static int shash_setkey_unaligned(struct crypto_shash *tfm, 
const u8 *key,
int err;
 
absize = keylen + (alignmask & ~(crypto_tfm_ctx_alignment() - 1));
-   buffer = kmalloc(absize, GFP_KERNEL);
+   buffer = kmalloc(absize, GFP_ATOMIC);
if (!buffer)
return -ENOMEM;
 
-- 
1.7.9.5

[PATCH] cw1200: Fix a sleep-in-atomic bug in cw1200_tx_confirm_cb and cw1200_cqm_bssloss_sm

2017-05-31 Thread Jia-Ju Bai 
The driver may sleep under a spin lock, and the function call path is:
cw1200_tx_confirm_cb (acquire the lock by spin_lock)
  __cw1200_cqm_bssloss_sm
cancel_work_sync --> may sleep

cw1200_cqm_bssloss_sm
  __cw1200_cqm_bssloss_sm
cancel_work_sync --> may sleep

To fix it, the lock is released before cancel_work_sync, and the lock 
is acquired again after this function.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/st/cw1200/sta.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/st/cw1200/sta.c 
b/drivers/net/wireless/st/cw1200/sta.c
index a522248..d5f7698 100644
--- a/drivers/net/wireless/st/cw1200/sta.c
+++ b/drivers/net/wireless/st/cw1200/sta.c
@@ -154,7 +154,9 @@ void __cw1200_cqm_bssloss_sm(struct cw1200_common *priv,
int tx = 0;
 
priv->delayed_link_loss = 0;
+   spin_unlock(>bss_loss_lock);
cancel_work_sync(>bss_params_work);
+   spin_lock(>bss_loss_lock);
 
pr_debug("[STA] CQM BSSLOSS_SM: state: %d init %d good %d bad: %d 
txlock: %d uj: %d\n",
 priv->bss_loss_state,
-- 
1.7.9.5

Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed

2017-05-31 Thread Jia-Ju Bai 

On 06/01/2017 08:07 AM, Larry Finger wrote:

On 05/31/2017 10:32 AM, Michael Büsch wrote:

On Wed, 31 May 2017 13:26:43 +0300
Kalle Valo <kv...@codeaurora.org> wrote:


Jia-Ju Bai <baijiaju1...@163.com> writes:


The driver may sleep under a spin lock, and the function call path is:
b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave)
   b43legacy_synchronize_irq
 synchronize_irq --> may sleep

To fix it, the lock is released before b43legacy_synchronize_irq, 
and the

lock is acquired again after this function.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
  drivers/net/wireless/broadcom/b43legacy/main.c |2 ++
  1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c 
b/drivers/net/wireless/broadcom/b43legacy/main.c

index f1e3dad..31ead21 100644
--- a/drivers/net/wireless/broadcom/b43legacy/main.c
+++ b/drivers/net/wireless/broadcom/b43legacy/main.c
@@ -2859,7 +2859,9 @@ static void 
b43legacy_op_bss_info_changed(struct ieee80211_hw *hw,

  b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
if (changed & BSS_CHANGED_BSSID) {
+spin_unlock_irqrestore(>irq_lock, flags);
  b43legacy_synchronize_irq(dev);
+spin_lock_irqsave(>irq_lock, flags);


To me this looks like a fragile workaround and not a real fix. You can
easily add new race conditions with releasing the lock like this.




I think releasing the lock possibly is fine. It certainly is better than
sleeping with a lock held.
We disabled the device interrupts just before this line.

However I think the synchronize_irq should be outside of the
conditional right after the write to B43legacy_MMIO_GEN_IRQ_MASK. (So
two lines above)
I don't think it makes sense to only synchronize if BSS_CHANGED_BSSID
is set.


On the other hand b43 does not have this irq-disabling foobar anymore.
So somebody must have removed it. Maybe you can find the commit that
removed this stuff from b43 and port it to b43legacy?


So I would vote for moving the synchronize_irq up outside of the
conditional and put the unlock/lock sequence around it.
And as a second patch on top of that try to remove this stuff
altogether like b43 did.


The patch that removed it in b43 is

commit 36dbd9548e92268127b0c31b0e121e63e9207108
Author: Michael Buesch <m...@bu3sch.de>
Date:   Fri Sep 4 22:51:29 2009 +0200

b43: Use a threaded IRQ handler

Use a threaded IRQ handler to allow locking the mutex and
sleeping while executing an interrupt.
This removes usage of the irq_lock spinlock, but introduces
a new hardirq_lock, which is _only_ used for the PCI/SSB lowlevel
hard-irq handler. Sleeping busses (SDIO) will use mutex instead.

Signed-off-by: Michael Buesch <m...@bu3sch.de>
Tested-by: Larry Finger <larry.fin...@lwfinger.net>
Signed-off-by: John W. Linville <linvi...@tuxdriver.com>

I vaguely remember this patch. Although it is roughly a 1000-line fix, 
I will try to port it to b43legacy. I still have an old BCM4306 PCMCIA 
card that I can test in a PowerBook G4.


I agree with Michael that this is the way to go. Both of Jia-Ju's 
patches should be rejected.


Larry




It is fine to me to fix the bug by porting this former patch.

Thanks,
Jia-Ju Bai

Re: [PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store

2017-05-31 Thread Jia-Ju Bai 

On 06/01/2017 01:33 AM, Larry Finger wrote:

On 05/31/2017 05:29 AM, Jia-Ju Bai wrote:

The driver may sleep under a spin lock, and the function call path is:
b43legacy_attr_interfmode_store (acquire the lock by spin_lock_irqsave)
   b43legacy_radio_set_interference_mitigation
 b43legacy_radio_interference_mitigation_disable
   b43legacy_calc_nrssi_slope
 b43legacy_synth_pu_workaround
   might_sleep and msleep --> may sleep

Fixing it may be complex, and a possible way is to remove
spin_lock_irqsave and spin_lock_irqrestore in
b43legacy_attr_interfmode_store, and the code has been protected by
mutex_lock and mutex_unlock.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
  drivers/net/wireless/broadcom/b43legacy/sysfs.c |2 --
  1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/sysfs.c 
b/drivers/net/wireless/broadcom/b43legacy/sysfs.c

index 2a1da15..9ede143 100644
--- a/drivers/net/wireless/broadcom/b43legacy/sysfs.c
+++ b/drivers/net/wireless/broadcom/b43legacy/sysfs.c
@@ -137,14 +137,12 @@ static ssize_t 
b43legacy_attr_interfmode_store(struct device *dev,

  }
mutex_lock(>wl->mutex);
-spin_lock_irqsave(>wl->irq_lock, flags);
err = b43legacy_radio_set_interference_mitigation(wldev, mode);
  if (err)
  b43legacyerr(wldev->wl, "Interference Mitigation not "
 "supported by device\n");
  mmiowb();
-spin_unlock_irqrestore(>wl->irq_lock, flags);
  mutex_unlock(>wl->mutex);
return err ? err : count;



Jia-Ju,

Did you actually observe the attempt to sleep under the spin lock, or 
did you discover this using some tool? In other words, have either of 
your patches been tested?


Larry


Hi,

In fact, my reported bugs are found by a static analysis tool written by 
me, and they are checked by my review of the driver code.

I admit my patches are not well tested, and they may not well fix the bugs.
I am looking forward to opinions and suggestions :)

Thanks,
Jia-Ju Bai

[PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_attr_interfmode_store

2017-05-31 Thread Jia-Ju Bai 
The driver may sleep under a spin lock, and the function call path is:
b43legacy_attr_interfmode_store (acquire the lock by spin_lock_irqsave)
  b43legacy_radio_set_interference_mitigation
b43legacy_radio_interference_mitigation_disable
  b43legacy_calc_nrssi_slope
b43legacy_synth_pu_workaround
  might_sleep and msleep --> may sleep

Fixing it may be complex, and a possible way is to remove 
spin_lock_irqsave and spin_lock_irqrestore in 
b43legacy_attr_interfmode_store, and the code has been protected by
mutex_lock and mutex_unlock.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/broadcom/b43legacy/sysfs.c |2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/b43legacy/sysfs.c 
b/drivers/net/wireless/broadcom/b43legacy/sysfs.c
index 2a1da15..9ede143 100644
--- a/drivers/net/wireless/broadcom/b43legacy/sysfs.c
+++ b/drivers/net/wireless/broadcom/b43legacy/sysfs.c
@@ -137,14 +137,12 @@ static ssize_t b43legacy_attr_interfmode_store(struct 
device *dev,
}
 
mutex_lock(>wl->mutex);
-   spin_lock_irqsave(>wl->irq_lock, flags);
 
err = b43legacy_radio_set_interference_mitigation(wldev, mode);
if (err)
b43legacyerr(wldev->wl, "Interference Mitigation not "
   "supported by device\n");
mmiowb();
-   spin_unlock_irqrestore(>wl->irq_lock, flags);
mutex_unlock(>wl->mutex);
 
return err ? err : count;
-- 
1.7.9.5

[PATCH] b43legacy: Fix a sleep-in-atomic bug in b43legacy_op_bss_info_changed

2017-05-31 Thread Jia-Ju Bai 
The driver may sleep under a spin lock, and the function call path is:
b43legacy_op_bss_info_changed (acquire the lock by spin_lock_irqsave)
  b43legacy_synchronize_irq
synchronize_irq --> may sleep

To fix it, the lock is released before b43legacy_synchronize_irq, and the 
lock is acquired again after this function.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/broadcom/b43legacy/main.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/broadcom/b43legacy/main.c 
b/drivers/net/wireless/broadcom/b43legacy/main.c
index f1e3dad..31ead21 100644
--- a/drivers/net/wireless/broadcom/b43legacy/main.c
+++ b/drivers/net/wireless/broadcom/b43legacy/main.c
@@ -2859,7 +2859,9 @@ static void b43legacy_op_bss_info_changed(struct 
ieee80211_hw *hw,
b43legacy_write32(dev, B43legacy_MMIO_GEN_IRQ_MASK, 0);
 
if (changed & BSS_CHANGED_BSSID) {
+   spin_unlock_irqrestore(>irq_lock, flags);
b43legacy_synchronize_irq(dev);
+   spin_lock_irqsave(>irq_lock, flags);
 
if (conf->bssid)
memcpy(wl->bssid, conf->bssid, ETH_ALEN);
-- 
1.7.9.5

[PATCH] iwl4965: Fix a memory leak in error handling code of __il4965_up

2016-01-07 Thread Jia-Ju Bai 
When il4965_hw_nic_init in __il4965_up fails, the memory allocated by 
iwl4965_sta_alloc_lq in iwl4965_alloc_bcast_station is not freed.

This patches adds il_dealloc_bcast_stations in the error handling code of 
__il4965_up to fix this problem.

This patch has been tested in real device, and it actually fixes the bug.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/iwlegacy/4965-mac.c |1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c 
b/drivers/net/wireless/iwlegacy/4965-mac.c
index 6656215..fd7b5c7 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -5577,6 +5577,7 @@ __il4965_up(struct il_priv *il)
ret = il4965_hw_nic_init(il);
if (ret) {
IL_ERR("Unable to init nic\n");
+   il_dealloc_bcast_stations(il);
return ret;
}
 
-- 
1.7.9.5


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

Re: [PATCH] rt2x00pci: Disable memory-write-invalidate when the driver exits

2016-01-04 Thread Jia-Ju Bai 

On 01/05/2016 12:50 AM, Helmut Schaa wrote:

On Mon, Jan 4, 2016 at 8:55 AM, Jia-Ju Bai<baijiaju1...@163.com>  wrote:

The driver calls pci_set_mwi to enable memory-write-invalidate when it
is initialized, but does not call pci_clear_mwi when it is removed. Many
other drivers calls pci_clear_mwi when pci_set_mwi is called, such as
r8169, 8139cp and e1000.

This patch adds pci_clear_mwi in error handling and removal procedure,
which can fix the problem.

Signed-off-by: Jia-Ju Bai<baijiaju1...@163.com>

Looks good to me.
Does this fix any actual issue?
If yes it might we worth to mention it in the commit message.
Helmut



Lacking pci_clear_mwi may cause a resource-release omission,
but this omission may not cause obvious issues.
For reliability, it is better to add pci_clear_mwi in the driver.
Many other drivers do so, such as r8169, 8139cp and e1000.


Jia-Ju Bai

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

[PATCH] rt2x00pci: Disable memory-write-invalidate when the driver exits

2016-01-03 Thread Jia-Ju Bai 
The driver calls pci_set_mwi to enable memory-write-invalidate when it
is initialized, but does not call pci_clear_mwi when it is removed. Many
other drivers calls pci_clear_mwi when pci_set_mwi is called, such as
r8169, 8139cp and e1000.

This patch adds pci_clear_mwi in error handling and removal procedure, 
which can fix the problem.

Signed-off-by: Jia-Ju Bai <baijiaju1...@163.com>
---
 drivers/net/wireless/rt2x00/rt2x00pci.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/rt2x00/rt2x00pci.c 
b/drivers/net/wireless/rt2x00/rt2x00pci.c
index d93db4b..eb6dbcd 100644
--- a/drivers/net/wireless/rt2x00/rt2x00pci.c
+++ b/drivers/net/wireless/rt2x00/rt2x00pci.c
@@ -149,6 +149,7 @@ exit_free_device:
ieee80211_free_hw(hw);
 
 exit_release_regions:
+   pci_clear_mwi(pci_dev);
pci_release_regions(pci_dev);
 
 exit_disable_device:
@@ -173,6 +174,7 @@ void rt2x00pci_remove(struct pci_dev *pci_dev)
/*
 * Free the PCI device data.
 */
+   pci_clear_mwi(pci_dev);
pci_disable_device(pci_dev);
pci_release_regions(pci_dev);
 }
-- 
1.7.9.5


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