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