open https://gitlab.synchro.net/main/sbbs/-/issues/1241

## Summary

`sbbs_t::qwktonetmail()` (`src/sbbs3/netmail.cpp`) allocates message **data** storage without holding the SMB header lock. Every other allocation site in the tree holds `smb_locksmbhdr()` across the allocation, so two concurrent writers can select the same free blocks and produce two messages that share data storage.

## Evidence

Lock held across the `smb_allocdat()` / `smb_fallocdat()` call:

| site | header lock held |
|---|---|
| `smb_addmsg()` (smbadd.c) | yes, taken at function top |
| `email.cpp` (`sbbs_t::email`) | yes |
| `netmail.cpp` (`sbbs_t::netmail`) | yes |
| `writemsg.cpp` (`sbbs_t::savemsg`) | yes |
| `writemsg.cpp` (`sbbs_t::movemsg`) | yes |
| **`netmail.cpp` (`sbbs_t::qwktonetmail`)** | **no** |

`qwktonetmail()` does acquire the header lock eventually, but only indirectly, when `smb_addmsghdr()` takes it internally about 50 lines later. By then the data blocks have already been chosen and written.

## Mechanism

`smb_allocdat()` scans the `.sda` for a run of consecutive free blocks, then seeks back and writes the reference counts. The window between "found a free run" and "marked those blocks in use" spans the remainder of the scan. Two writers allocating concurrently, with at least one on the unlocked path, can both settle on the same run. The result is two message headers whose data offsets overlap, which `chksmb` reports as duplicated or corrupt data records.

Header allocation is not affected: it always happens inside `smb_addmsghdr()`, under the lock that function takes. So the expected signature is corruption of **data** records specifically.

This path runs on every QWK network mail import, so on a system carrying a QWK network it executes routinely.

## Why it is rarely seen today

The race is not specific to self-packing storage, but the window size is. Measured on a real 8,800-message mail base:

| allocator | base state | window |
|---|---|---|
| `smb_allocdat` (self-packing) | typical, some messages expired | 0.003 ms |
| `smb_allocdat` (self-packing) | freshly packed, no free blocks | 33.8 ms |
| `smb_fallocdat` (fast allocation) | any | 0.002 ms |

`smb_fallocdat()` seeks to EOF and writes the reference counts immediately, so its window is roughly three orders of magnitude narrower in the normal case. Switching a mail base from self-packing to fast allocation therefore makes this class of error appear to vanish without fixing anything, which matches operator reports of occasional `chksmb` data-record errors that stopped after such a switch.

## Fix

Take `smb_locksmbhdr()` before the allocation and let `smb_addmsghdr()` release it, as the other callers do. `smb_new_msghdr()` honors an already-held lock via `smb->smbhdr_locked`, so no other change is needed; the two error paths between the lock and `smb_addmsghdr()` need an `smb_unlocksmbhdr()`.

Related: #1172 (base-lock weaknesses), #1170 (unchecked allocation failures).

— *Authored by Claude (Claude Code), on behalf of @rswindell*

---
￭ Synchronet ￭ Vertrauen ￭ Home of Synchronet ￭ [vert/cvs/bbs].synchro.net