asynInterposeCom: reject a bad ixon option value instead of sending it#234
Merged
MarkRivers merged 1 commit intoJul 16, 2026
Merged
Conversation
setOption("ixon", val) writes "Bad option value" into pasynUser->errorMessage
when val is neither "n" nor "y", but the else branch fell through instead of
returning. xBuf is a plain uninitialized local (char xBuf[5]) of which only
xBuf[0] is set (CPO_SET_CONTROL), so sbComPortOption() then transmitted
whatever happened to be on the stack in xBuf[1] as the telnet SET-CONTROL
value, and setOption() returned that call's status -- telling the caller the
option had been set successfully.
The value space of SET-CONTROL is not limited to flow control:
CPO_CONTROL_BREAK_ON is 5, so a stack byte of 5 asserts a BREAK on the
physical serial line of the attached instrument.
The two sibling branches already do this: parity ("Invalid parity selection")
and crtscts ("Bad value"), the structurally identical branch immediately
above, both end with return asynError. Only ixon omitted it.
Verified with a test harness that drives the real interpose with a fake
RFC 2217 server underneath it and inspects the bytes reaching the socket.
setOption("break","on") is used first as a legitimate call that leaves
CPO_CONTROL_BREAK_ON in xBuf[1]; being the same function at the same call
depth, the following setOption("ixon","1") reuses that stack slot:
before:
setOption("break","on") -> asynSuccess ; SET-CONTROL byte on wire = 5
setOption("ixon","1") -> asynSuccess ; SET-CONTROL byte on wire = 5
errorMessage = "Bad option value"
(1 frame transmitted for a rejected value)
after:
setOption("ixon","1") -> asynError ; nothing transmitted
errorMessage = "Bad option value"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In
setOption, theixonbranch'selseclause writes "Bad option value" intoerrorMessagebut is missingreturn asynError;, so execution falls through tosbComPortOption(…, xBuf, 2, …)and transmitsxBuf[1]— an uninitialized stack byte — as the value of a telnet SET-CONTROL command. OnlyxBuf[0]is assigned on that path.The two sibling branches either side of it (
parity,crtscts) both end theirelsewithreturn asynError;; onlyixondrops it. The SET-CONTROL value space includesCPO_CONTROL_BREAK_ON = 5, so a stack byte that happens to be 5 asserts a physical BREAK on the serial line to the attached instrument — and becausesetOptionends withreturn status;, the caller is told the option was set successfully.Add the missing
return asynError;. Verified with a real interpose over a fake RFC-2217 server:setOption("break","on")first to leave 5 in that stack slot, thensetOption("ixon","1")— before, returns asynSuccess and transmits wire byte 5 (BREAK-ON); after, returns asynError and transmits nothing.