net80211: fix a race between ieee80211_sta_join and scan entries
ClosedPublic
Actions

Authored by bz on Wed, Apr 16, 7:24 PM.

Details

Reviewers

adrian

Group Reviewers

wireless

Commits

rG8d6b75819946: net80211: fix a race between ieee80211_sta_join and scan entries
rG34cd36918652: net80211: fix a race between ieee80211_sta_join and scan entries
rGaff56b4f0b25: net80211: fix a race between ieee80211_sta_join and scan entries

Summary

We were seeing panics during ieee80211_sta_join() which seemed that
the ni->ni_chan was not valid anymore, which was true.
We also saw errors indicating data put into ni_ies became inalid.

The problem was that the ieee80211_scan_entry passed into
ieee80211_sta_join() (in the observed case from setmlme_assoc_sta())
became invalid during ieee80211_alloc_node().
As a result for the ni_chan case the the rateset and len in rates[1]
became invalid. Similarly for the IEs.

Make a (deep)copy of the scan entry in setmlme_assoc_sta() and return
the copy as once we leave ieee80211_scan_iterate() we can no longer
rely on the scan entry to be valid.

Sponsored by: The FreeBSD Foundation
MFC after: 3 days
Reported by: rm
PR: 286063

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 63847
Build 60731: arc lint + arc unit

Event Timeline

bz created this revision.Wed, Apr 16, 7:24 PM

Herald added subscribers: glebius, melifaro, imp. · View Herald TranscriptWed, Apr 16, 7:24 PM

bz requested review of this revision.Wed, Apr 16, 7:24 PM

Harbormaster completed remote builds in B63550: Diff 153752.Wed, Apr 16, 7:24 PM

@adrian, does this also fix your problem from D49514 or is that a similar yet different problem?

Bjoern, as I said in the beginning I wasn't able to reproduce the panic, it only happened once. I tested with your patch, and can say everything works smoothly on my side.
Please let me know if you need any additional info, command outputs etc. Thank you!

rm added inline comments.Thu, Apr 17, 9:03 PM

sys/net80211/ieee80211_ioctl.c
1558	There is a typo in a word "everything"

In D49865#1137332, @rm wrote:

Bjoern, as I said in the beginning I wasn't able to reproduce the panic, it only happened once. I tested with your patch, and can say everything works smoothly on my side.
Please let me know if you need any additional info, command outputs etc. Thank you!

Thanks. I could reliably reproduce it (at some point) before this patch. Given it's a race it happens .. or not and a change to system may just make it happen. I had enabled lots of debugging to console at that point.

@adrian @thj does this make sense here?

@adrian I do not think this fixes your problem from D49514. Need to look into that. I need an AP supporting WEP and GCMP anyway so ... maybe I'll hit it too.

Adding Alexander as he also reported a panic with this.

Hope someone will review it to get the fix in for 14.3-BETA2 at least.

Looks reasonable

sys/net80211/ieee80211_ioctl.c
1558
1559	What about letting the compiler do it? `look->se = *se`

To get a baseline, I rebuilt to yesterday mornings current. I was unable to reproduce the panic I was getting from even by letting the phone hotspot run out of power during an active network transaction on that. I have been running current with this patch, and there seems to be no regressions.

Fix typo.
Let the compiler do the memcpy.

Harbormaster completed remote builds in B63847: Diff 154728.Fri, May 2, 10:15 PM

bz marked 3 inline comments as done.Fri, May 2, 10:15 PM

adrian accepted this revision.Mon, May 5, 2:17 AM

This revision is now accepted and ready to land.Mon, May 5, 2:17 AM

Closed by commit rGaff56b4f0b25: net80211: fix a race between ieee80211_sta_join and scan entries (authored by bz). · Explain WhyMon, May 5, 2:59 PM

This revision was automatically updated to reflect the committed changes.

bz added a commit: rGaff56b4f0b25: net80211: fix a race between ieee80211_sta_join and scan entries.

bz added a commit: rG34cd36918652: net80211: fix a race between ieee80211_sta_join and scan entries.Wed, May 7, 11:51 AM

bz added a commit: rG8d6b75819946: net80211: fix a race between ieee80211_sta_join and scan entries.Thu, May 8, 9:23 PM