| Message ID | 20240520073311.44117-1-linma@zju.edu.cn (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1,net] cfg80211: pmsr: use correct nla_get_uX functions | expand |
Couple of minor comments: The subject is missing wifi: prefix. On Mon, 2024-05-20 at 15:33 +0800, Lin Ma wrote: > The commit 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM > initiator API") defines four attributes, which are > > - `NL80211_PMSR_FTM_REQ_ATTR_NUM_BURSTS_EXP` with NLA_U8 type > - `NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD` with NLA_U16 type > - `NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION` with NLA_U8 type > - `NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES` with NLA_U8 type > > However, the consumers of these attributes in `pmsr_parse_ftm` blindly > all use `nla_get_u32`, which is incorrect. Hence, fix these with correct > `nla_get_u8` and `nla_get_u16`. Maybe mention a bit more clearly that the _policy_ actually defines them with those types, so the users are incorrect, vs. a possible scenario where the kernel's documentation is just incorrect and we have to keep the code as-is for userspace to continue to function. I believe the changes are correct since the policy wants the right types and then on little-endian platforms it'll all work ... johannes
Hello there, > > Maybe mention a bit more clearly that the _policy_ actually defines them > with those types, so the users are incorrect, vs. a possible scenario > where the kernel's documentation is just incorrect and we have to keep > the code as-is for userspace to continue to function. > > I believe the changes are correct since the policy wants the right types > and then on little-endian platforms it'll all work ... > > johannes Thanks for the kind reply and suggestions, I've sent the V2 version and please check that. Regards Lin
diff --git a/net/wireless/pmsr.c b/net/wireless/pmsr.c index e106dcea3977..c569c37da317 100644 --- a/net/wireless/pmsr.c +++ b/net/wireless/pmsr.c @@ -56,7 +56,7 @@ static int pmsr_parse_ftm(struct cfg80211_registered_device *rdev, out->ftm.burst_period = 0; if (tb[NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD]) out->ftm.burst_period = - nla_get_u32(tb[NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD]); + nla_get_u16(tb[NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD]); out->ftm.asap = !!tb[NL80211_PMSR_FTM_REQ_ATTR_ASAP]; if (out->ftm.asap && !capa->ftm.asap) { @@ -75,7 +75,7 @@ static int pmsr_parse_ftm(struct cfg80211_registered_device *rdev, out->ftm.num_bursts_exp = 0; if (tb[NL80211_PMSR_FTM_REQ_ATTR_NUM_BURSTS_EXP]) out->ftm.num_bursts_exp = - nla_get_u32(tb[NL80211_PMSR_FTM_REQ_ATTR_NUM_BURSTS_EXP]); + nla_get_u8(tb[NL80211_PMSR_FTM_REQ_ATTR_NUM_BURSTS_EXP]); if (capa->ftm.max_bursts_exponent >= 0 && out->ftm.num_bursts_exp > capa->ftm.max_bursts_exponent) { @@ -88,7 +88,7 @@ static int pmsr_parse_ftm(struct cfg80211_registered_device *rdev, out->ftm.burst_duration = 15; if (tb[NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION]) out->ftm.burst_duration = - nla_get_u32(tb[NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION]); + nla_get_u8(tb[NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION]); out->ftm.ftms_per_burst = 0; if (tb[NL80211_PMSR_FTM_REQ_ATTR_FTMS_PER_BURST]) @@ -107,7 +107,7 @@ static int pmsr_parse_ftm(struct cfg80211_registered_device *rdev, out->ftm.ftmr_retries = 3; if (tb[NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES]) out->ftm.ftmr_retries = - nla_get_u32(tb[NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES]); + nla_get_u8(tb[NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES]); out->ftm.request_lci = !!tb[NL80211_PMSR_FTM_REQ_ATTR_REQUEST_LCI]; if (out->ftm.request_lci && !capa->ftm.request_lci) {
The commit 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API") defines four attributes, which are - `NL80211_PMSR_FTM_REQ_ATTR_NUM_BURSTS_EXP` with NLA_U8 type - `NL80211_PMSR_FTM_REQ_ATTR_BURST_PERIOD` with NLA_U16 type - `NL80211_PMSR_FTM_REQ_ATTR_BURST_DURATION` with NLA_U8 type - `NL80211_PMSR_FTM_REQ_ATTR_NUM_FTMR_RETRIES` with NLA_U8 type However, the consumers of these attributes in `pmsr_parse_ftm` blindly all use `nla_get_u32`, which is incorrect. Hence, fix these with correct `nla_get_u8` and `nla_get_u16`. Fixes: 9bb7e0f24e7e ("cfg80211: add peer measurement with FTM initiator API") Signed-off-by: Lin Ma <linma@zju.edu.cn> --- net/wireless/pmsr.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)