Ueventd: Fix a corner case in ReadUevent() that triggers duplicate firmware loading.
Presently, within ReadUevent(), true is returned for a successful case
as well as for the case where we read an invalid uevent (overflowed
buffer)where the Uevent object is not cleared, and the caller calls the
callback (with the earlier stored uevent object),leading to duplicate
firmware loading.
Uevent uevent;
while (ReadUevent(&uevent)) {
if (callback(uevent) == ListenerAction::kStop) return;
}
Scenario:
1. Proper Uevent received and callback is called (firmware loading
is triggered).
2. Overflowed uevent is received as part of the same ReadUevent session,
ReadUevent() returns true, but the uevent object is not cleared and
still has earlier event values.
3. Callback is called again, leading to duplicate firmware load.
Handle it by adding explicit return codes to let the caller know if the
uevent read is invalid, and the caller can ignore it and read
further pending uevents.
Bug: 161580785
Test: in AOSP
Merged-In: I09e80052337fd1495b968dc02ecff5ceb683da18
Change-Id: I09e80052337fd1495b968dc02ecff5ceb683da18
(cherry picked from commit a86bf05a012e871c41b4676b0cc947b1baea194b)
diff --git a/init/uevent_listener.cpp b/init/uevent_listener.cpp
index d8d9b36..7cd396a 100644
--- a/init/uevent_listener.cpp
+++ b/init/uevent_listener.cpp
@@ -95,20 +95,18 @@
fcntl(device_fd_, F_SETFL, O_NONBLOCK);
}
-bool UeventListener::ReadUevent(Uevent* uevent) const {
+ReadUeventResult UeventListener::ReadUevent(Uevent* uevent) const {
char msg[UEVENT_MSG_LEN + 2];
int n = uevent_kernel_multicast_recv(device_fd_, msg, UEVENT_MSG_LEN);
if (n <= 0) {
if (errno != EAGAIN && errno != EWOULDBLOCK) {
PLOG(ERROR) << "Error reading from Uevent Fd";
}
- return false;
+ return ReadUeventResult::kFailed;
}
if (n >= UEVENT_MSG_LEN) {
LOG(ERROR) << "Uevent overflowed buffer, discarding";
- // Return true here even if we discard as we may have more uevents pending and we
- // want to keep processing them.
- return true;
+ return ReadUeventResult::kInvalid;
}
msg[n] = '\0';
@@ -116,7 +114,7 @@
ParseEvent(msg, uevent);
- return true;
+ return ReadUeventResult::kSuccess;
}
// RegenerateUevents*() walks parts of the /sys tree and pokes the uevent files to cause the kernel
@@ -137,7 +135,10 @@
close(fd);
Uevent uevent;
- while (ReadUevent(&uevent)) {
+ ReadUeventResult result;
+ while ((result = ReadUevent(&uevent)) != ReadUeventResult::kFailed) {
+ // Skip processing the uevent if it is invalid.
+ if (result == ReadUeventResult::kInvalid) continue;
if (callback(uevent) == ListenerAction::kStop) return ListenerAction::kStop;
}
}
@@ -212,7 +213,10 @@
// We're non-blocking, so if we receive a poll event keep processing until
// we have exhausted all uevent messages.
Uevent uevent;
- while (ReadUevent(&uevent)) {
+ ReadUeventResult result;
+ while ((result = ReadUevent(&uevent)) != ReadUeventResult::kFailed) {
+ // Skip processing the uevent if it is invalid.
+ if (result == ReadUeventResult::kInvalid) continue;
if (callback(uevent) == ListenerAction::kStop) return;
}
}
diff --git a/init/uevent_listener.h b/init/uevent_listener.h
index aea094e..d308fa7 100644
--- a/init/uevent_listener.h
+++ b/init/uevent_listener.h
@@ -37,6 +37,12 @@
kContinue, // Continue regenerating uevents as we haven't seen the one(s) we're interested in.
};
+enum class ReadUeventResult {
+ kSuccess = 0, // Uevent was successfully read.
+ kFailed, // Uevent reading has failed.
+ kInvalid, // An Invalid Uevent was read (like say, the msg received is >= UEVENT_MSG_LEN).
+};
+
using ListenerCallback = std::function<ListenerAction(const Uevent&)>;
class UeventListener {
@@ -50,7 +56,7 @@
const std::optional<std::chrono::milliseconds> relative_timeout = {}) const;
private:
- bool ReadUevent(Uevent* uevent) const;
+ ReadUeventResult ReadUevent(Uevent* uevent) const;
ListenerAction RegenerateUeventsForDir(DIR* d, const ListenerCallback& callback) const;
android::base::unique_fd device_fd_;