On 5/28/2025 11:17 PM, Jeff Johnson wrote:
> On 5/21/2025 10:33 PM, Roopni Devanathan wrote:
> ...
>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> index fd5f79266471..4e8507649d15 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -854,6 +854,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
>> [NL80211_ATTR_MLO_RECONF_REM_LINKS] = { .type = NLA_U16 },
>> [NL80211_ATTR_EPCS] = { .type = NLA_FLAG },
>> [NL80211_ATTR_ASSOC_MLD_EXT_CAPA_OPS] = { .type = NLA_U16 },
>> + [NL80211_ATTR_WIPHY_RADIO_INDEX] = { .type = NLA_U8 },
>> };
>>
>> /* policy for the key attributes */
>> @@ -3608,6 +3609,33 @@ static int nl80211_set_channel(struct sk_buff *skb, struct genl_info *info)
>> return __nl80211_set_channel(rdev, netdev, info, link_id);
>> }
>>
>> +static int nl80211_set_wiphy_radio(struct genl_info *info,
>> + struct cfg80211_registered_device *rdev,
>> + int radio_id)
>
> refer to prior comment about s/radio_id/radio_idx/g
>
Noted.
>> +{
>> + u32 rts_threshold = 0, old_rts, changed = 0;
>> + int result;
>> +
>> + if (!rdev->ops->set_wiphy_params)
>> + return -EOPNOTSUPP;
>> +
>> + if (info->attrs[NL80211_ATTR_WIPHY_RTS_THRESHOLD]) {
>> + rts_threshold = nla_get_u32(
>> + info->attrs[NL80211_ATTR_WIPHY_RTS_THRESHOLD]);
>> + changed |= WIPHY_PARAM_RTS_THRESHOLD;
>> + }
>> +
>> + old_rts = rdev->wiphy.radio_cfg[radio_id].rts_threshold;
>> +
>> + rdev->wiphy.radio_cfg[radio_id].rts_threshold = rts_threshold;
>> +
>> + result = rdev_set_wiphy_params(rdev, radio_id, changed);
>> + if (result)
>> + rdev->wiphy.radio_cfg[radio_id].rts_threshold = old_rts;
>> +
>> + return 0;
>> +}
>> +
>> static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> {
>> struct cfg80211_registered_device *rdev = NULL;
>> @@ -3617,6 +3645,7 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> struct nlattr *nl_txq_params;
>> u32 changed;
>> u8 retry_short = 0, retry_long = 0;
>> + int radio_id = -1;
>> u32 frag_threshold = 0, rts_threshold = 0;
>> u8 coverage_class = 0;
>> u32 txq_limit = 0, txq_memory_limit = 0, txq_quantum = 0;
>> @@ -3670,6 +3699,19 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> if (result)
>> return result;
>>
>> + if (info->attrs[NL80211_ATTR_WIPHY_RADIO_INDEX]) {
>> + /* Radio idx is not expected for non-multi radio wiphy */
>> + if (rdev->wiphy.n_radio <= 0)
>> + return -EINVAL;
>> +
>> + radio_id = nla_get_u8(
>> + info->attrs[NL80211_ATTR_WIPHY_RADIO_INDEX]);
>> + if (radio_id >= rdev->wiphy.n_radio)
>> + return -EINVAL;
>> + else
>> + return nl80211_set_wiphy_radio(info, rdev, radio_id);
>
> nit: no need for else after return, so the above two lines should be just:
>
> return nl80211_set_wiphy_radio(info, rdev, radio_id);
>
Noted. Will change this in next revision.
>> + }
>> +
>> if (info->attrs[NL80211_ATTR_WIPHY_TXQ_PARAMS]) {
>> struct ieee80211_txq_params txq_params;
>> struct nlattr *tb[NL80211_TXQ_ATTR_MAX + 1];
>> @@ -3879,16 +3921,26 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> if (changed) {
>> u8 old_retry_short, old_retry_long;
>> u32 old_frag_threshold, old_rts_threshold;
>> - u8 old_coverage_class;
>> + u8 old_coverage_class, i;
>> u32 old_txq_limit, old_txq_memory_limit, old_txq_quantum;
>> + u32 *old_radio_rts_threshold;
>>
>> if (!rdev->ops->set_wiphy_params)
>> return -EOPNOTSUPP;
>>
>> + old_radio_rts_threshold = kcalloc(rdev->wiphy.n_radio,
>> + sizeof(u32),
>> + GFP_KERNEL);
>
> can't n_radio be 0 for legacy drivers, and hence this will attempt to allocate
> a zero-sized block of memory?
>
> seems the allocation and references need to be guarded by:
> if (rdev->wiphy.n_radio)
>
I agree. Will change this in next version.
>> + if (!old_radio_rts_threshold)
>> + return -ENOMEM;
>> +
>> old_retry_short = rdev->wiphy.retry_short;
>> old_retry_long = rdev->wiphy.retry_long;
>> old_frag_threshold = rdev->wiphy.frag_threshold;
>> old_rts_threshold = rdev->wiphy.rts_threshold;
>> + for (i = 0 ; i < rdev->wiphy.n_radio; i++)
>> + old_radio_rts_threshold[i] =
>> + rdev->wiphy.radio_cfg[i].rts_threshold;
>> old_coverage_class = rdev->wiphy.coverage_class;
>> old_txq_limit = rdev->wiphy.txq_limit;
>> old_txq_memory_limit = rdev->wiphy.txq_memory_limit;
>> @@ -3900,8 +3952,12 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> rdev->wiphy.retry_long = retry_long;
>> if (changed & WIPHY_PARAM_FRAG_THRESHOLD)
>> rdev->wiphy.frag_threshold = frag_threshold;
>> - if (changed & WIPHY_PARAM_RTS_THRESHOLD)
>> + if (changed & WIPHY_PARAM_RTS_THRESHOLD) {
>> rdev->wiphy.rts_threshold = rts_threshold;
>> + for (i = 0 ; i < rdev->wiphy.n_radio; i++)
>> + rdev->wiphy.radio_cfg[i].rts_threshold =
>> + rdev->wiphy.rts_threshold;
>> + }
>> if (changed & WIPHY_PARAM_COVERAGE_CLASS)
>> rdev->wiphy.coverage_class = coverage_class;
>> if (changed & WIPHY_PARAM_TXQ_LIMIT)
>> @@ -3911,18 +3967,23 @@ static int nl80211_set_wiphy(struct sk_buff *skb, struct genl_info *info)
>> if (changed & WIPHY_PARAM_TXQ_QUANTUM)
>> rdev->wiphy.txq_quantum = txq_quantum;
>>
>> - result = rdev_set_wiphy_params(rdev, changed);
>> + result = rdev_set_wiphy_params(rdev, radio_id, changed);
>> if (result) {
>> rdev->wiphy.retry_short = old_retry_short;
>> rdev->wiphy.retry_long = old_retry_long;
>> rdev->wiphy.frag_threshold = old_frag_threshold;
>> rdev->wiphy.rts_threshold = old_rts_threshold;
>> + for (i = 0 ; i < rdev->wiphy.n_radio; i++)
>> + rdev->wiphy.radio_cfg[i].rts_threshold =
>> + old_radio_rts_threshold[i];
>> rdev->wiphy.coverage_class = old_coverage_class;
>> rdev->wiphy.txq_limit = old_txq_limit;
>> rdev->wiphy.txq_memory_limit = old_txq_memory_limit;
>> rdev->wiphy.txq_quantum = old_txq_quantum;
>> - return result;
>> }
>> +
>> + kfree(old_radio_rts_threshold);
>> + return result;
>> }
>>
>> return 0;
