| Message ID | 20180508080636.13827-1-baijiaju1990@gmail.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | net: wireless: ath: ath9k: Fix a possible data race in ath_chanctx_set_next | expand |
Jia-Ju Bai <baijiaju1990@gmail.com> writes: > 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 <baijiaju1990@gmail.com> I need this reviewed by someone else before I'm willing to take it.
Kalle Valo <kvalo@codeaurora.org> writes: > Jia-Ju Bai <baijiaju1990@gmail.com> writes: > >> 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 <baijiaju1990@gmail.com> > > I need this reviewed by someone else before I'm willing to take it. Only possible issue I can see is that it puts a call to getrawmonotonic() under the spinlock. Not sure if that has any bad implications... -Toke
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(&sc->chan_lock); if (sc->next_chan == &sc->offchannel.chan) { getrawmonotonic(&ts); measure_time = true; } + spin_unlock_bh(&sc->chan_lock); ath9k_chanctx_stop_queues(sc, sc->cur_chan); queues_stopped = true;
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 <baijiaju1990@gmail.com> --- drivers/net/wireless/ath/ath9k/channel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)