Ok, trusty isn't affected because the loop there has an exit clause:
+              ret = read(s, rbuf+rc, sizeof(rbuf)-rc);
+              if ( ret<0 ) {
+                 rc = ret;
+                 break;
+              } else {
+                 if (ret == 0) {
+                   loopc += 1;
+                 } else {
+                   loopc = 0;
+                 }
+                 if (loopc > sizeof(rbuf)) { // arbitrary chosen value
+                   break;
+                 }


That comes from trusty's patch named 0034-fix_dovecot_authentication.patch. So 
trusty does loop for a bit (sizeof(rbuf) is 1000), but won't get stuck. Someone 
added the loop counter as a safety net, but didn't change the "ret<0" check 
into "ret<=0" which would also have fixed this.

In precise, that same patch (0034) adds the loop, but *without* an exit
clause, hence this bug.

Xenial is interesting. Upstream at some point adopted the patch that
does *NOT* exit the loop, but the code is so similar that someone
decided the patch from the package was already applied and dropped it
from the package, reintroducing the bug.

To add to the confusion, in xenial that patch file was super slightly renamed 
from 0034_fix_dovecot_authentication.patch to 
0034-fix_dovecot_authentication.patch (can you spot what changed?) and got 
totally different contents:
--- cyrus-sasl2.orig/lib/checkpw.c
+++ cyrus-sasl2/lib/checkpw.c
@@ -587,16 +587,14 @@ static int read_wait(int fd, unsigned de
        /* Timeout. */
        errno = ETIMEDOUT;
        return -1;
-   case +1:
-       if (FD_ISSET(fd, &rfds)) {
-       /* Success, file descriptor is readable. */
-       return 0;
-       }
-       return -1;
    case -1:
        if (errno == EINTR || errno == EAGAIN)
        continue;
    default:
+       if (FD_ISSET(fd, &rfds)) {
+       /* Success, file descriptor is readable. */
+       return 0;
+       }
        /* Error catch-all. */
        return -1;
    }

>From bionic onwards, the upstream version has the loop with no loop
counter, but it checks read()'s result for <= 0, not just 0, so it's
fixed there.

Bottom line, only xenial is currently affected (and precise, but precise
is EOL).

** Changed in: cyrus-sasl2 (Ubuntu Trusty)
       Status: Triaged => Invalid

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/997217

Title:
  salsauthd maxes cpu

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/cyrus-sasl2/+bug/997217/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to