Cancel the subscription in destructor to avoid the use-after-free issue

Signed-off-by: Lo,Chin-Ran <chin-ran.lo@nxp.com>

Author: crlonxp

src/controller/CHIPDeviceController.cpp

@@ -472,7 +472,7 @@ DeviceCommissioner::DeviceCommissioner() :
 DeviceCommissioner::~DeviceCommissioner()
 {
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
-    SetChkObjValid((void *) this, ObjChkAction::Clear, nullptr);
+    DeviceLayer::ConnectivityMgr().WiFiPAFCancelConnect();
 #endif
 }
 
@@ -833,7 +833,6 @@ CHIP_ERROR DeviceCommissioner::EstablishPASEConnection(NodeId remoteDeviceId, Re
                 ExitNow(CHIP_ERROR_INTERNAL);
             }
             mRendezvousParametersForDeviceDiscoveredOverWiFiPAF = params;
-            SetChkObjValid((void *) this, ObjChkAction::Set, nullptr);
             DeviceLayer::ConnectivityMgr().WiFiPAFConnect(params.GetSetupDiscriminator().value(), (void *) this,
                                                           OnWiFiPAFSubscribeComplete, OnWiFiPAFSubscribeError);
             ExitNow(CHIP_NO_ERROR);
@@ -908,70 +907,8 @@ void DeviceCommissioner::OnDiscoveredDeviceOverBleError(void * appState, CHIP_ER
 #endif // CONFIG_NETWORK_LAYER_BLE
 
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
-void DeviceCommissioner::SetChkObjValid(void * appObj, ObjChkAction action, bool * pIsObjValid)
-{
-    static std::vector<void *> ObjVector;
-    bool IsObjValid = false;
-
-    switch (action)
-    {
-    case ObjChkAction::Set: {
-        for (auto lt = ObjVector.begin(); lt != ObjVector.end(); lt++)
-        {
-            if (*lt == appObj)
-            {
-                IsObjValid = true;
-                break;
-            }
-        }
-        if (IsObjValid == false)
-        {
-            ObjVector.push_back(appObj);
-            IsObjValid = true;
-        }
-    }
-    break;
-    case ObjChkAction::Check: {
-        for (auto lt = ObjVector.begin(); lt != ObjVector.end(); lt++)
-        {
-            if (*lt == appObj)
-            {
-                IsObjValid = true;
-                break;
-            }
-        }
-    }
-    break;
-    case ObjChkAction::Clear: {
-        for (auto lt = ObjVector.begin(); lt != ObjVector.end(); lt++)
-        {
-            if (*lt == appObj)
-            {
-                // Already existed in the list => Remove it
-                ObjVector.erase(lt);
-                break;
-            }
-        }
-    }
-    break;
-    }
-    if (pIsObjValid != nullptr)
-    {
-        *pIsObjValid = IsObjValid;
-    }
-    return;
-}
-
 void DeviceCommissioner::OnWiFiPAFSubscribeComplete(void * appState)
 {
-    bool isObjValid;
-    SetChkObjValid(appState, ObjChkAction::Check, &isObjValid);
-    if (isObjValid == false)
-    {
-        // The caller has been released.
-        ChipLogError(Controller, "DeviceCommissioner has been destroyed!");
-        return;
-    }
     auto self   = (DeviceCommissioner *) appState;
     auto device = self->mDeviceInPASEEstablishment;
 
@@ -989,14 +926,6 @@ void DeviceCommissioner::OnWiFiPAFSubscribeComplete(void * appState)
 
 void DeviceCommissioner::OnWiFiPAFSubscribeError(void * appState, CHIP_ERROR err)
 {
-    bool isObjValid;
-    SetChkObjValid(appState, ObjChkAction::Check, &isObjValid);
-    if (isObjValid == false)
-    {
-        // The caller has been released.
-        ChipLogError(Controller, "DeviceCommissioner has been destroyed!");
-        return;
-    }
     auto self   = (DeviceCommissioner *) appState;
     auto device = self->mDeviceInPASEEstablishment;
 

src/controller/CHIPDeviceController.h

@@ -821,15 +821,6 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
         return ExtendArmFailSafeInternal(proxy, step, armFailSafeTimeout, commandTimeout, onSuccess, onFailure,
                                          /* fireAndForget = */ true);
     }
-#if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
-    enum ObjChkAction
-    {
-        Set,
-        Check,
-        Clear
-    };
-    static void SetChkObjValid(void * appState, ObjChkAction action, bool * pIsObjValid);
-#endif
 
 private:
     DevicePairingDelegate * mPairingDelegate = nullptr;

src/controller/SetUpCodePairer.cpp

@@ -63,7 +63,7 @@ CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload)
 SetUpCodePairer::~SetUpCodePairer()
 {
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
-    DeviceCommissioner::SetChkObjValid((void *) this, DeviceCommissioner::ObjChkAction::Clear, nullptr);
+    DeviceLayer::ConnectivityMgr().WiFiPAFCancelConnect();
 #endif
 }
 
@@ -266,8 +266,6 @@ CHIP_ERROR SetUpCodePairer::StartDiscoverOverWiFiPAF(SetupPayload & payload)
     ChipLogProgress(Controller, "Starting commissioning discovery over WiFiPAF");
     VerifyOrReturnError(mCommissioner != nullptr, CHIP_ERROR_INCORRECT_STATE);
     mWaitingForDiscovery[kWiFiPAFTransport] = true;
-
-    DeviceCommissioner::SetChkObjValid((void *) this, DeviceCommissioner::ObjChkAction::Set, nullptr);
     CHIP_ERROR err = DeviceLayer::ConnectivityMgr().WiFiPAFConnect(payload.discriminator, (void *) this, OnWiFiPAFSubscribeComplete,
                                                                    OnWiFiPAFSubscribeError);
     if (err != CHIP_NO_ERROR)
@@ -399,28 +397,12 @@ void SetUpCodePairer::OnWifiPAFDiscoveryError(CHIP_ERROR err)
 
 void SetUpCodePairer::OnWiFiPAFSubscribeComplete(void * appState)
 {
-    bool isObjValid;
-    DeviceCommissioner::SetChkObjValid(appState, DeviceCommissioner::ObjChkAction::Check, &isObjValid);
-    if (isObjValid == false)
-    {
-        // The caller has been released.
-        ChipLogError(Controller, "SetUpCodePairer has been destroyed!");
-        return;
-    }
     auto self = (SetUpCodePairer *) appState;
     self->OnDiscoveredDeviceOverWifiPAF();
 }
 
 void SetUpCodePairer::OnWiFiPAFSubscribeError(void * appState, CHIP_ERROR err)
 {
-    bool isObjValid;
-    DeviceCommissioner::SetChkObjValid(appState, DeviceCommissioner::ObjChkAction::Check, &isObjValid);
-    if (isObjValid == false)
-    {
-        // The caller has been released.
-        ChipLogError(Controller, "SetUpCodePairer has been destroyed!");
-        return;
-    }
     auto self = (SetUpCodePairer *) appState;
     self->OnWifiPAFDiscoveryError(err);
 }

src/include/platform/ConnectivityManager.h

@@ -182,6 +182,7 @@ class ConnectivityManager
     typedef void (*OnConnectionErrorFunct)(void * appState, CHIP_ERROR err);
     CHIP_ERROR WiFiPAFConnect(const SetupDiscriminator & connDiscriminator, void * appState, OnConnectionCompleteFunct onSuccess,
                               OnConnectionErrorFunct onError);
+    CHIP_ERROR WiFiPAFCancelConnect();
     CHIP_ERROR WiFiPAFSend(System::PacketBufferHandle && msgBuf);
     Transport::WiFiPAFBase * GetWiFiPAF();
     void SetWiFiPAF(Transport::WiFiPAFBase * pmWiFiPAF);
@@ -444,6 +445,11 @@ inline CHIP_ERROR ConnectivityManager::WiFiPAFConnect(const SetupDiscriminator &
     return static_cast<ImplClass *>(this)->_WiFiPAFConnect(connDiscriminator, appState, onSuccess, onError);
 }
 
+inline CHIP_ERROR ConnectivityManager::WiFiPAFCancelConnect()
+{
+    return static_cast<ImplClass *>(this)->_WiFiPAFCancelConnect();
+}
+
 inline CHIP_ERROR ConnectivityManager::WiFiPAFSend(chip::System::PacketBufferHandle && msgBuf)
 {
     return static_cast<ImplClass *>(this)->_WiFiPAFSend(std::move(msgBuf));

src/platform/Linux/ConnectivityManagerImpl.cpp

@@ -1444,6 +1444,20 @@ void ConnectivityManagerImpl::OnNanReceive(GVariant * obj)
     return;
 }
 
+void ConnectivityManagerImpl::OnNanSubscribeTerminated(gint term_subscribe_id, gint reason)
+{
+    ChipLogProgress(Controller, "WiFi-PAF: Subscription terminated (%d, %d)", term_subscribe_id, reason);
+    if (mpresubscribe_id == (uint32_t) term_subscribe_id)
+    {
+        mpresubscribe_id = 0;
+    }
+    if (mpaf_info.subscribe_id == (uint32_t) term_subscribe_id)
+    {
+        mpaf_info.subscribe_id = 0;
+    }
+    return;
+}
+
 CHIP_ERROR ConnectivityManagerImpl::_WiFiPAFConnect(const SetupDiscriminator & connDiscriminator, void * appState,
                                                     OnConnectionCompleteFunct onSuccess, OnConnectionErrorFunct onError)
 {
@@ -1478,6 +1492,7 @@ CHIP_ERROR ConnectivityManagerImpl::_WiFiPAFConnect(const SetupDiscriminator & c
     wpa_fi_w1_wpa_supplicant1_interface_call_nansubscribe_sync(mWpaSupplicant.iface, args, &subscribe_id, nullptr,
                                                                &err.GetReceiver());
     ChipLogProgress(DeviceLayer, "WiFi-PAF: subscribe_id: [%d]", subscribe_id);
+    mpresubscribe_id        = subscribe_id;
     mOnPafSubscribeComplete = onSuccess;
     mOnPafSubscribeError    = onError;
     g_signal_connect(mWpaSupplicant.iface, "nan-discoveryresult",
@@ -1491,6 +1506,27 @@ CHIP_ERROR ConnectivityManagerImpl::_WiFiPAFConnect(const SetupDiscriminator & c
                      }),
                      this);
 
+    g_signal_connect(mWpaSupplicant.iface, "nan-subscribeterminated",
+                     G_CALLBACK(+[](WpaFiW1Wpa_supplicant1Interface * proxy, gint term_subscribe_id, gint reason,
+                                    ConnectivityManagerImpl * self) {
+                         return self->OnNanSubscribeTerminated(term_subscribe_id, reason);
+                     }),
+                     this);
+
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR ConnectivityManagerImpl::_WiFiPAFCancelConnect()
+{
+    if (mpresubscribe_id == 0)
+    {
+        return CHIP_NO_ERROR;
+    }
+    GAutoPtr<GError> err;
+    gchar args[MAX_PAF_PUBLISH_SSI_BUFLEN];
+
+    snprintf(args, sizeof(args), "subscribe_id=%d", mpresubscribe_id);
+    wpa_fi_w1_wpa_supplicant1_interface_call_nancancel_subscribe_sync(mWpaSupplicant.iface, args, nullptr, &err.GetReceiver());
     return CHIP_NO_ERROR;
 }
 

src/platform/Linux/ConnectivityManagerImpl.h

@@ -143,9 +143,10 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
 #if CHIP_DEVICE_CONFIG_ENABLE_WIFIPAF
     CHIP_ERROR _WiFiPAFConnect(const SetupDiscriminator & connDiscriminator, void * appState, OnConnectionCompleteFunct onSuccess,
                                OnConnectionErrorFunct onError);
+    CHIP_ERROR _WiFiPAFCancelConnect();
     void OnDiscoveryResult(gboolean success, GVariant * obj);
     void OnNanReceive(GVariant * obj);
-
+    void OnNanSubscribeTerminated(gint term_subscribe_id, gint reason);
     CHIP_ERROR _WiFiPAFSend(chip::System::PacketBufferHandle && msgBuf);
     Transport::WiFiPAFBase * _GetWiFiPAF();
     void _SetWiFiPAF(Transport::WiFiPAFBase * pWiFiPAF);
@@ -236,6 +237,7 @@ class ConnectivityManagerImpl final : public ConnectivityManager,
         uint8_t peer_addr[6];
         uint32_t ssi_len;
     };
+    uint32_t mpresubscribe_id;
     struct wpa_dbus_discov_info mpaf_info;
     struct wpa_dbus_nanrx_info
     {