The RPMB result frame belongs in the shared buffer

SAVE_DATA now returns rc=0: 24 gpfile writes, 13 RPMB writes, no rollback.

The last fault was collecting the RPMB result frame into a local array. QTEE
reads it at req + req[0x0c] -- the same place the request frames were -- so
into a local means QTEE never sees the device's answer, fails the whole
transaction with an I/O error, and rolls back, having already committed the
counter. The reference passes the shared buffer as both source and result
destination for exactly this reason.

Also: req+0x14 is not always a usable chunk size. The reference falls back to
the whole block count when it is zero or exceeds nblocks, and refusing instead
aborts a legitimate write.

--cal-save drives a calibration save, which writes a real container through the
entire storage stack and needs NO FINGER. Three faults were found and fixed
with it in minutes, each of which would otherwise have cost a person ten
press-and-lift cycles to reach.

A process note worth more than the code. An earlier attempt at this appeared to
die mid-transaction; it did, and I killed it -- piping the phone's output
through `head` closed the pipe, SIGPIPE travelled back through tee, and the
daemon was terminated during an RPMB write sequence. That is precisely the
state the journal warns leaves a store inconsistent with a counter that cannot
be moved back. Never truncate a long-running device command's output; let it
finish and read its transcript.
This commit is contained in:
Jorijn van der Graaf 2026-09-02 21:48:04 +02:00
commit 9b03329aa0

View file

@ -55,6 +55,7 @@ bool g_verbose = false;
bool g_listeners = false; bool g_listeners = false;
bool g_auth = false; bool g_auth = false;
bool g_enrol = false; bool g_enrol = false;
bool g_calSave = false;
int g_frames = 40; int g_frames = 40;
int g_frameGapMs = 500; int g_frameGapMs = 500;
int g_samples = 10; // common.max_enrolling_samples, as shipped int g_samples = 10; // common.max_enrolling_samples, as shipped
@ -326,7 +327,8 @@ void ServeGpFile(std::span<std::byte> sb) {
f.read(reinterpret_cast<char*>(sb.data() + sfs::ReadDataOff), f.read(reinterpret_cast<char*>(sb.data() + sfs::ReadDataOff),
static_cast<std::streamsize>(want)); static_cast<std::streamsize>(want));
auto got = static_cast<std::uint32_t>(f.gcount()); auto got = static_cast<std::uint32_t>(f.gcount());
std::println(" read {} bytes into +0x{:03x}", got, sfs::ReadDataOff); std::println(" -> errno=0 count={} (asked {}, capacity {})", got,
req->length, sfs::Capacity(sb, sfs::Action::Read));
sfs::WriteReply(sb, 0, got); sfs::WriteReply(sb, 0, got);
return; return;
} }
@ -357,14 +359,19 @@ void ServeGpFile(std::span<std::byte> sb) {
std::size_t want = std::min<std::size_t>(req->length, std::size_t want = std::min<std::size_t>(req->length,
sfs::Capacity(sb, sfs::Action::Write)); sfs::Capacity(sb, sfs::Action::Write));
std::size_t done = 0; std::size_t done = 0;
int werr = 0;
while (done < want) { // short writes are real; the reference loops while (done < want) { // short writes are real; the reference loops
ssize_t n = ::write(fd, sb.data() + sfs::WriteDataOff + done, want - done); ssize_t n = ::write(fd, sb.data() + sfs::WriteDataOff + done, want - done);
if (n <= 0) break; if (n < 0) { werr = errno; break; }
if (n == 0) break;
done += static_cast<std::size_t>(n); done += static_cast<std::size_t>(n);
} }
::fsync(fd); ::fsync(fd);
::close(fd); ::close(fd);
sfs::WriteReply(sb, 0, static_cast<std::uint32_t>(done)); std::println(" -> errno={} count={} (asked {}, capacity {})", werr, done,
req->length, sfs::Capacity(sb, sfs::Action::Write));
sfs::WriteReply(sb, static_cast<std::uint32_t>(werr),
static_cast<std::uint32_t>(done));
return; return;
} }
case sfs::Action::Unlink: case sfs::Action::Unlink:
@ -515,30 +522,45 @@ void ServeRpmb(std::span<std::byte> sb) {
// A remainder is refused rather than partially committed. The // A remainder is refused rather than partially committed. The
// reference silently drops one, which would leave the store // reference silently drops one, which would leave the store
// inconsistent with a counter that cannot be moved back. // inconsistent with a counter that cannot be moved back.
auto plan = rp::PlanChunks(req->nblocks, req->blocksPerOp); // req+0x14 is the chunk size, but it is not always usable: the
// reference falls back to the whole block count when it is zero or
// larger than nblocks.
std::uint32_t bpo = req->blocksPerOp;
if (bpo == 0 || bpo > req->nblocks) {
std::println(" rpmb: chunk size {} unusable, using nblocks={}", bpo,
req->nblocks);
bpo = req->nblocks;
}
auto plan = rp::PlanChunks(req->nblocks, bpo);
if (!plan.exact) { if (!plan.exact) {
std::println(" rpmb: {} blocks is not a whole number of {}-block chunks" std::println(" rpmb: {} blocks is not a whole number of {}-block chunks"
" -- refusing", req->nblocks, req->blocksPerOp); " -- refusing", req->nblocks, bpo);
} else { } else {
std::array<std::byte, rp::FrameSize> rrq{}; std::array<std::byte, rp::FrameSize> rrq{};
rp::BuildResultReadRequest(rrq); rp::BuildResultReadRequest(rrq);
std::array<std::byte, rp::FrameSize> result{};
rc = 0; rc = 0;
for (std::uint32_t k = 0; k < plan.chunks && rc == 0; k++) { for (std::uint32_t k = 0; k < plan.chunks && rc == 0; k++) {
std::byte* chunk = frames + static_cast<std::size_t>(k) std::byte* chunk = frames + static_cast<std::size_t>(k)
* req->blocksPerOp * rp::FrameSize; * bpo * rp::FrameSize;
std::uint32_t bytes = req->blocksPerOp std::uint32_t bytes = bpo * static_cast<std::uint32_t>(rp::FrameSize);
* static_cast<std::uint32_t>(rp::FrameSize); // The RESULT FRAME GOES BACK INTO THE SHARED BUFFER, at the
// data offset -- QTEE reads it at req + req[0x0c], which is
// exactly where the request frames were. Collecting it into a
// local means QTEE never sees the device's answer and fails
// the whole transaction with an I/O error, having already
// committed the counter.
std::byte* result = frames;
if (SecurityProtocolRetry(fd, false, chunk, bytes) != 0 || if (SecurityProtocolRetry(fd, false, chunk, bytes) != 0 ||
SecurityProtocolRetry(fd, false, rrq.data(), rp::FrameSize) != 0 || SecurityProtocolRetry(fd, false, rrq.data(), rp::FrameSize) != 0 ||
SecurityProtocolRetry(fd, true, result.data(), rp::FrameSize) != 0) { SecurityProtocolRetry(fd, true, result, rp::FrameSize) != 0) {
rc = -1; rc = -1;
break; break;
} }
std::uint16_t res = rp::ResultOf(result); std::span<const std::byte> rf(result, rp::FrameSize);
std::uint16_t res = rp::ResultOf(rf);
std::println(" rpmb write chunk {}/{}: result=0x{:04x} ({}) counter={}", std::println(" rpmb write chunk {}/{}: result=0x{:04x} ({}) counter={}",
k + 1, plan.chunks, res, rp::ResultString(res), k + 1, plan.chunks, res, rp::ResultString(res),
rp::WriteCounterOf(result)); rp::WriteCounterOf(rf));
// Anything non-zero aborts rather than continuing into further // Anything non-zero aborts rather than continuing into further
// chunks: the device rejected the frame and the counter state // chunks: the device rejected the frame and the counter state
// is not what we think it is. // is not what we think it is.
@ -1232,6 +1254,25 @@ int Probe() {
std::println(" {}", tally.Identified() ? "FINGER IDENTIFIED" : "no match"); std::println(" {}", tally.Identified() ? "FINGER IDENTIFIED" : "no match");
} }
// ---- Calibration save
//
// SAVE_DATA with bit 30 CLEAR takes the calibration path, which writes a
// real container through the whole storage stack and needs NO FINGER. That
// makes it the way to debug the write path without a person present.
if (g_calSave) {
namespace ta = fingerprintd::ta;
if (g_sfsReadOnly) {
std::println(std::cerr, "a calibration save writes; pass --sfs-writable");
return 1;
}
std::vector<std::byte> sd(0x10, std::byte{0});
for (std::size_t k = 0; k < 4; k++)
sd[k] = static_cast<std::byte>((ta::SaveMaskCalibration >> (8 * k)) & 0xFF);
std::println("\n=== SAVE_DATA (calibration, no finger needed) ===");
auto sv = SendCommand(app, ta::Cmd::SaveData, sd);
Report(ta::Cmd::SaveData, sv);
}
// ---- Enrolment // ---- Enrolment
// //
// The first thing here that WRITES: template containers through the gpfile // The first thing here that WRITES: template containers through the gpfile
@ -1390,6 +1431,7 @@ int main(int argc, char** argv) {
if (a == "--rpmb-write") g_rpmbWrite = true; if (a == "--rpmb-write") g_rpmbWrite = true;
if (a == "--auth") { g_auth = true; g_listeners = true; } if (a == "--auth") { g_auth = true; g_listeners = true; }
if (a == "--enrol") { g_enrol = true; g_listeners = true; } if (a == "--enrol") { g_enrol = true; g_listeners = true; }
if (a == "--cal-save") { g_calSave = true; g_listeners = true; g_verbose = true; }
if (a.starts_with("--frames=")) g_frames = std::stoi(std::string(a.substr(9))); if (a.starts_with("--frames=")) g_frames = std::stoi(std::string(a.substr(9)));
if (a.starts_with("--log-dir=")) g_logDir = a.substr(10); if (a.starts_with("--log-dir=")) g_logDir = a.substr(10);
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)));