From: Andrei Otcheretianski In cfg80211_nan_set_local_schedule(), after calling the driver, the old channel definitions were freed and only then the new ones were allocated and stored. This had two problems: - On allocation failure the previous channel definitions had already been freed while the driver had already applied the new schedule, leaving the wdev with no channel definitions. - If the driver completes a deferred schedule update synchronously, the completion notification to user space fired before the new channel definitions were stored, exposing an inconsistent state. Allocate and fill the new channel definitions before calling the driver, swap them in, and roll back on failure. Also set the pending flag before the driver call so that a synchronous deferred completion can clear it. Signed-off-by: Andrei Otcheretianski Signed-off-by: Miri Korenblit --- net/wireless/core.c | 53 +++++++++++++++++++++++++++++---------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/net/wireless/core.c b/net/wireless/core.c index 2f46243773f9..3e1d2c00b502 100644 --- a/net/wireless/core.c +++ b/net/wireless/core.c @@ -288,6 +288,8 @@ int cfg80211_nan_set_local_schedule(struct cfg80211_registered_device *rdev, struct wireless_dev *wdev, struct cfg80211_nan_local_sched *sched) { + struct cfg80211_chan_def *chandefs = NULL, *old_chandefs; + u8 old_n_channels; int ret; lockdep_assert_held(&rdev->wiphy.mtx); @@ -298,29 +300,42 @@ int cfg80211_nan_set_local_schedule(struct cfg80211_registered_device *rdev, if (wdev->u.nan.sched_update_pending) return -EBUSY; - ret = rdev_nan_set_local_sched(rdev, wdev, sched); - if (ret) - return ret; - - wdev->u.nan.sched_update_pending = sched->deferred; - - kfree(wdev->u.nan.chandefs); - wdev->u.nan.chandefs = NULL; - wdev->u.nan.n_channels = 0; + /* + * Pre-allocate chandefs, so we don't ruin the current + * schedule on allocation failure. + */ + if (sched->n_channels) { + chandefs = kcalloc(sched->n_channels, sizeof(*chandefs), + GFP_KERNEL); + if (!chandefs) + return -ENOMEM; - if (!sched->n_channels) - return 0; + for (int i = 0; i < sched->n_channels; i++) + chandefs[i] = sched->nan_channels[i].chandef; + } - wdev->u.nan.chandefs = kcalloc(sched->n_channels, - sizeof(*wdev->u.nan.chandefs), - GFP_KERNEL); - if (!wdev->u.nan.chandefs) - return -ENOMEM; + /* + * Swap in the new chandefs before calling the driver and set the + * deferred flag, so if the driver completes the update synchronously + * the new schedule will be already in place. + */ + old_chandefs = wdev->u.nan.chandefs; + old_n_channels = wdev->u.nan.n_channels; + wdev->u.nan.chandefs = chandefs; + wdev->u.nan.n_channels = sched->n_channels; + wdev->u.nan.sched_update_pending = sched->deferred; - for (int i = 0; i < sched->n_channels; i++) - wdev->u.nan.chandefs[i] = sched->nan_channels[i].chandef; + ret = rdev_nan_set_local_sched(rdev, wdev, sched); + if (ret) { + /* Restore the previous schedule on failure */ + wdev->u.nan.sched_update_pending = false; + wdev->u.nan.chandefs = old_chandefs; + wdev->u.nan.n_channels = old_n_channels; + kfree(chandefs); + return ret; + } - wdev->u.nan.n_channels = sched->n_channels; + kfree(old_chandefs); return 0; } -- 2.34.1