• editmsg() frees a message's data blocks before allocating the replacem

    From Rob Swindell@1:103/705 to GitLab issue in main/sbbs on Mon Sep 21 21:36:58 2026
    open https://gitlab.synchro.net/main/sbbs/-/work_items/1252

    ## Summary

    `sbbs_t::editmsg()` (`src/sbbs3/writemsg.cpp`) frees the message's existing data blocks **before** it allocates and writes the replacement text. The message header is not updated until after the new text has been written, so between those two points the on-disk header references blocks that are already marked free in the allocation table.

    ## Detail

    The free happens at `writemsg.cpp:1743`, right after the data-allocation file is opened:

    ```c
    if (!(smb->status.attr & SMB_HYPERALLOC)) {
    if ((i = smb_open_da(smb)) != SMB_SUCCESS) {
    errormsg(WHERE, ERR_OPEN, smb->file, i, smb->last_error);
    return false;
    }
    if ((i = smb_freemsg_dfields(smb, msg, 1)) != SMB_SUCCESS)
    errormsg(WHERE, ERR_WRITE, smb->file, i, smb->last_error);
    }
    ```

    The replacement is allocated about 20 lines later, and the header is written last, by `smb_putmsghdr()` at the end of the function.

    Every failure path between those two points returns `false` without restoring anything:

    | line | failure |
    |---|---|
    | 1770 | `smb_allocdat()` / `smb_fallocdat()` returned an error |
    | 1777 | the edited temp file could not be reopened |
    | 1789 | writing the xlat terminator failed |
    | 1802 | writing a block of message text failed |

    In each case the old blocks stay free and the header still names them. A crash anywhere in the same window has the same result.

    Two further notes on the same code:

    * A `smb_freemsg_dfields()` failure is only logged; the function continues and
    allocates anyway.
    * The in-memory `msg->dfield[0].length` is overwritten with the new length
    before the allocation, so the record is inconsistent even in the paths that
    return early.

    ## Consequence

    This is not only "the edited text may be lost". Once those blocks are free, another writer can be given them, and the message then renders as unrelated text while `chksmb` still reports the base as OK, because the allocation table and the header agree from its point of view.

    ## Suggested fix

    Use the order the file base now uses in `smb_updatefile()` (smblib): write the replacement into newly allocated blocks, commit the header, and free the old blocks last. A failure part way through then leaks blocks, which `chksmb` reports and `fixsmb` reclaims, instead of leaving a live header pointing at free space.

    Related: #1241, which covered the header lock around data allocation across the same set of call sites.

    -- *Authored by Claude (Claude Code), on behalf of @rswindell*
    --- SBBSecho 3.37-Linux
    * Origin: Vertrauen - [vert/cvs/bbs].synchro.net (1:103/705)