Skip to content

SerialOverLan session lifecycle: no BMC key, no cleanup on failed activation or deactivation, alternate-port session leak #130

Description

@bertysentry

Where: src/main/java/org/metricshub/ipmi/core/api/sol/SerialOverLan.java (constructors, resolveSession, close), src/main/java/org/metricshub/ipmi/core/connection/SessionManager.java (establishSession).

What happens:

  1. No BMC key. The constructors that open their own session (SerialOverLan(connector, host, [port,] user, password, selector)) go through SessionManager.establishSession(), which always calls openSession(handle, user, password, null). They cannot log in to a BMC configured with two-key logins (Kg). The session that resolveSession() opens on the SOL payload port has the same limitation.
  2. Nothing is cleaned up when the constructor fails. When the session opens but the payload cannot be activated (SOL disabled, no free payload instance), the constructor throws SOLException and leaves the session open and the connector running: the caller has no object to close.
  3. close() does not log out when deactivation fails. close() sends Deactivate Payload first; if it fails (error completion code or no reply), it throws IOException before closeSession() and tearDown(), so the BMC session stays allocated until it expires.
  4. Alternate-port session leak. When the BMC serves SOL on another UDP port, the host/password constructors open a first session on the configured port, then resolveSession() opens a second one on the payload port. close() only closes the second, so the first stays allocated on the BMC; opening and closing consoles in a loop can use up the BMC's session slots.

Evidence: by the code (found by Codex while reviewing #124); Serial over LAN was not exercised on the test BMCs.

Suggested fix: add a bmcKey parameter to the session-opening constructors and pass it through establishSession(); close the session (and the connector, for internal sessions) when activation fails; in close(), close the session(s) and tear down the connector in a finally, rethrowing the deactivation error; track and close every session the instance opened. Unit tests against a fake BMC, or at least for the cleanup paths.

Related: #123 (writeBytes chunking), #95 (establishSession tears down the whole connector).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions