openssh bugs - Sep 2021 - [Bug 3345] New: sshd freeze when build without HAVE_PSELECT

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

            Bug ID: 3345
           Summary: sshd freeze when build without HAVE_PSELECT
           Product: Portable OpenSSH
           Version: 8.7p1
          Hardware: Itanium
                OS: Other
            Status: NEW
          Severity: normal
          Priority: P5
         Component: sshd
          Assignee: unassigned-bugs at mindrot.org
          Reporter: yaroslav.kuzmin at vmssoftware.com

In openbsd-compat/bsd-pselect.c file in pselect() function.

Line 196 incorrect call to select() function.
If unmasked !=0 , nfds need +1

   8   if (unmasked) {
   7     pselect_notify_setup();
   6     pselect_notify_prepare(readfds);
   5     nfds = MAX(nfds, notify_pipe[0]);
   4   }
   3 
   2   /* Unmask signals, call select then restore signal mask. */
   1   sigprocmask(SIG_SETMASK, mask, &osig);
 196   ret = select(nfds, readfds, writefds, exceptfds, tvp);
   1   saved_errno = errno;
   2   sigprocmask(SIG_SETMASK, &osig, NULL);
   3 
   4   if (unmasked)
   5     pselect_notify_done(readfds);

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dtucker at dtucker.net

--- Comment #1 from Darren Tucker <dtucker at dtucker.net> ---
(In reply to Yaroslav from comment #0)
> In openbsd-compat/bsd-pselect.c file in pselect() function.
> 
> Line 196 incorrect call to select() function.
> If unmasked !=0 , nfds need +1
For the common case nfds should already have the 1 added by the caller,
eg in sshd.c:

    /* Wait until a connection arrives or a child exits. */
    ret = pselect(maxfd+1, fdset, NULL, NULL, NULL, &osigset);

I could see this being a problem in the case where notify_pipe[0] is
higher than any descriptor in the fdsets, in which case I think the +1
should be added to notify_pipe[0]:

          nfds = MAX(nfds, notify_pipe[0]+1);

Would that solve the problem?  Also, out of curiosity, what platform is
this (you indicated "Other")?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |3339


Referenced Bugs:

https://bugzilla.mindrot.org/show_bug.cgi?id=3339
[Bug 3339] Tracking bug for openssh-8.8
-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

--- Comment #2 from Yaroslav <yaroslav.kuzmin at vmssoftware.com> ---
I added the following code and it solved the problem

200     nfds = MAX(nfds, notify_pipe[0]);
201     ++nfds;

I am porting OpenSSH to OpenVMS.

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

--- Comment #3 from Darren Tucker <dtucker at dtucker.net> ---
(In reply to Yaroslav from comment #2)
> I added the following code and it solved the problem
> 
> 200     nfds = MAX(nfds, notify_pipe[0]);
> 201     ++nfds;
As I described above I don't think that's correct.  In the case where
nfds was not increased in the MAX(), nfds is being incremented twice
which may cause other problems.

The code I suggested does not do that.  Does it also fix the problem?

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

--- Comment #4 from Yaroslav <yaroslav.kuzmin at vmssoftware.com> ---
(In reply to Darren Tucker from comment #3)
> (In reply to Yaroslav from comment #2)
> > I added the following code and it solved the problem
> > 
> > 200     nfds = MAX(nfds, notify_pipe[0]);
> > 201     ++nfds;
> 
> As I described above I don't think that's correct.  In the case
> where nfds was not increased in the MAX(), nfds is being incremented
> twice which may cause other problems.
> 
> The code I suggested does not do that.  Does it also fix the problem?
I tested on 5 connections everything works correctly.

But I agree that nfsd  contains information about the number of file
descriptors, and MAX  will check the maximum descriptor


  if (unmasked) {
    pselect_notify_setup();
    pselect_notify_prepare(readfds);
    --nfds;
    nfds = MAX(nfds, notify_pipe[0]);
    ++nfds;
 }

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

Darren Tucker <dtucker at dtucker.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|NEW                         |RESOLVED

--- Comment #5 from Darren Tucker <dtucker at dtucker.net> ---
Thanks, I have committed the fix and cherry-picked it into the 8.7
branch so it will be in the next release.

-- 
You are receiving this mail because:
You are watching the assignee of the bug.
You are watching someone on the CC list of the bug.

https://bugzilla.mindrot.org/show_bug.cgi?id=3345

Damien Miller <djm at mindrot.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |CLOSED

--- Comment #6 from Damien Miller <djm at mindrot.org> ---
closing bugs resolved before openssh-8.9

-- 
You are receiving this mail because:
You are watching someone on the CC list of the bug.
You are watching the assignee of the bug.