Board: [fsxNet] Retro Computing Subject: SMB data refcounts: per-field free leaks From: Joe Stollings (21:2/137) Date: Tue Oct 06, 2026 18:25 RS> open https://gitlab.synchro.net/main/sbbs/-/work_items/1253 RS> RS> ## Summary RS> RS> `smb_freemsg_dfields()` and `smb_incmsg_dfields()` adjust data-block reference RS> counts one data field at a time, while every allocation (and `smbutil pack`) RS> accounts for a message's data as one contiguous span. For any message whose RS> body ends partway through a block and is followed by a tail, the two RS> accountings disagree: RS> RS> 1. **Leak.** The block the tail spills into is never decremented, so it is RS> never freed. RS> 2. **Premature free after a pack.** Once `smbutil pack` has re-referenced RS> shared RS> data, deleting one copy frees a block that other copies still use. The next RS> allocation can then hand that block to a new message and overwrite it. RS> RS> The per-field loops date from e296fec9f1 (roots-7-pitch, 2003-08-20). The RS> recent in-place file-record update, 3a687044d3 (item-11-focal, 2026-09-21), RS> frees old text through the same function, so it now exercises the leak on RS> every auxdata rewrite. RS> RS> ## Mechanism RS> RS> Data fields are packed contiguously: `smb_dfield()` sets each field's offset to RS> the sum of the preceding lengths, and `smb_addmsg()` allocates once for the RS> total. Freeing and referencing then go field by field RS> (`smballoc.c`, `smb_freemsg_dfields()` / `smb_incmsg_dfields()`), each call RS> starting at `floor(offset / SDT_BLOCK_LEN)` and covering RS> `smb_datblocks(length)` blocks. RS> RS> Take a 200-byte body and 100-byte tail. With the xlat words the fields are RS> 202 and 102 bytes, 304 in total, so 2 blocks are allocated. RS> RS> | field | offset | length | blocks adjusted | RS> |---|---|---|---| RS> | body | 0 | 202 | block 0 | RS> | tail | 202 | 102 | block 0 (starts there; length alone rounds to 1) | RS> RS> Block 0 is adjusted twice, and block 1 is never adjusted, even though the RS> tail's bytes 256-303 live in it. RS> RS> With per-field referencing on both sides, the double count is harmless: the RS> shared block gains 2 per extra reference and loses 2 per delete, so it only RS> overshoots on the last delete, where the clamp in `smb_freemsgdat()` absorbs RS> it. Only the leak remains. RS> RS> `smbutil pack` breaks that symmetry. When a second header points at the same RS> data it calls `smb_incmsgdat(.., smb_getmsgdatlen(&msg), 1)`, which references RS> the whole span once, so every block ends up at N for N copies. Each later RS> delete still takes 2 from the shared block, so with two copies the **first** RS> delete frees data the second copy still uses. RS> RS> ## Reproducer RS> RS> [smb_dfield_refs.c](/uploads/5a827dc451605d9e3fd3a7d2a5d19a4b/smb_dfield_refs.c RS> RS> links against the built static libraries (build line in its header) and RS> prints the `.sda` reference counts: RS> RS> ``` RS> 1. One copy: the block the tail spills into is never freed RS> stored block0=1 block1=1 RS> delete copy 1 of 1 block0=0 block1=1 RS> RS> 2. Three copies, referenced per field (as mail delivery does) RS> stored, 3 copies block0=5 block1=1 RS> delete copy 1 of 3 block0=3 block1=1 RS> delete copy 2 of 3 block0=1 block1=1 RS> delete copy 3 of 3 block0=0 block1=1 RS> RS> 3. Three copies, re-referenced by span (as smbutil pack does) RS> stored, 3 copies block0=3 block1=3 RS> delete copy 1 of 3 block0=1 block1=3 RS> delete copy 2 of 3 block0=0 block1=3 RS> delete copy 3 of 3 block0=0 block1=3 RS> ``` RS> RS> Case 3 shows `block0=0` while one copy still references it. RS> RS> ## Exposure RS> RS> The premature free needs shared data with a body and a tail. That is ordinary RS> inbound mail: RS> RS> * `savemsg()` stores text through `smb_addmsg(.., findsig(msgbuf))`, and RS> `findsig()` splits off everything after the standard `\n-- \r\n` signature RS> delimiter as the tail. RS> * The mail server stores a message once and shares it across local RS> recipients with `smb_incmsg_dfields()` (`mailsrvr.cpp`, `rcpt_count > 1`). RS> Bulk mail, multi-recipient netmail, forwarding and JavaScript RS> `MsgBase.save_msg()` with a recipient list do the same. RS> RS> So: mail to two or more local users with a signature, a pack of the mail base, RS> then one recipient deleting their copy. RS> RS> The mechanism is demonstrated above against the library. Corruption has RS> **not** been observed in a real message base. RS> RS> Neither problem is visible to `chksmb`: it verifies that the blocks a header RS> uses are allocated, not that every allocated block is used. Each pack RS> reclaims the leaked blocks, which is likely why the leak has gone unnoticed. RS> RS> ## Fixing it needs a migration, not just a code change RS> RS> On-disk reference counts are currently a mix: span-based for shared data RS> packed since it was written, per-field for data that has not been. Two RS> rebuild tools also disagree: `smbutil pack` references by span, while RS> `fixsmb` (`fixsmb.c`) re-references with the per-field RS> `smb_incmsg_dfields()`. RS> RS> Changing only the free to use the span would fix packed data and break RS> unpacked shared data the same way in reverse: its spill block holds 1 for N RS> references and would reach zero after the first delete. RS> RS> A fix probably needs to: RS> RS> 1. account by span in both `smb_freemsg_dfields()` and `smb_incmsg_dfields()` RS> (for example with `smb_getmsgdatlen()`, matching the allocation); RS> 2. make `fixsmb` rebuild the same way `smbutil pack` does; and RS> 3. normalize existing bases, by pack or fixsmb, when the fix is deployed. RS> RS> Related: #1252 (a separate `editmsg()` ordering problem in the same free path). RS> RS> -- *Authored by Claude (Claude Code), on behalf of @rswindell* RS> --- SBBSecho 3.37-Linux RS> * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705) --- TriToss (tm) Professional 11.0 - (Unregistered) * Origin: Camelot BBS - Milton, Wv (21:2/137.0) .