Skip to content

Commit 1107902

Browse files
arkqandy31415
authored andcommitted
Check WPA supplicant state before saving config (#22895)
* Check WPA supplicant state before saving config * Add save config log message * Fix segfault when WiFi is not enabled but ble-wifi pairing is used * Fix typo in variable name associattion -> association * C++ initialization for GDBusWpaSupplicant struct * Add missing mWpaSupplicantMutex locks --------- Co-authored-by: Andrei Litvin <[email protected]>
1 parent 681fa48 commit 1107902

File tree

4 files changed

+77
-49
lines changed

4 files changed

+77
-49
lines changed

src/platform/Linux/ConnectivityManagerImpl.cpp

+33-15
Original file line numberDiff line numberDiff line change
@@ -147,7 +147,8 @@ void ConnectivityManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
147147
}
148148

149149
#if CHIP_DEVICE_CONFIG_ENABLE_WPA
150-
bool ConnectivityManagerImpl::mAssociattionStarted = false;
150+
151+
bool ConnectivityManagerImpl::mAssociationStarted = false;
151152
BitFlags<Internal::GenericConnectivityManagerImpl_WiFi<ConnectivityManagerImpl>::ConnectivityFlags>
152153
ConnectivityManagerImpl::mConnectivityFlag;
153154
struct GDBusWpaSupplicant ConnectivityManagerImpl::mWpaSupplicant;
@@ -157,6 +158,7 @@ ConnectivityManager::WiFiStationMode ConnectivityManagerImpl::_GetWiFiStationMod
157158
{
158159
if (mWiFiStationMode != kWiFiStationMode_ApplicationControlled)
159160
{
161+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
160162
mWiFiStationMode = (mWpaSupplicant.iface != nullptr) ? kWiFiStationMode_Enabled : kWiFiStationMode_Disabled;
161163
}
162164

@@ -397,7 +399,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
397399
{
398400
if (g_strcmp0(value_str, "\'associating\'") == 0)
399401
{
400-
mAssociattionStarted = true;
402+
mAssociationStarted = true;
401403
}
402404
else if (g_strcmp0(value_str, "\'disconnected\'") == 0)
403405
{
@@ -409,7 +411,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
409411
delegate->OnConnectionStatusChanged(static_cast<uint8_t>(ConnectionStatusEnum::kConnected));
410412
}
411413

412-
if (mAssociattionStarted)
414+
if (mAssociationStarted)
413415
{
414416
uint8_t associationFailureCause = static_cast<uint8_t>(AssociationFailureCauseEnum::kUnknown);
415417
uint16_t status = WLAN_STATUS_UNSPECIFIED_FAILURE;
@@ -447,7 +449,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
447449

448450
DeviceLayer::SystemLayer().ScheduleLambda([]() { ConnectivityMgrImpl().UpdateNetworkStatus(); });
449451

450-
mAssociattionStarted = false;
452+
mAssociationStarted = false;
451453
}
452454
else if (g_strcmp0(value_str, "\'associated\'") == 0)
453455
{
@@ -460,7 +462,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
460462
}
461463
else if (g_strcmp0(value_str, "\'completed\'") == 0)
462464
{
463-
if (mAssociattionStarted)
465+
if (mAssociationStarted)
464466
{
465467
DeviceLayer::SystemLayer().ScheduleLambda([]() {
466468
if (mpConnectCallback != nullptr)
@@ -471,7 +473,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
471473
ConnectivityMgrImpl().PostNetworkConnect();
472474
});
473475
}
474-
mAssociattionStarted = false;
476+
mAssociationStarted = false;
475477
}
476478
}
477479

@@ -723,14 +725,10 @@ void ConnectivityManagerImpl::_OnWpaProxyReady(GObject * source_object, GAsyncRe
723725

724726
void ConnectivityManagerImpl::StartWiFiManagement()
725727
{
728+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
729+
726730
mConnectivityFlag.ClearAll();
727-
mWpaSupplicant.state = GDBusWpaSupplicant::INIT;
728-
mWpaSupplicant.scanState = GDBusWpaSupplicant::WIFI_SCANNING_IDLE;
729-
mWpaSupplicant.proxy = nullptr;
730-
mWpaSupplicant.iface = nullptr;
731-
mWpaSupplicant.bss = nullptr;
732-
mWpaSupplicant.interfacePath = nullptr;
733-
mWpaSupplicant.networkPath = nullptr;
731+
mWpaSupplicant = GDBusWpaSupplicant{};
734732

735733
ChipLogProgress(DeviceLayer, "wpa_supplicant: Start WiFi management");
736734

@@ -752,6 +750,8 @@ void ConnectivityManagerImpl::DriveAPState()
752750
CHIP_ERROR err = CHIP_NO_ERROR;
753751
WiFiAPState targetState;
754752

753+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
754+
755755
// If the AP interface is not under application control...
756756
if (mWiFiAPMode != kWiFiAPMode_ApplicationControlled)
757757
{
@@ -867,6 +867,8 @@ CHIP_ERROR ConnectivityManagerImpl::ConfigureWiFiAP()
867867
uint16_t discriminator = 0;
868868
char ssid[32];
869869

870+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
871+
870872
channel = ConnectivityUtils::MapChannelToFrequency(kWiFi_BAND_2_4_GHZ, CHIP_DEVICE_CONFIG_WIFI_AP_CHANNEL);
871873

872874
if (GetCommissionableDataProvider()->GetSetupDiscriminator(discriminator) != CHIP_NO_ERROR)
@@ -961,8 +963,11 @@ ConnectivityManagerImpl::ConnectWiFiNetworkAsync(ByteSpan ssid, ByteSpan credent
961963
char ssidStr[kMaxWiFiSSIDLength + 1u] = { 0 };
962964
char keyStr[kMaxWiFiKeyLength + 1u] = { 0 };
963965

966+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
967+
964968
VerifyOrReturnError(ssid.size() <= kMaxWiFiSSIDLength, CHIP_ERROR_INVALID_ARGUMENT);
965969
VerifyOrReturnError(credentials.size() <= kMaxWiFiKeyLength, CHIP_ERROR_INVALID_ARGUMENT);
970+
VerifyOrReturnError(mWpaSupplicant.iface != nullptr, CHIP_ERROR_INCORRECT_STATE);
966971

967972
// There is another ongoing connect request, reject the new one.
968973
VerifyOrReturnError(mpConnectCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
@@ -1046,6 +1051,9 @@ void ConnectivityManagerImpl::_ConnectWiFiNetworkAsyncCallback(GObject * source_
10461051
ConnectivityManagerImpl * this_ = reinterpret_cast<ConnectivityManagerImpl *>(user_data);
10471052
std::unique_ptr<GVariant, GVariantDeleter> attachRes;
10481053
std::unique_ptr<GError, GErrorDeleter> err;
1054+
1055+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
1056+
10491057
{
10501058
gboolean result = wpa_fi_w1_wpa_supplicant1_interface_call_select_network_finish(mWpaSupplicant.iface, res,
10511059
&MakeUniquePointerReceiver(err).Get());
@@ -1132,7 +1140,15 @@ CHIP_ERROR ConnectivityManagerImpl::CommitConfig()
11321140
gboolean result;
11331141
std::unique_ptr<GError, GErrorDeleter> err;
11341142

1135-
ChipLogProgress(DeviceLayer, "wpa_supplicant: connected to network");
1143+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
1144+
1145+
if (mWpaSupplicant.state != GDBusWpaSupplicant::WPA_INTERFACE_CONNECTED)
1146+
{
1147+
ChipLogError(DeviceLayer, "wpa_supplicant: CommitConfig: interface proxy not connected");
1148+
return CHIP_ERROR_INCORRECT_STATE;
1149+
}
1150+
1151+
ChipLogProgress(DeviceLayer, "wpa_supplicant: save config");
11361152

11371153
result = wpa_fi_w1_wpa_supplicant1_interface_call_save_config_sync(mWpaSupplicant.iface, nullptr,
11381154
&MakeUniquePointerReceiver(err).Get());
@@ -1196,7 +1212,7 @@ CHIP_ERROR ConnectivityManagerImpl::GetWiFiSecurityType(SecurityTypeEnum & secur
11961212

11971213
if (mWpaSupplicant.state != GDBusWpaSupplicant::WPA_INTERFACE_CONNECTED)
11981214
{
1199-
ChipLogError(DeviceLayer, "wpa_supplicant: _GetWiFiSecurityType: interface proxy not connected");
1215+
ChipLogError(DeviceLayer, "wpa_supplicant: GetWiFiSecurityType: interface proxy not connected");
12001216
return CHIP_ERROR_INCORRECT_STATE;
12011217
}
12021218

@@ -1605,6 +1621,8 @@ bool ConnectivityManagerImpl::_GetBssInfo(const gchar * bssPath, NetworkCommissi
16051621

16061622
void ConnectivityManagerImpl::_OnWpaInterfaceScanDone(GObject * source_object, GAsyncResult * res, gpointer user_data)
16071623
{
1624+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
1625+
16081626
ChipLogProgress(DeviceLayer, "wpa_supplicant: network scan done");
16091627
gchar ** bsss = wpa_fi_w1_wpa_supplicant1_interface_dup_bsss(mWpaSupplicant.iface);
16101628
gchar ** oldBsss = bsss;

src/platform/Linux/ConnectivityManagerImpl.h

+15-11
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ namespace DeviceLayer {
6565
#if CHIP_DEVICE_CONFIG_ENABLE_WPA
6666
struct GDBusWpaSupplicant
6767
{
68-
enum
68+
enum WpaState
6969
{
7070
INIT,
7171
WPA_CONNECTING,
@@ -74,19 +74,21 @@ struct GDBusWpaSupplicant
7474
WPA_NO_INTERFACE_PATH,
7575
WPA_GOT_INTERFACE_PATH,
7676
WPA_INTERFACE_CONNECTED,
77-
} state;
77+
};
7878

79-
enum
79+
enum WpaScanning
8080
{
8181
WIFI_SCANNING_IDLE,
8282
WIFI_SCANNING,
83-
} scanState;
83+
};
8484

85-
WpaFiW1Wpa_supplicant1 * proxy;
86-
WpaFiW1Wpa_supplicant1Interface * iface;
87-
WpaFiW1Wpa_supplicant1BSS * bss;
88-
gchar * interfacePath;
89-
gchar * networkPath;
85+
WpaState state = INIT;
86+
WpaScanning scanState = WIFI_SCANNING_IDLE;
87+
WpaFiW1Wpa_supplicant1 * proxy = nullptr;
88+
WpaFiW1Wpa_supplicant1Interface * iface = nullptr;
89+
WpaFiW1Wpa_supplicant1BSS * bss = nullptr;
90+
gchar * interfacePath = nullptr;
91+
gchar * networkPath = nullptr;
9092
};
9193
#endif
9294

@@ -203,9 +205,11 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
203205

204206
static bool _GetBssInfo(const gchar * bssPath, NetworkCommissioning::WiFiScanResponse & result);
205207

206-
static bool mAssociattionStarted;
208+
static bool mAssociationStarted;
207209
static BitFlags<ConnectivityFlags> mConnectivityFlag;
208-
static struct GDBusWpaSupplicant mWpaSupplicant;
210+
static GDBusWpaSupplicant mWpaSupplicant;
211+
// Access to mWpaSupplicant has to be protected by a mutex because it is accessed from
212+
// the CHIP event loop thread and dedicated D-Bus thread started by platform manager.
209213
static std::mutex mWpaSupplicantMutex;
210214

211215
NetworkCommissioning::Internal::BaseDriver::NetworkStatusChangeCallback * mpStatusChangeCallback = nullptr;

src/platform/webos/ConnectivityManagerImpl.cpp

+17-13
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ void ConnectivityManagerImpl::_OnPlatformEvent(const ChipDeviceEvent * event)
143143
}
144144

145145
#if CHIP_DEVICE_CONFIG_ENABLE_WPA
146-
bool ConnectivityManagerImpl::mAssociattionStarted = false;
146+
147+
bool ConnectivityManagerImpl::mAssociationStarted = false;
147148
BitFlags<Internal::GenericConnectivityManagerImpl_WiFi<ConnectivityManagerImpl>::ConnectivityFlags>
148149
ConnectivityManagerImpl::mConnectivityFlag;
149150
struct GDBusWpaSupplicant ConnectivityManagerImpl::mWpaSupplicant;
@@ -393,7 +394,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
393394
{
394395
if (g_strcmp0(value_str, "\'associating\'") == 0)
395396
{
396-
mAssociattionStarted = true;
397+
mAssociationStarted = true;
397398
}
398399
else if (g_strcmp0(value_str, "\'disconnected\'") == 0)
399400
{
@@ -405,7 +406,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
405406
delegate->OnConnectionStatusChanged(static_cast<uint8_t>(ConnectionStatusEnum::kConnected));
406407
}
407408

408-
if (mAssociattionStarted)
409+
if (mAssociationStarted)
409410
{
410411
uint8_t associationFailureCause = static_cast<uint8_t>(AssociationFailureCauseEnum::kUnknown);
411412
uint16_t status = WLAN_STATUS_UNSPECIFIED_FAILURE;
@@ -435,7 +436,7 @@ void ConnectivityManagerImpl::_OnWpaPropertiesChanged(WpaFiW1Wpa_supplicant1Inte
435436

436437
DeviceLayer::SystemLayer().ScheduleLambda([]() { ConnectivityMgrImpl().UpdateNetworkStatus(); });
437438

438-
mAssociattionStarted = false;
439+
mAssociationStarted = false;
439440
}
440441
else if (g_strcmp0(value_str, "\'associated\'") == 0)
441442
{
@@ -697,13 +698,7 @@ void ConnectivityManagerImpl::_OnWpaProxyReady(GObject * source_object, GAsyncRe
697698
void ConnectivityManagerImpl::StartWiFiManagement()
698699
{
699700
mConnectivityFlag.ClearAll();
700-
mWpaSupplicant.state = GDBusWpaSupplicant::INIT;
701-
mWpaSupplicant.scanState = GDBusWpaSupplicant::WIFI_SCANNING_IDLE;
702-
mWpaSupplicant.proxy = nullptr;
703-
mWpaSupplicant.iface = nullptr;
704-
mWpaSupplicant.bss = nullptr;
705-
mWpaSupplicant.interfacePath = nullptr;
706-
mWpaSupplicant.networkPath = nullptr;
701+
mWpaSupplicant = GDBusWpaSupplicant{};
707702

708703
ChipLogProgress(DeviceLayer, "wpa_supplicant: Start WiFi management");
709704

@@ -936,6 +931,7 @@ ConnectivityManagerImpl::ConnectWiFiNetworkAsync(ByteSpan ssid, ByteSpan credent
936931

937932
VerifyOrReturnError(ssid.size() <= kMaxWiFiSSIDLength, CHIP_ERROR_INVALID_ARGUMENT);
938933
VerifyOrReturnError(credentials.size() <= kMaxWiFiKeyLength, CHIP_ERROR_INVALID_ARGUMENT);
934+
VerifyOrReturnError(mWpaSupplicant.iface != nullptr, CHIP_ERROR_INCORRECT_STATE);
939935

940936
// There is another ongoing connect request, reject the new one.
941937
VerifyOrReturnError(mpConnectCallback == nullptr, CHIP_ERROR_INCORRECT_STATE);
@@ -1094,7 +1090,15 @@ CHIP_ERROR ConnectivityManagerImpl::CommitConfig()
10941090
gboolean result;
10951091
std::unique_ptr<GError, GErrorDeleter> err;
10961092

1097-
ChipLogProgress(DeviceLayer, "wpa_supplicant: connected to network");
1093+
std::lock_guard<std::mutex> lock(mWpaSupplicantMutex);
1094+
1095+
if (mWpaSupplicant.state != GDBusWpaSupplicant::WPA_INTERFACE_CONNECTED)
1096+
{
1097+
ChipLogError(DeviceLayer, "wpa_supplicant: CommitConfig: interface proxy not connected");
1098+
return CHIP_ERROR_INCORRECT_STATE;
1099+
}
1100+
1101+
ChipLogProgress(DeviceLayer, "wpa_supplicant: save config");
10981102

10991103
result = wpa_fi_w1_wpa_supplicant1_interface_call_save_config_sync(mWpaSupplicant.iface, nullptr,
11001104
&MakeUniquePointerReceiver(err).Get());
@@ -1158,7 +1162,7 @@ CHIP_ERROR ConnectivityManagerImpl::GetWiFiSecurityType(SecurityTypeEnum & secur
11581162

11591163
if (mWpaSupplicant.state != GDBusWpaSupplicant::WPA_INTERFACE_CONNECTED)
11601164
{
1161-
ChipLogError(DeviceLayer, "wpa_supplicant: _GetWiFiSecurityType: interface proxy not connected");
1165+
ChipLogError(DeviceLayer, "wpa_supplicant: GetWiFiSecurityType: interface proxy not connected");
11621166
return CHIP_ERROR_INCORRECT_STATE;
11631167
}
11641168

src/platform/webos/ConnectivityManagerImpl.h

+12-10
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ namespace DeviceLayer {
6565
#if CHIP_DEVICE_CONFIG_ENABLE_WPA
6666
struct GDBusWpaSupplicant
6767
{
68-
enum
68+
enum WpaState
6969
{
7070
INIT,
7171
WPA_CONNECTING,
@@ -74,19 +74,21 @@ struct GDBusWpaSupplicant
7474
WPA_NO_INTERFACE_PATH,
7575
WPA_GOT_INTERFACE_PATH,
7676
WPA_INTERFACE_CONNECTED,
77-
} state;
77+
};
7878

79-
enum
79+
enum WpaScanning
8080
{
8181
WIFI_SCANNING_IDLE,
8282
WIFI_SCANNING,
83-
} scanState;
83+
};
8484

85-
WpaFiW1Wpa_supplicant1 * proxy;
86-
WpaFiW1Wpa_supplicant1Interface * iface;
87-
WpaFiW1Wpa_supplicant1BSS * bss;
88-
gchar * interfacePath;
89-
gchar * networkPath;
85+
WpaState state = INIT;
86+
WpaScanning scanState = WIFI_SCANNING_IDLE;
87+
WpaFiW1Wpa_supplicant1 * proxy = nullptr;
88+
WpaFiW1Wpa_supplicant1Interface * iface = nullptr;
89+
WpaFiW1Wpa_supplicant1BSS * bss = nullptr;
90+
gchar * interfacePath = nullptr;
91+
gchar * networkPath = nullptr;
9092
};
9193
#endif
9294

@@ -203,7 +205,7 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
203205

204206
static bool _GetBssInfo(const gchar * bssPath, NetworkCommissioning::WiFiScanResponse & result);
205207

206-
static bool mAssociattionStarted;
208+
static bool mAssociationStarted;
207209
static BitFlags<ConnectivityFlags> mConnectivityFlag;
208210
static struct GDBusWpaSupplicant mWpaSupplicant;
209211
static std::mutex mWpaSupplicantMutex;

0 commit comments

Comments
 (0)