Skip to content

Commit 047ff11

Browse files
senamakelmedullabot
andcommitted
fix(pairing): destroy port backing before refunding capacity
Co-authored-by: Medulla <medulla@tinyhumans.ai>
1 parent bc8b7dc commit 047ff11

2 files changed

Lines changed: 127 additions & 6 deletions

File tree

‎crates/tinysecurity-crypto/src/pairing.rs‎

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -267,19 +267,29 @@ pub struct PortBuffer {
267267
inner: Arc<Inner>,
268268
account: usize,
269269
binding: [u8; 32],
270-
bytes: StagingBytes,
270+
bytes: Option<StagingBytes>,
271271
slot: usize,
272272
charge: usize,
273+
#[cfg(test)]
274+
after_drop_unlock: Option<Box<dyn FnOnce() + Send>>,
273275
}
274276
impl Drop for PortBuffer {
275277
fn drop(&mut self) {
276278
if let Ok(mut state) = self.inner.state.lock() {
277-
state.pending_bytes = state.pending_bytes.saturating_sub(self.charge);
278279
if let Some(pending) = state.accounts[self.account].pending.as_mut()
279280
&& pending.binding == self.binding
280281
{
281282
pending.buffers[self.slot] = None;
282283
}
284+
// No backing Arc escapes these two private owners. Destroy the final
285+
// Arc, its zeroizing Vec and allocation before admission can observe
286+
// a refund; implicit field Drop would happen after unlocking State.
287+
drop(self.bytes.take());
288+
state.pending_bytes = state.pending_bytes.saturating_sub(self.charge);
289+
}
290+
#[cfg(test)]
291+
if let Some(observe) = self.after_drop_unlock.take() {
292+
observe();
283293
}
284294
}
285295
}
@@ -291,7 +301,12 @@ impl PortBuffer {
291301
pub fn with_bytes(&mut self, f: impl FnOnce(&mut [u8]) -> Result<()>) -> Result<()> {
292302
let mut state = self.inner.lock()?;
293303
self.inner.check(&mut state, self.account, &self.binding)?;
294-
let mut bytes = self.bytes.lock().map_err(|_| Error::NativeDenied)?;
304+
let mut bytes = self
305+
.bytes
306+
.as_ref()
307+
.ok_or(Error::NativeDenied)?
308+
.lock()
309+
.map_err(|_| Error::NativeDenied)?;
295310
f(&mut bytes)
296311
}
297312
}
@@ -346,7 +361,12 @@ impl StagedDelivery {
346361
return Err(Error::NativeDenied);
347362
}
348363
self.with_staged_secret(|secret| {
349-
let mut out = buffer.bytes.lock().map_err(|_| Error::NativeDenied)?;
364+
let mut out = buffer
365+
.bytes
366+
.as_ref()
367+
.ok_or(Error::NativeDenied)?
368+
.lock()
369+
.map_err(|_| Error::NativeDenied)?;
350370
if out.len() < secret.len() {
351371
return Err(Error::NativeCapacity);
352372
}
@@ -629,9 +649,11 @@ impl Inner {
629649
inner: Arc::clone(self),
630650
account,
631651
binding: *binding,
632-
bytes,
652+
bytes: Some(bytes),
633653
slot,
634654
charge,
655+
#[cfg(test)]
656+
after_drop_unlock: None,
635657
})
636658
}
637659
fn lock(&self) -> Result<MutexGuard<'_, State>> {

‎crates/tinysecurity-crypto/src/pairing_tests.rs‎

Lines changed: 100 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1127,7 +1127,16 @@ async fn full_backing_charge_survives_revoke_and_releases_only_with_owned_buffer
11271127
})
11281128
.is_err()
11291129
);
1130-
assert!(buffer.bytes.lock().checked().iter().all(|b| *b == 0));
1130+
assert!(
1131+
buffer
1132+
.bytes
1133+
.as_ref()
1134+
.checked()
1135+
.lock()
1136+
.checked()
1137+
.iter()
1138+
.all(|b| *b == 0)
1139+
);
11311140
task.abort();
11321141
let _ = task.await;
11331142
assert_eq!(owner.inner.state.lock().checked().pending_bytes, charge);
@@ -1398,3 +1407,93 @@ async fn unpublished_retained_token_reservation_releases_once_on_cancel() {
13981407
assert_eq!(state.reserved_tokens, 0);
13991408
assert_eq!(state.tokens, 0);
14001409
}
1410+
1411+
#[tokio::test]
1412+
async fn refunded_port_capacity_is_reusable_only_after_backing_destruction() {
1413+
let (_, clock, _, verifier, entropy) = setup(true, vec![], Limits::default());
1414+
let mut second = registration(true, vec![]);
1415+
second.account = "second".into();
1416+
let owner = Arc::new(
1417+
PairingOwner::construct(
1418+
vec![registration(true, vec![]), second],
1419+
1,
1420+
Limits {
1421+
pending_bytes: 2 * OP_BYTES + 8192 + 256,
1422+
..Limits::default()
1423+
},
1424+
300_000,
1425+
clock,
1426+
Arc::new(AggregateConsumer),
1427+
verifier.clone(),
1428+
entropy,
1429+
)
1430+
.checked(),
1431+
);
1432+
let first = reference(&owner).await;
1433+
let other = TrustedAccountRequest {
1434+
account: "second",
1435+
..account_request()
1436+
};
1437+
let second = match owner.begin(&other).await.checked() {
1438+
BeginOutcome::Active(reference) => reference,
1439+
BeginOutcome::Inactive => unreachable!("second challenge must be active"),
1440+
};
1441+
verifier.pause.store(true, Ordering::SeqCst);
1442+
let mut first_work = Box::pin(owner.verify(attempt(&first, "000000")));
1443+
let mut second_attempt = attempt(&second, "000000");
1444+
second_attempt.account = other;
1445+
let mut second_work = Box::pin(owner.verify(second_attempt));
1446+
for work in [&mut first_work, &mut second_work] {
1447+
let polled =
1448+
std::future::poll_fn(|cx| std::task::Poll::Ready(work.as_mut().poll(cx))).await;
1449+
assert!(polled.is_pending());
1450+
}
1451+
let deliveries: Vec<_> = (0..2)
1452+
.map(|account| StagedDelivery {
1453+
inner: owner.inner.clone(),
1454+
account,
1455+
binding: owner.inner.state.lock().checked().accounts[account]
1456+
.pending
1457+
.as_ref()
1458+
.checked()
1459+
.binding,
1460+
})
1461+
.collect();
1462+
let mut first_buffer = deliveries[0].allocate_buffer(8192).checked();
1463+
first_buffer
1464+
.with_bytes(|bytes| {
1465+
bytes.fill(7);
1466+
Ok(())
1467+
})
1468+
.checked();
1469+
assert!(matches!(
1470+
deliveries[1].allocate_buffer(8192),
1471+
Err(Error::NativeCapacity)
1472+
));
1473+
// Weak observation adds no strong backing owner. This callback is the exact
1474+
// preemption boundary after unlocking State and before implicit field Drop;
1475+
// it runs outside the lock, with both accounts' original work still pending.
1476+
let first_backing = Arc::downgrade(first_buffer.bytes.as_ref().checked());
1477+
let next_delivery = StagedDelivery {
1478+
inner: owner.inner.clone(),
1479+
account: 1,
1480+
binding: deliveries[1].binding,
1481+
};
1482+
let observed = Arc::new(AtomicBool::new(false));
1483+
let observed_drop = observed.clone();
1484+
first_buffer.after_drop_unlock = Some(Box::new(move || {
1485+
let next_buffer = next_delivery.allocate_buffer(8192).checked();
1486+
assert_eq!(
1487+
first_backing.strong_count(),
1488+
0,
1489+
"refunded capacity admitted another backing before the old backing was destroyed"
1490+
);
1491+
observed_drop.store(true, Ordering::SeqCst);
1492+
drop(next_buffer);
1493+
}));
1494+
drop(first_buffer);
1495+
assert!(observed.load(Ordering::SeqCst));
1496+
drop(first_work);
1497+
drop(second_work);
1498+
assert_eq!(owner.inner.state.lock().checked().pending_bytes, 0);
1499+
}

0 commit comments

Comments
 (0)