Improve the locking design inside TrafficController
Rename the mOwnerMatchMutex lock to mMutex and use it to protect all
data structures in TrafficController that could be accessed
concurrently. Add necessary annotations to the data structures needing
protection to catch unsafe access at compile time.
Test: netd_unit_test, netd_integration_test
Bug: 130334320
Change-Id: I847ce518df8626f647e1d80468e4daf4604872f2
diff --git a/server/TrafficController.h b/server/TrafficController.h
index d30749b..cafc319 100644
--- a/server/TrafficController.h
+++ b/server/TrafficController.h
@@ -57,7 +57,7 @@
* the spinlock initialized with the map. So the behavior of two modules
* should be the same. No additional lock needed.
*/
- int tagSocket(int sockFd, uint32_t tag, uid_t uid, uid_t callingUid);
+ int tagSocket(int sockFd, uint32_t tag, uid_t uid, uid_t callingUid) EXCLUDES(mMutex);
/*
* The untag process is similiar to tag socket and both old qtaguid module and
@@ -69,7 +69,7 @@
/*
* Similiar as above, no external lock required.
*/
- int setCounterSet(int counterSetNum, uid_t uid, uid_t callingUid);
+ int setCounterSet(int counterSetNum, uid_t uid, uid_t callingUid) EXCLUDES(mMutex);
/*
* When deleting a tag data, the qtaguid module will grab the spinlock of each
@@ -79,7 +79,7 @@
* each map one by one. And deleting processes are also protected by the
* spinlock of the map. So no additional lock is required.
*/
- int deleteTagData(uint32_t tag, uid_t uid, uid_t callingUid);
+ int deleteTagData(uint32_t tag, uid_t uid, uid_t callingUid) EXCLUDES(mMutex);
/*
* Check if the current device have the bpf traffic stats accounting service
@@ -90,7 +90,7 @@
/*
* Swap the stats map config from current active stats map to the idle one.
*/
- netdutils::Status swapActiveStatsMap();
+ netdutils::Status swapActiveStatsMap() EXCLUDES(mMutex);
/*
* Add the interface name and index pair into the eBPF map.
@@ -105,25 +105,27 @@
const std::vector<int32_t>& uids);
netdutils::Status updateOwnerMapEntry(UidOwnerMatchType match, uid_t uid, FirewallRule rule,
- FirewallType type);
+ FirewallType type) EXCLUDES(mMutex);
- void dump(netdutils::DumpWriter& dw, bool verbose);
+ void dump(netdutils::DumpWriter& dw, bool verbose) EXCLUDES(mMutex);
- netdutils::Status replaceRulesInMap(UidOwnerMatchType match, const std::vector<int32_t>& uids);
+ netdutils::Status replaceRulesInMap(UidOwnerMatchType match, const std::vector<int32_t>& uids)
+ EXCLUDES(mMutex);
- netdutils::Status addUidInterfaceRules(const int ifIndex, const std::vector<int32_t>& uids);
- netdutils::Status removeUidInterfaceRules(const std::vector<int32_t>& uids);
+ netdutils::Status addUidInterfaceRules(const int ifIndex, const std::vector<int32_t>& uids)
+ EXCLUDES(mMutex);
+ netdutils::Status removeUidInterfaceRules(const std::vector<int32_t>& uids) EXCLUDES(mMutex);
netdutils::Status updateUidOwnerMap(const std::vector<std::string>& appStrUids,
BandwidthController::IptJumpOp jumpHandling,
- BandwidthController::IptOp op);
+ BandwidthController::IptOp op) EXCLUDES(mMutex);
static const String16 DUMP_KEYWORD;
- int toggleUidOwnerMap(ChildChain chain, bool enable);
+ int toggleUidOwnerMap(ChildChain chain, bool enable) EXCLUDES(mMutex);
static netdutils::StatusOr<std::unique_ptr<NetlinkListenerInterface>> makeSkDestroyListener();
- void setPermissionForUids(int permission, const std::vector<uid_t>& uids);
+ void setPermissionForUids(int permission, const std::vector<uid_t>& uids) EXCLUDES(mMutex);
private:
/*
@@ -162,9 +164,9 @@
* Map Value: struct Stats, contains packet count and byte count of each
* transport protocol on egress and ingress direction.
*/
- BpfMap<StatsKey, StatsValue> mStatsMapA;
+ BpfMap<StatsKey, StatsValue> mStatsMapA GUARDED_BY(mMutex);
- BpfMap<StatsKey, StatsValue> mStatsMapB;
+ BpfMap<StatsKey, StatsValue> mStatsMapB GUARDED_BY(mMutex);
/*
* mIfaceIndexNameMap: Store the index name pair of each interface show up
@@ -191,42 +193,42 @@
* Userspace can do scraping and cleaning job on the other one depending on the
* current configs.
*/
- BpfMap<uint32_t, uint8_t> mConfigurationMap GUARDED_BY(mOwnerMatchMutex);
+ BpfMap<uint32_t, uint8_t> mConfigurationMap GUARDED_BY(mMutex);
/*
* mUidOwnerMap: Store uids that are used for bandwidth control uid match.
*/
- BpfMap<uint32_t, UidOwnerValue> mUidOwnerMap GUARDED_BY(mOwnerMatchMutex);
+ BpfMap<uint32_t, UidOwnerValue> mUidOwnerMap GUARDED_BY(mMutex);
/*
* mUidOwnerMap: Store uids that are used for INTERNET permission check.
*/
- BpfMap<uint32_t, uint8_t> mUidPermissionMap;
+ BpfMap<uint32_t, uint8_t> mUidPermissionMap GUARDED_BY(mMutex);
std::unique_ptr<NetlinkListenerInterface> mSkDestroyListener;
netdutils::Status removeRule(BpfMap<uint32_t, UidOwnerValue>& map, uint32_t uid,
- UidOwnerMatchType match) REQUIRES(mOwnerMatchMutex);
+ UidOwnerMatchType match) REQUIRES(mMutex);
netdutils::Status addRule(BpfMap<uint32_t, UidOwnerValue>& map, uint32_t uid,
- UidOwnerMatchType match, uint32_t iif = 0) REQUIRES(mOwnerMatchMutex);
+ UidOwnerMatchType match, uint32_t iif = 0) REQUIRES(mMutex);
bpf::BpfLevel mBpfLevel;
- std::mutex mOwnerMatchMutex;
+ std::mutex mMutex;
netdutils::Status loadAndAttachProgram(bpf_attach_type type, const char* path, const char* name,
base::unique_fd& cg_fd);
- netdutils::Status initMaps();
+ netdutils::Status initMaps() EXCLUDES(mMutex);
// Keep track of uids that have permission UPDATE_DEVICE_STATS so we don't
// need to call back to system server for permission check.
- std::set<uid_t> mPrivilegedUser;
+ std::set<uid_t> mPrivilegedUser GUARDED_BY(mMutex);
UidOwnerMatchType jumpOpToMatch(BandwidthController::IptJumpOp jumpHandling);
- bool hasUpdateDeviceStatsPermission(uid_t uid);
+ bool hasUpdateDeviceStatsPermission(uid_t uid) REQUIRES(mMutex);
// For testing
friend class TrafficControllerTest;
};