Release the capture region on both invoke paths, and stop duplicating the sample count
Three fixes from the same session, all of which had been left uncommitted. The tee_shm fd leak is the serious one. The per-frame capture region was released only on the invoke-FAILURE path, on the belief that an invoke consumes its input objects. That is libqcomtee's rule for callback objects, not memory objects -- its own ta_load.c hands a memory object in exactly this way and releases it unconditionally afterwards. So one fd leaked per captured frame, and the first daemon instance to live through ~1000 frames hit the 1024 limit: every capture then answered "memory region alloc failed" and an enrolment in progress ran out of frames with nothing on the sensor to blame. Soak-proven fixed at 93 frames with the fd count flat at 2-3. The enrolment sample count lived in two places, a hardcoded 20 here and common.max_enrolling_samples in the trustlet config. When the config said 30 the daemon still advertised 20 stages, so the progress counter froze at 20/20 and the run looked clamped or hung when in fact all 30 samples were taken. It now comes from the same JSON the trustlet is given, so the two cannot drift, with --samples= as a deliberate override. --probe-ta-load loads one TA image through the daemon's own loader and reports the raw loader result. It brings up only what a load needs and unloads afterwards, so a refusal is inert.
This commit is contained in:
parent
faf0dd9ba1
commit
36085c1b18
1 changed files with 99 additions and 11 deletions
|
|
@ -77,7 +77,15 @@ bool g_verbose = false;
|
||||||
// feels. A frame costs four QTEE round trips regardless; the gap on top is
|
// feels. A frame costs four QTEE round trips regardless; the gap on top is
|
||||||
// pure delay.
|
// pure delay.
|
||||||
int g_frameGapMs = 40;
|
int g_frameGapMs = 40;
|
||||||
int g_samples = 20; // common.max_enrolling_samples, stock's value
|
// The enrolment sample count lives in ONE place: common.max_enrolling_samples
|
||||||
|
// in the trustlet config, read by SyncConfig(). It is not duplicated here,
|
||||||
|
// because the trustlet enforces that value and the daemon only counts against
|
||||||
|
// it -- if the two disagree the progress reporting is silently wrong, which is
|
||||||
|
// how a 30-sample enrolment came to advertise 20 stages. -1 means "not read
|
||||||
|
// yet"; --samples= overrides for a deliberate experiment.
|
||||||
|
int g_samples = -1;
|
||||||
|
constexpr int SamplesFallback = 20; // stock's value, if the config lacks the key
|
||||||
|
bool g_samplesForced = false;
|
||||||
std::string g_logDir = "/var/log/fingerprintd";
|
std::string g_logDir = "/var/log/fingerprintd";
|
||||||
std::string g_stateDir = "/var/lib/fingerprintd";
|
std::string g_stateDir = "/var/lib/fingerprintd";
|
||||||
int g_rescan = -1; // -1 = leave the config's value alone
|
int g_rescan = -1; // -1 = leave the config's value alone
|
||||||
|
|
@ -93,6 +101,7 @@ bool g_undecidedIsNoMatch = false;
|
||||||
bool g_irqObserve = false;
|
bool g_irqObserve = false;
|
||||||
bool g_edgeWake = false;
|
bool g_edgeWake = false;
|
||||||
|
|
||||||
|
|
||||||
// The namespace key the trustlet hashes into the SFS group's directory name.
|
// The namespace key the trustlet hashes into the SFS group's directory name.
|
||||||
// It defaults to Android's because that is what this device's existing store
|
// It defaults to Android's because that is what this device's existing store
|
||||||
// was written under. It does NOT isolate anything -- SET_ACTIVE_GROUP's path
|
// was written under. It does NOT isolate anything -- SET_ACTIVE_GROUP's path
|
||||||
|
|
@ -968,8 +977,18 @@ CommandResult SendCommand(qcomtee_object* app, fingerprintd::ta::Cmd cmd,
|
||||||
//
|
//
|
||||||
// Two traps: the offsets array applies to EVERY command in a run, so it is
|
// Two traps: the offsets array applies to EVERY command in a run, so it is
|
||||||
// scoped to this one command -- patching a pointer into SYNC_CONFIG's
|
// scoped to this one command -- patching a pointer into SYNC_CONFIG's
|
||||||
// request breaks it. And an invoke CONSUMES its input objects, so the
|
// request breaks it. And the region is allocated fresh per frame and
|
||||||
// region is allocated fresh each time.
|
// released right after the invoke, success or not. An earlier version
|
||||||
|
// released it only on failure, believing "an invoke consumes its input
|
||||||
|
// objects" -- that is libqcomtee's rule for CALLBACK objects, not memory
|
||||||
|
// objects. Its own ta_load.c hands a memory object in exactly like this
|
||||||
|
// and releases it unconditionally afterwards ("QTEE releases its copy").
|
||||||
|
// Releasing only on failure leaked one tee_shm fd per capture; a
|
||||||
|
// long-lived daemon hit the 1024-fd limit after ~1000 frames, every
|
||||||
|
// capture then answered "memory region alloc failed", and an enrolment
|
||||||
|
// in progress ran out of frames with nothing on the sensor to blame.
|
||||||
|
// Nothing reads the region after the invoke -- the pixels never reach
|
||||||
|
// the normal world -- so there is no reason to hold it.
|
||||||
qcomtee_object* region = QCOMTEE_OBJECT_NULL;
|
qcomtee_object* region = QCOMTEE_OBJECT_NULL;
|
||||||
std::uint32_t offsets = tee::EmbeddedBufOffsetValue;
|
std::uint32_t offsets = tee::EmbeddedBufOffsetValue;
|
||||||
if (cmd == static_cast<ta::Cmd>(tee::RegionScopedToCommand)) {
|
if (cmd == static_cast<ta::Cmd>(tee::RegionScopedToCommand)) {
|
||||||
|
|
@ -986,11 +1005,11 @@ CommandResult SendCommand(qcomtee_object* app, fingerprintd::ta::Cmd cmd,
|
||||||
}
|
}
|
||||||
|
|
||||||
CommandResult out;
|
CommandResult out;
|
||||||
if (qcomtee_object_invoke(app, tee::AppSendRequestOp, p, 10, &out.result)) {
|
int invokeFailed = qcomtee_object_invoke(app, tee::AppSendRequestOp, p, 10, &out.result);
|
||||||
if (region != QCOMTEE_OBJECT_NULL)
|
if (region != QCOMTEE_OBJECT_NULL)
|
||||||
qcomtee_memory_object_release(region);
|
qcomtee_memory_object_release(region); // closes our fd; QTEE holds its own ref
|
||||||
|
if (invokeFailed)
|
||||||
return out;
|
return out;
|
||||||
}
|
|
||||||
out.invoked = true;
|
out.invoked = true;
|
||||||
out.rc = ta::ResultCode(reqOut);
|
out.rc = ta::ResultCode(reqOut);
|
||||||
out.metric = ta::CaptureMetric(reqOut);
|
out.metric = ta::CaptureMetric(reqOut);
|
||||||
|
|
@ -1198,7 +1217,7 @@ public:
|
||||||
std::println("enrolling gid={}", gid);
|
std::println("enrolling gid={}", gid);
|
||||||
|
|
||||||
en::TouchTracker tracker;
|
en::TouchTracker tracker;
|
||||||
en::EnrolSession enrol(g_samples);
|
en::EnrolSession enrol(EnrolStages());
|
||||||
int lastAccepted = 0;
|
int lastAccepted = 0;
|
||||||
bool pressHadTouch = false;
|
bool pressHadTouch = false;
|
||||||
for (int i = 0; i < maxFrames && !enrol.Complete() && !cancel; i++) {
|
for (int i = 0; i < maxFrames && !enrol.Complete() && !cancel; i++) {
|
||||||
|
|
@ -1438,7 +1457,9 @@ public:
|
||||||
}
|
}
|
||||||
|
|
||||||
bool FingerPresent() const { return fingerPresent_.load(); }
|
bool FingerPresent() const { return fingerPresent_.load(); }
|
||||||
int EnrolStages() const { return g_samples; }
|
// Before SyncConfig has run there is no answer yet; stock's value is the
|
||||||
|
// only honest stand-in, and a client asking this early gets it.
|
||||||
|
int EnrolStages() const { return g_samples > 0 ? g_samples : SamplesFallback; }
|
||||||
qcomtee_object* App() const { return app_; }
|
qcomtee_object* App() const { return app_; }
|
||||||
|
|
||||||
// IRQ edge observer: poll() the line and log each edge. This is the
|
// IRQ edge observer: poll() the line and log each edge. This is the
|
||||||
|
|
@ -1499,6 +1520,27 @@ private:
|
||||||
g_rescan);
|
g_rescan);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Take the sample count from the same JSON the trustlet is about to
|
||||||
|
// be given, so the two cannot drift. Deliberately crude: this is the
|
||||||
|
// only key the daemon needs back out, and pulling in a JSON parser to
|
||||||
|
// read one integer is not worth it.
|
||||||
|
if (!g_samplesForced) {
|
||||||
|
int found = -1;
|
||||||
|
for (std::string_view key : { "\"max_enrolling_samples\":", "\"max_enrolling_samples\" :" }) {
|
||||||
|
auto at = json.find(key);
|
||||||
|
if (at == std::string::npos) continue;
|
||||||
|
auto num = json.find_first_of("0123456789", at + key.size());
|
||||||
|
if (num == std::string::npos) continue;
|
||||||
|
found = std::atoi(json.c_str() + num);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
g_samples = (found > 0) ? found : SamplesFallback;
|
||||||
|
if (found <= 0)
|
||||||
|
std::println("config has no max_enrolling_samples; using {}", g_samples);
|
||||||
|
}
|
||||||
|
std::println("enrolment samples: {}{}", g_samples,
|
||||||
|
g_samplesForced ? " (forced on the command line)" : " (from the config)");
|
||||||
|
|
||||||
// The trustlet wants the terminating NUL counted.
|
// The trustlet wants the terminating NUL counted.
|
||||||
std::vector<std::byte> cfg(json.size() + 1, std::byte{0});
|
std::vector<std::byte> cfg(json.size() + 1, std::byte{0});
|
||||||
for (std::size_t i = 0; i < json.size(); i++)
|
for (std::size_t i = 0; i < json.size(); i++)
|
||||||
|
|
@ -2105,7 +2147,7 @@ GVariant* OnGetProperty(GDBusConnection*, const gchar*, const gchar*, const gcha
|
||||||
const gchar* prop, GError**, gpointer) {
|
const gchar* prop, GError**, gpointer) {
|
||||||
std::string_view p = prop;
|
std::string_view p = prop;
|
||||||
if (p == "name") return g_variant_new_string(DeviceName);
|
if (p == "name") return g_variant_new_string(DeviceName);
|
||||||
if (p == "num-enroll-stages") return g_variant_new_int32(g_worker ? g_worker->TheSession().EnrolStages() : 10);
|
if (p == "num-enroll-stages") return g_variant_new_int32(g_worker ? g_worker->TheSession().EnrolStages() : SamplesFallback);
|
||||||
if (p == "scan-type") return g_variant_new_string("press");
|
if (p == "scan-type") return g_variant_new_string("press");
|
||||||
if (p == "finger-present") return g_variant_new_boolean(g_worker && g_worker->TheSession().FingerPresent());
|
if (p == "finger-present") return g_variant_new_boolean(g_worker && g_worker->TheSession().FingerPresent());
|
||||||
if (p == "finger-needed") return g_variant_new_boolean(g_claim.op != Op::None);
|
if (p == "finger-needed") return g_variant_new_boolean(g_claim.op != Op::None);
|
||||||
|
|
@ -2200,9 +2242,52 @@ int RunProbe(bool doAuth, bool doEnrol, bool doCalSave, std::uint32_t gid, int f
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
|
|
||||||
|
// A bounded experiment: does the loader that accepts the OEM-signed focal64
|
||||||
|
// reject the SAME image with one code byte changed? This brings up ONLY what a
|
||||||
|
// load needs -- root, the supplicant (credentials is a callback object), the
|
||||||
|
// client env, and the compat loader -- then hands the image to loadFromBuffer
|
||||||
|
// and reports the loader's raw result. No listeners, no sensor, no bus. It
|
||||||
|
// UnloadStale()s first (inside LoadTrustlet) so a resident copy cannot mask
|
||||||
|
// the answer with "already loaded", and unloads a successful load so it leaves
|
||||||
|
// nothing resident. Refusal is inert: QTEE simply does not run the image.
|
||||||
|
int RunProbeTaLoad(const std::string& path) {
|
||||||
|
namespace tee = fingerprintd::tee;
|
||||||
|
std::string dev(tee::DevTee);
|
||||||
|
g_root = qcomtee_object_root_init(dev.c_str(), TeeCall, nullptr, nullptr);
|
||||||
|
if (g_root == QCOMTEE_OBJECT_NULL) {
|
||||||
|
std::println(std::cerr, "root object on {}: {}", tee::DevTee, ::strerror(errno));
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
pthread_t sup = 0;
|
||||||
|
if (pthread_create(&sup, nullptr, Supplicant, nullptr) != 0) {
|
||||||
|
std::println(std::cerr, "supplicant thread failed to start");
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
std::uint32_t uid = ::getuid();
|
||||||
|
qcomtee_object* env = GetClientEnv(uid);
|
||||||
|
if (env == QCOMTEE_OBJECT_NULL) return 1;
|
||||||
|
std::println("client env obtained (uid {})", uid);
|
||||||
|
qcomtee_object* loader = OpenService(env, tee::UidQseecomCompatAppLoader);
|
||||||
|
if (loader == QCOMTEE_OBJECT_NULL) return 1;
|
||||||
|
|
||||||
|
std::println("=== probe: loadFromBuffer('{}') ===", path);
|
||||||
|
qcomtee_object* app = LoadTrustlet(loader, path);
|
||||||
|
if (app == QCOMTEE_OBJECT_NULL) {
|
||||||
|
std::println("PROBE RESULT: loader REFUSED the image (see result= above)");
|
||||||
|
return 2;
|
||||||
|
}
|
||||||
|
qcomtee_result_t result = 0;
|
||||||
|
qcomtee_object_invoke(app, 2, nullptr, 0, &result); // op 2 = unload
|
||||||
|
std::println("PROBE RESULT: loader ACCEPTED the image; unloaded -> result={}",
|
||||||
|
static_cast<int>(result));
|
||||||
|
qcomtee_object_refs_dec(app);
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
int main(int argc, char** argv) {
|
int main(int argc, char** argv) {
|
||||||
std::span<char*> args(argv, static_cast<std::size_t>(argc));
|
std::span<char*> args(argv, static_cast<std::size_t>(argc));
|
||||||
bool probe = false, daemon = false, doAuth = false, doEnrol = false, doCalSave = false;
|
bool probe = false, daemon = false, doAuth = false, doEnrol = false, doCalSave = false;
|
||||||
|
std::string probeTa;
|
||||||
std::uint32_t gid = 0;
|
std::uint32_t gid = 0;
|
||||||
int frames = 120;
|
int frames = 120;
|
||||||
for (std::string_view a : args.subspan(1)) {
|
for (std::string_view a : args.subspan(1)) {
|
||||||
|
|
@ -2210,6 +2295,7 @@ int main(int argc, char** argv) {
|
||||||
if (a == "--daemon") daemon = true;
|
if (a == "--daemon") daemon = true;
|
||||||
if (a == "--probe-tee") probe = true;
|
if (a == "--probe-tee") probe = true;
|
||||||
if (a.starts_with("--ta=")) g_taPath = a.substr(5);
|
if (a.starts_with("--ta=")) g_taPath = a.substr(5);
|
||||||
|
if (a.starts_with("--probe-ta-load=")) probeTa = a.substr(16);
|
||||||
if (a.starts_with("--config=")) g_cfgPath = a.substr(9);
|
if (a.starts_with("--config=")) g_cfgPath = a.substr(9);
|
||||||
if (a == "--verbose") g_verbose = true;
|
if (a == "--verbose") g_verbose = true;
|
||||||
// Serving the store writable lets QTEE UNLINK a container it rejects,
|
// Serving the store writable lets QTEE UNLINK a container it rejects,
|
||||||
|
|
@ -2230,11 +2316,12 @@ int main(int argc, char** argv) {
|
||||||
if (a.starts_with("--state-dir=")) g_stateDir = a.substr(12);
|
if (a.starts_with("--state-dir=")) g_stateDir = a.substr(12);
|
||||||
if (a.starts_with("--rescan=")) g_rescan = std::stoi(std::string(a.substr(9)));
|
if (a.starts_with("--rescan=")) g_rescan = std::stoi(std::string(a.substr(9)));
|
||||||
if (a.starts_with("--group-path=")) g_groupPath = a.substr(13);
|
if (a.starts_with("--group-path=")) g_groupPath = a.substr(13);
|
||||||
if (a.starts_with("--samples=")) g_samples = std::stoi(std::string(a.substr(10)));
|
if (a.starts_with("--samples=")) { g_samples = std::stoi(std::string(a.substr(10))); g_samplesForced = true; }
|
||||||
if (a.starts_with("--sfs-root=")) g_sfsRoot = a.substr(11);
|
if (a.starts_with("--sfs-root=")) g_sfsRoot = a.substr(11);
|
||||||
if (a.starts_with("--gid=")) gid = static_cast<std::uint32_t>(std::stoul(std::string(a.substr(6))));
|
if (a.starts_with("--gid=")) gid = static_cast<std::uint32_t>(std::stoul(std::string(a.substr(6))));
|
||||||
}
|
}
|
||||||
StartTranscript(g_logDir);
|
StartTranscript(g_logDir);
|
||||||
|
if (!probeTa.empty()) return RunProbeTaLoad(probeTa);
|
||||||
if (daemon) return RunDaemon();
|
if (daemon) return RunDaemon();
|
||||||
if (probe) return RunProbe(doAuth, doEnrol, doCalSave, gid, frames);
|
if (probe) return RunProbe(doAuth, doEnrol, doCalSave, gid, frames);
|
||||||
|
|
||||||
|
|
@ -2242,6 +2329,7 @@ int main(int argc, char** argv) {
|
||||||
"fingerprintd {}\n"
|
"fingerprintd {}\n"
|
||||||
" --daemon own net.reactivated.Fprint on the system bus\n"
|
" --daemon own net.reactivated.Fprint on the system bus\n"
|
||||||
" --probe-tee [--gid=N] bring the session up and report\n"
|
" --probe-tee [--gid=N] bring the session up and report\n"
|
||||||
|
" --probe-ta-load=PATH load one TA image and report the loader result\n"
|
||||||
" --auth | --enrol | --cal-save diagnostic loops (see README)\n"
|
" --auth | --enrol | --cal-save diagnostic loops (see README)\n"
|
||||||
" --sfs-root=DIR --sfs-writable --rpmb-write storage policy",
|
" --sfs-root=DIR --sfs-writable --rpmb-write storage policy",
|
||||||
Version);
|
Version);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue