Adjust X11 display search range to support larger x11_display_offset values#559
Closed
Romk-a wants to merge 1 commit intoopenssh:masterfrom
Closed
Adjust X11 display search range to support larger x11_display_offset values#559Romk-a wants to merge 1 commit intoopenssh:masterfrom
Romk-a wants to merge 1 commit intoopenssh:masterfrom
Conversation
Contributor
|
Why do you want this? X11 ports usually are in this range |
Author
|
@djmdjm, I have a large server with virtualization deployed, and sometimes it happens that the ports overlap, which is why I have problems running the hypervisor and virtual machines. And I want to shift the ssh connection with x11 forwarding to about the 12000 port range. |
Author
|
@djmdjm, May I provide the information you are interested in for approval of my PR? |
Contributor
|
Looks reasonable to me. @djmdjm ok? |
Contributor
|
ok to commit @daztucker |
Contributor
|
Committed upstream and synced back as a356d97. Thanks. |
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.
Description:
This pull request modifies the X11 display search logic in x11_create_display_inet to allow the x11_display_offset parameter to exceed 1000, addressing a limitation in the current implementation.
Problem
Currently, setting x11_display_offset to a value greater than 1000 prevents the function from entering the for loop, as the condition display_number < MAX_DISPLAYS (where MAX_DISPLAYS is 1000) fails immediately. Execution then proceeds to the check if (display_number >= MAX_DISPLAYS), resulting in an error: "Failed to allocate internet-domain X11 display socket." This behavior is incorrect because it unnecessarily restricts the usable range of x11_display_offset, limiting the port range to 6000–7000.
Solution
The changes in this PR adjust the loop condition from display_number < MAX_DISPLAYS to display_number < x11_display_offset + MAX_DISPLAYS, and update the subsequent check accordingly. This ensures the loop iterates over a range of MAX_DISPLAYS (1000) displays starting from x11_display_offset, rather than being capped at a fixed maximum of 1000. As a result, users can specify a higher x11_display_offset to select a more convenient port range (e.g., beyond 7000).
Safety
The modification is safe because the existing check x11_display_offset > UINT16_MAX - X11_BASE_PORT - MAX_DISPLAYS already enforces an upper bound. For X11_BASE_PORT = 6000 and MAX_DISPLAYS = 1000, this limits x11_display_offset to 58535, ensuring the maximum port (6000 + 58535 + 999 = 65534) stays within the valid range of 65535. Thus, these changes align with the existing safety constraints while improving flexibility.
Benefits
This adjustment allows users to configure x11_display_offset more freely, enabling X11 forwarding on a broader and more customizable range of ports, rather than being restricted to 6000–7000.
Let me know if further clarification or adjustments are needed!