unix: guard fd-touching port methods against use after Close#223
Open
ultramcu wants to merge 1 commit into
Open
unix: guard fd-touching port methods against use after Close#223ultramcu wants to merge 1 commit into
ultramcu wants to merge 1 commit into
Conversation
Only Read() checked whether the port was still open before touching the file descriptor. Write, Break, SetMode, SetDTR, SetRTS, GetModemStatusBits, Drain, ResetInputBuffer and ResetOutputBuffer issued their read/write/ioctl directly on port.handle. After Close() the fd is closed and its number may be reassigned by the OS to an unrelated file, so a late or concurrent call could operate on the wrong fd (silent data corruption) or return a confusing EBADF instead of a clear "port closed" error. Related to (but distinct from) bugst#150. Add a small withGuard helper mirroring the existing Read() guard: it takes the close-lock read side and verifies the port is still open before running the operation, returning PortError{PortClosed} otherwise. Every fd-touching method is wrapped with it (Write keeps the guard inline because it returns (int, error)). SetReadTimeout is intentionally left unguarded as it only mutates an in-memory field. serial_postclose_linux_test.go opens a pseudo-terminal (no real serial hardware needed; skips cleanly if /dev/ptmx is unavailable), closes the port, then asserts every method returns PortError{PortClosed}. Without the guards the test fails with "bad file descriptor" on all methods; with them it passes.
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
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.
Problem
On the unix backend, only
Read()checked whether the port was still open before touching the file descriptor.Write,Break,SetMode,SetDTR,SetRTS,GetModemStatusBits,Drain,ResetInputBufferandResetOutputBufferissued their read/write/ioctl directly onport.handle.After
Close()the fd is closed and its integer number may be reassigned by the OS to an entirely unrelated file, so a late or concurrent call could:Writesilently succeeds against a reused descriptor — silent data corruption), orEBADF("bad file descriptor") instead of a clear "port closed" error.Related to (but distinct from) #150 — that issue is about
Closenot unblocking a blockedWrite; this is about the missing open-check on the non-Readmethods.Fix
Add a small
withGuardhelper that mirrors the existingRead()guard: it takes the close-lock read side and verifiesport.opened == 1before running the operation, returningPortError{PortClosed}otherwise. Every fd-touching method is wrapped with it (Writekeeps the guard inline because it returns(int, error)).SetReadTimeoutis intentionally left unguarded as it only mutates an in-memory field.Test
serial_postclose_linux_test.goopens a pseudo-terminal (no real serial hardware needed; skips cleanly if/dev/ptmxis unavailable), closes the port, then asserts every method returnsPortError{PortClosed}. Verified fail-before/pass-after on Linux: without the guards the test fails with "bad file descriptor" on all methods; with them it passes. Builds andgo vetclean on linux/darwin/freebsd/openbsd/windows;gofmtclean;-raceclean.