From da9f5fe137ffaf1b020657e6a9ac8b820f28a089 Mon Sep 17 00:00:00 2001
From: Les Lee <lesl@google.com>
Date: Tue, 29 Jun 2021 22:48:11 +0800
Subject: [PATCH] WIFI: Set MAC address for bridged interface

The MAC address of the bridged interface will be dynamically generated
by kernel when any bridged iface is changed. This means that the
bridged interface MAC address will be changed when we remove one of the
instances from the bridged interface (shutdown unused interface case).
The MAC change will break operation of bpf and it may cause the SAP
client to send wrong ns packets because the tethering module is
still using the old MAC in the ra packet.

Always set MAC address so the bridged interface can avoid MAC changing.

Bug: 191611764
Bug: 192315721
Test: Manual test with IPv6 tethering. Make sure client won't disconnect
because it doesn't get na response.
Test: Manual test in two scenarios: 1. MAC randomization 2. reset to factory MAC.

Change-Id: I854fc74b6532824b7d7b5a1aa4bc20a3cf9fd588
---
 wifi/1.5/default/wifi_ap_iface.cpp   | 27 ++++++++++++++++++++-------
 wifi/1.5/default/wifi_iface_util.cpp |  4 ++--
 wifi/1.5/default/wifi_iface_util.h   |  4 ++--
 3 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/wifi/1.5/default/wifi_ap_iface.cpp b/wifi/1.5/default/wifi_ap_iface.cpp
index b438a4a832..1ae7905f74 100644
--- a/wifi/1.5/default/wifi_ap_iface.cpp
+++ b/wifi/1.5/default/wifi_ap_iface.cpp
@@ -136,24 +136,25 @@ WifiApIface::getValidFrequenciesForBandInternal(V1_0::WifiBand band) {
 
 WifiStatus WifiApIface::setMacAddressInternal(
     const std::array<uint8_t, 6>& mac) {
-    bool status;
     // Support random MAC up to 2 interfaces
     if (instances_.size() == 2) {
         int rbyte = 1;
         for (auto const& intf : instances_) {
             std::array<uint8_t, 6> rmac = mac;
-            // reverse the bits to avoid clision
+            // reverse the bits to avoid collision
             rmac[rbyte] = 0xff - rmac[rbyte];
-            status = iface_util_.lock()->setMacAddress(intf, rmac);
-            if (!status) {
+            if (!iface_util_.lock()->setMacAddress(intf, rmac)) {
                 LOG(INFO) << "Failed to set random mac address on " << intf;
+                return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN);
             }
             rbyte++;
         }
-    } else {
-        status = iface_util_.lock()->setMacAddress(ifname_, mac);
     }
-    if (!status) {
+    // It also needs to set mac address for bridged interface, otherwise the mac
+    // address of bridged interface will be changed after one of instance
+    // down.
+    if (!iface_util_.lock()->setMacAddress(ifname_, mac)) {
+        LOG(ERROR) << "Fail to config MAC for interface " << ifname_;
         return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN);
     }
     return createWifiStatus(WifiStatusCode::SUCCESS);
@@ -181,6 +182,18 @@ WifiStatus WifiApIface::resetToFactoryMacAddressInternal() {
                 return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN);
             }
         }
+        // It needs to set mac address for bridged interface, otherwise the mac
+        // address of the bridged interface will be changed after one of the
+        // instance down. Thus we are generating a random MAC address for the
+        // bridged interface even if we got the request to reset the Factory
+        // MAC. Since the bridged interface is an internal interface for the
+        // operation of bpf and others networking operation.
+        if (!iface_util_.lock()->setMacAddress(
+                ifname_, iface_util_.lock()->createRandomMacAddress())) {
+            LOG(ERROR) << "Fail to config MAC for bridged interface "
+                       << ifname_;
+            return createWifiStatus(WifiStatusCode::ERROR_UNKNOWN);
+        }
     } else {
         getMacResult = getFactoryMacAddressInternal(ifname_);
         LOG(DEBUG) << "Reset MAC to factory MAC on " << ifname_;
diff --git a/wifi/1.5/default/wifi_iface_util.cpp b/wifi/1.5/default/wifi_iface_util.cpp
index d1434e3a41..7bf830b875 100644
--- a/wifi/1.5/default/wifi_iface_util.cpp
+++ b/wifi/1.5/default/wifi_iface_util.cpp
@@ -86,9 +86,9 @@ bool WifiIfaceUtil::setMacAddress(const std::string& iface_name,
         event_handlers.on_state_toggle_off_on(iface_name);
     }
     if (!success) {
-        LOG(ERROR) << "SetMacAddress failed.";
+        LOG(ERROR) << "SetMacAddress failed on " << iface_name;
     } else {
-        LOG(DEBUG) << "SetMacAddress succeeded.";
+        LOG(DEBUG) << "SetMacAddress succeeded on " << iface_name;
     }
     return success;
 }
diff --git a/wifi/1.5/default/wifi_iface_util.h b/wifi/1.5/default/wifi_iface_util.h
index b449077e9e..544f575d41 100644
--- a/wifi/1.5/default/wifi_iface_util.h
+++ b/wifi/1.5/default/wifi_iface_util.h
@@ -71,10 +71,10 @@ class WifiIfaceUtil {
 
     virtual bool removeIfaceFromBridge(const std::string& br_name,
                                        const std::string& if_name);
+    // Get a random MAC address.
+    virtual std::array<uint8_t, 6> createRandomMacAddress();
 
    private:
-    std::array<uint8_t, 6> createRandomMacAddress();
-
     std::weak_ptr<wifi_system::InterfaceTool> iface_tool_;
     std::weak_ptr<legacy_hal::WifiLegacyHal> legacy_hal_;
     std::unique_ptr<std::array<uint8_t, 6>> random_mac_address_;