Skip to content

journal_restart: seek before writing to the restarted journal - #579

Merged
millert merged 1 commit into
sudo-project:mainfrom
iefa-m:journal-restart-seek
Oct 23, 2026
Merged

millert merged 1 commit into
sudo-project:mainfrom
iefa-m:journal-restart-seek

Conversation

@iefa-m

@iefa-m iefa-m commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

journal_restart() reopens an incoming relay journal with fdopen(fd, "r+") and journal_seek() reads it with fread() up to the client's resume point. The next journal_write() then calls fwrite() on the same stream with no fseek() in between, which C leaves undefined. glibc happens to reposition, but BSD and macOS stdio drop the read buffer and write at the fd offset left by read-ahead: EOF for a small journal, or a block boundary in the middle of a message for a larger one. So with store_first = true, a client that reconnects with a RestartMessage whose resume point is before the end of the journal gets its resent messages written in the wrong place.

Repro on macOS with a store_first relay in front of a second sudo_logsrvd: sudo_sendlog -s 3,0 a 400-record session, then sudo_sendlog -i <log_id> -r 1,0. The client reports the log as transmitted, but the relay fails replaying the journal with "client message too large: 2021161080" (0x78787878, payload bytes read as a length) and nothing reaches the upstream server. With the fix the upstream ttyout matches the original byte for byte.

Seek to the current position once the resume point is found, the same way store_restart_local() already does before writing to restarted I/O log files. make check passes.

journal_seek() reads the journal with fread() up to the resume point
and journal_write() then appends with fwrite() on the same "r+" stream.
Without an intervening fseek() the write position is undefined; BSD
and macOS stdio write at the underlying file offset left by read-ahead,
corrupting the journal so it can no longer be relayed.

@millert millert left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I don't think this was tested on non-Linux systems.

@millert
millert merged commit c47546f into sudo-project:main Oct 23, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants