[Bug 997217] Re: salsauthd maxes cpu

[Andreas Hasenack]

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
Server, which is subscribed to the bug report.
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