[PATCH v1 net] cfg80211: pmsr: use correct nla_get_uX functions

[PATCH v1 net] cfg80211: pmsr: use correct nla_get_uX functions

[Lin Ma]

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(-)

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) {
-- 
2.34.1

Re: [PATCH v1 net] cfg80211: pmsr: use correct nla_get_uX functions

[Johannes Berg]

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

Re: [PATCH v1 net] cfg80211: pmsr: use correct nla_get_uX functions

[Lin Ma]

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