http://bugzilla.mindrot.org/show_bug.cgi?id=1090
Summary: Increase MAX_SESSIONS?
Product: Portable OpenSSH
Version: -current
Platform: All
OS/Version: All
Status: NEW
Severity: enhancement
Priority: P2
Component: sshd
AssignedTo: bitbucket at mindrot.org
ReportedBy: cjwatson at debian.org
Now that we have the excellent new session multiplexing facility, I have various
users (including myself) who'd like to use it to make their use of ssh more
efficient in an environment where we have to ssh ProxyCommand through a bastion
host in order to get to other systems in a datacentre. The obvious
implementation of this is to set 'ControlMaster auto' on the
configuration
stanza for the bastion host, which works well up to a point.
However, each sshd only supports 10 sessions (MAX_SESSIONS), which makes this
rather inconvenient. I realise that the functions in session.c do linear
searches through the sessions array etc., but 10 seems a bit small nowadays with
good session multiplexing support. Could this be raised (64 or so?), and perhaps
made into a configuration parameter as well?
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090 ------- Additional Comments From lamont.jones at hp.com 2005-09-24 01:05 ------- Created an attachment (id=963) --> (http://bugzilla.mindrot.org/attachment.cgi?id=963&action=view) proposed patch to add MaxSessions config parameter This patch makes MaxSessions a config parameter, default of 64, and grows the table one entry at a time, to minimize memory usage. If MaxSessions is set to 0, then there is no limit. It does not appear (at first glance) that any of the session_* functions are performance criticial, so I didn't work on removing the linear search. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090
lamont.jones at hp.com changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |lamont.jones at hp.com
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090
t8m at centrum.cz changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |t8m at centrum.cz
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090 ------- Additional Comments From dtucker at zip.com.au 2005-10-03 13:37 ------- (From update of attachment 963) Personally, I have no objection to this in principle. Some comments on the patch:>+ num_sessions=1;I'd be tempted to allocate them in blocks (eg of 8) to avoid excessive reallocs but that's not critical.>+ sessions=calloc(num_sessions,sizeof(sessions[0]));Since sessions is initialized as NULL, could you use xrealloc (which is guaranteed to be happy with xrealloc(NULL, size)) rather than calloc/realloc to simplify this?>+ Session *n=realloc(sessions,++num_sessions*sizeof(Session)); >+ if (!n) >+ return NULL;If the realloc fails you will have already incremented num_sessions, so the next new channel will overflow the array bounds. Along similar lines: it's unlikely but what's to prevent num_sessions exceeding INT_MAX and wrapping when MaxSessions=0 and you have gobs of memory?>+ sessions=n; >+ s=sessions+num_sessions-1;The rest of the function uses array syntax, it's probably easier to follow if you stick to that. "s = sessions[i]" would be right, no?>+.It Cm MaxSessionShould be "MaxSessions"?. There were a few style nits too but they're not a big deal. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090 ------- Additional Comments From dtucker at zip.com.au 2005-10-03 13:49 ------- (In reply to comment #2)> Since sessions is initialized as NULL, could you use xrealloc (which is > guaranteed to be happy with xrealloc(NULL, size)) rather than calloc/realloc > to simplify this?OK, that's a bad idea. xrealloc will fatal() if the allocation fails, realloc allows graceful failure by refusing to allocate the new session. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090 ------- Additional Comments From dtucker at zip.com.au 2005-10-03 13:52 ------- (From update of attachment 963)>+ num_sessions=1; >+ sessions=calloc(num_sessions,sizeof(sessions[0]));Also: it doesn't check if calloc failed. ------- You are receiving this mail because: ------- You are the assignee for the bug, or are watching the assignee.
http://bugzilla.mindrot.org/show_bug.cgi?id=1090
dtucker at zip.com.au changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #963 is|0 |1
obsolete| |
------- Additional Comments From dtucker at zip.com.au 2005-10-03 14:39 -------
Created an attachment (id=979)
--> (http://bugzilla.mindrot.org/attachment.cgi?id=979&action=view)
rework patch based on comments #2 and #3.
One other thing: MaxSessions=0 could have a legitimate use (eg to permit port
forwarding only and no shells) so that might not be the best choice to mean
"unlimited".
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.