On 2023-12-05 12:15 +0900, Greg Kroah-Hartman wrote:
> 6.6-stable review patch. If anyone has any objections, please let me know.
>
> From: Johannes Berg <johannes.berg@xxxxxxxxx>
>
> commit 7e7efdda6adb385fbdfd6f819d76bc68c923c394 upstream.
>
> My prior race fix here broke CQM when ranges aren't used, as
> the reporting worker now requires the cqm_config to be set in
> the wdev, but isn't set when there's no range configured.
>
> Rather than continuing to special-case the range version, set
> the cqm_config always and configure accordingly, also tracking
> if range was used or not to be able to clear the configuration
> appropriately with the same API, which was actually not right
> if both were implemented by a driver for some reason, as is
> the case with mac80211 (though there the implementations are
> equivalent so it doesn't matter.)
>
> Also, the original multiple-RSSI commit lost checking for the
> callback, so might have potentially crashed if a driver had
> neither implementation, and userspace tried to use it despite
> not being advertised as supported.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 4a4b8169501b ("cfg80211: Accept multiple RSSI thresholds for CQM")
> Fixes: 37c20b2effe9 ("wifi: cfg80211: fix cqm_config access race")
> Signed-off-by: Johannes Berg <johannes.berg@xxxxxxxxx>
> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> ---
> net/wireless/core.h | 1
> net/wireless/nl80211.c | 50 ++++++++++++++++++++++++++++++-------------------
> 2 files changed, 32 insertions(+), 19 deletions(-)
>
> --- a/net/wireless/core.h
> +++ b/net/wireless/core.h
> @@ -299,6 +299,7 @@ struct cfg80211_cqm_config {
> u32 rssi_hyst;
> s32 last_rssi_event_value;
> enum nl80211_cqm_rssi_threshold_event last_rssi_event_type;
> + bool use_range_api;
> int n_rssi_thresholds;
> s32 rssi_thresholds[] __counted_by(n_rssi_thresholds);
> };
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -12824,10 +12824,6 @@ static int cfg80211_cqm_rssi_update(stru
> int i, n, low_index;
> int err;
>
> - /* RSSI reporting disabled? */
> - if (!cqm_config)
> - return rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
> -
> /*
> * Obtain current RSSI value if possible, if not and no RSSI threshold
> * event has been received yet, we should receive an event after a
> @@ -12902,18 +12898,6 @@ static int nl80211_set_cqm_rssi(struct g
> wdev->iftype != NL80211_IFTYPE_P2P_CLIENT)
> return -EOPNOTSUPP;
>
> - if (n_thresholds <= 1 && rdev->ops->set_cqm_rssi_config) {
> - if (n_thresholds == 0 || thresholds[0] == 0) /* Disabling */
> - return rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
> -
> - return rdev_set_cqm_rssi_config(rdev, dev,
> - thresholds[0], hysteresis);
> - }
> -
> - if (!wiphy_ext_feature_isset(&rdev->wiphy,
> - NL80211_EXT_FEATURE_CQM_RSSI_LIST))
> - return -EOPNOTSUPP;
> -
> if (n_thresholds == 1 && thresholds[0] == 0) /* Disabling */
> n_thresholds = 0;
>
> @@ -12921,6 +12905,20 @@ static int nl80211_set_cqm_rssi(struct g
> old = rcu_dereference_protected(wdev->cqm_config,
> lockdep_is_held(&wdev->mtx));
>
> + /* if already disabled just succeed */
> + if (!n_thresholds && !old)
> + return 0;
> +
> + if (n_thresholds > 1) {
> + if (!wiphy_ext_feature_isset(&rdev->wiphy,
> + NL80211_EXT_FEATURE_CQM_RSSI_LIST) ||
> + !rdev->ops->set_cqm_rssi_range_config)
> + return -EOPNOTSUPP;
> + } else {
> + if (!rdev->ops->set_cqm_rssi_config)
> + return -EOPNOTSUPP;
> + }
> +
> if (n_thresholds) {
> cqm_config = kzalloc(struct_size(cqm_config, rssi_thresholds,
> n_thresholds),
> @@ -12935,13 +12933,26 @@ static int nl80211_set_cqm_rssi(struct g
> memcpy(cqm_config->rssi_thresholds, thresholds,
> flex_array_size(cqm_config, rssi_thresholds,
> n_thresholds));
> + cqm_config->use_range_api = n_thresholds > 1 ||
> + !rdev->ops->set_cqm_rssi_config;
>
> rcu_assign_pointer(wdev->cqm_config, cqm_config);
> +
> + if (cqm_config->use_range_api)
> + err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
> + else
> + err = rdev_set_cqm_rssi_config(rdev, dev,
> + thresholds[0],
> + hysteresis);
> } else {
> RCU_INIT_POINTER(wdev->cqm_config, NULL);
> + /* if enabled as range also disable via range */
> + if (old->use_range_api)
> + err = rdev_set_cqm_rssi_range_config(rdev, dev, 0, 0);
> + else
> + err = rdev_set_cqm_rssi_config(rdev, dev, 0, 0);
> }
>
> - err = cfg80211_cqm_rssi_update(rdev, dev, cqm_config);
> if (err) {
> rcu_assign_pointer(wdev->cqm_config, old);
> kfree_rcu(cqm_config, rcu_head);
> @@ -19131,10 +19142,11 @@ void cfg80211_cqm_rssi_notify_work(struc
> wdev_lock(wdev);
> cqm_config = rcu_dereference_protected(wdev->cqm_config,
> lockdep_is_held(&wdev->mtx));
> - if (!wdev->cqm_config)
> + if (!cqm_config)
> goto unlock;
>
> - cfg80211_cqm_rssi_update(rdev, wdev->netdev, cqm_config);
> + if (cqm_config->use_range_api)
> + cfg80211_cqm_rssi_update(rdev, wdev->netdev, cqm_config);
>
> rssi_level = cqm_config->last_rssi_event_value;
> rssi_event = cqm_config->last_rssi_event_type;
After upgrading to 6.6.5, I noticed that my laptop would hang on
shutdown and bisected that problem to this patch. Reverting it makes
the problem go away.
More specifically, NetworkManager and wpa_supplicant processes are hung.
This can also be triggered by "systemctl stop NetworkManager.service"
which does not complete and brings these two processes into a state of
uninterruptible sleep.
I also tried 6.1 and mainline, and could reproduce the hang in 6.1.66
but _not_ in 6.7-rc4, so maybe some patch from Linus' tree needs to be
applied to the stable branches.
Cheers,
Sven
