[PATCH] station: check return of handshake_state_set_pmksa

[PATCH] station: check return of handshake_state_set_pmksa

[James Prestwood]

If this fails num_pmkids and pmkids would get set, but to an
uninitialized buffer. This would then fail to build the handshake
object later when copying the PMKID.
---
 src/station.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/station.c b/src/station.c
index 50997f5f..6c9e8d13 100644
--- a/src/station.c
+++ b/src/station.c
@@ -1378,9 +1378,13 @@ build_ie:
 					bss->ssid, bss->ssid_len,
 					info.akm_suites);
 		if (pmksa) {
-			handshake_state_set_pmksa(hs, pmksa);
-			info.num_pmkids = 1;
-			info.pmkids = hs->pmksa->pmkid;
+			if (!handshake_state_set_pmksa(hs, pmksa)) {
+				l_warn("failed to set PMKSA to handshake");
+				pmksa_cache_free(pmksa);
+			} else {
+				info.num_pmkids = 1;
+				info.pmkids = hs->pmksa->pmkid;
+			}
 		}
 	}
 
-- 
2.34.1

Re: [PATCH] station: check return of handshake_state_set_pmksa

[Denis Kenzior]

Hi James,

On 12/3/25 9:03 AM, James Prestwood wrote:
> If this fails num_pmkids and pmkids would get set, but to an
> uninitialized buffer. This would then fail to build the handshake
> object later when copying the PMKID.
> ---
>   src/station.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/src/station.c b/src/station.c
> index 50997f5f..6c9e8d13 100644
> --- a/src/station.c
> +++ b/src/station.c
> @@ -1378,9 +1378,13 @@ build_ie:
>   					bss->ssid, bss->ssid_len,
>   					info.akm_suites);
>   		if (pmksa) {
> -			handshake_state_set_pmksa(hs, pmksa);
> -			info.num_pmkids = 1;
> -			info.pmkids = hs->pmksa->pmkid;
> +			if (!handshake_state_set_pmksa(hs, pmksa)) {

This seems fishy.  Are you sure you're not masking the real issue here?  Would 
this possibly be related to fast transition re-using an old handshake?  Maybe we 
should not try to use or set the PMKSA for FT ?

> +				l_warn("failed to set PMKSA to handshake");
> +				pmksa_cache_free(pmksa);
> +			} else {
> +				info.num_pmkids = 1;
> +				info.pmkids = hs->pmksa->pmkid;
> +			}
>   		}
>   	}
>   

Regards,
-Denis

Re: [PATCH] station: check return of handshake_state_set_pmksa

[James Prestwood]

On 12/3/25 2:19 PM, Denis Kenzior wrote:
> Hi James,
>
> On 12/3/25 9:03 AM, James Prestwood wrote:
>> If this fails num_pmkids and pmkids would get set, but to an
>> uninitialized buffer. This would then fail to build the handshake
>> object later when copying the PMKID.
>> ---
>>   src/station.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/station.c b/src/station.c
>> index 50997f5f..6c9e8d13 100644
>> --- a/src/station.c
>> +++ b/src/station.c
>> @@ -1378,9 +1378,13 @@ build_ie:
>>                       bss->ssid, bss->ssid_len,
>>                       info.akm_suites);
>>           if (pmksa) {
>> -            handshake_state_set_pmksa(hs, pmksa);
>> -            info.num_pmkids = 1;
>> -            info.pmkids = hs->pmksa->pmkid;
>> +            if (!handshake_state_set_pmksa(hs, pmksa)) {
>
> This seems fishy.  Are you sure you're not masking the real issue 
> here?  Would this possibly be related to fast transition re-using an 
> old handshake?  Maybe we should not try to use or set the PMKSA for FT ?

Possibly. What happens is we complete an initial association (call it 
AP1) and establish a PMKSA. Possibly unrelated but IWD did fail to FT 
due to an association rejection status=40, IWD connects again (different 
AP), and we then try to FT back to the original AP1. This is where 
handshake_state_set_pmksa() fails.

Not using PMKSA for FT is reasonable for sure, since there's not really 
a point there...

>
>> +                l_warn("failed to set PMKSA to handshake");
>> +                pmksa_cache_free(pmksa);
>> +            } else {
>> +                info.num_pmkids = 1;
>> +                info.pmkids = hs->pmksa->pmkid;
>> +            }
>>           }
>>       }
>
> Regards,
> -Denis