broken error check in cups_write - can cause hang/crash

[michaelrsweet]

Version: 1.2.2
CUPS.org User: simonkelley

I found this whilst trying to diagnose a cupsd which would go into a 100% CPU hang after almost any activity. Running cupsd under strace showed an endless series if write() system calls, each one returning -1 with ernno as No Space Left on Device (that's an easy workaround). Each write call has a buffer address one less then the previous one, and a requested size one greater.

Pretty much all write calls in cups seem to be wrapped by cups_write() in file.c, which, simplified, looks like this.

size_t total, count

total = 0;
while (bytes > 0)
{
count = write(fp->fd, buf, bytes);

if (count < 0)
{
  if (errno == EAGAIN || errno == EINTR)
    continue;
  else
    return (-1);
}
bytes -= count;
total += count;
buf   += count;

}

The problem is that count is declared as size_t, which is unsigned, so the if (count < 0) test never succeeds, and when write returns -1 the code loops forever, decrementing buf and incrementing total. Presumably buf will eventually go out of bounds and the code will crash.

The fix is just to change the type of count and total to be ssize_t.

This bug exists in at least version2 1.2.2 (which I was using) and 1.3svn, downloaded today.

[michaelrsweet]

CUPS.org User: mike

Fixed in Subversion repository.

[michaelrsweet]

"str1996.patch":

Index: file.c

--- file.c (revision 5989)
+++ file.c (working copy)
@@ -377,7 +377,7 @@
int /* O - 0 on success, -1 on error /
cupsFileFlush(cups_file_t *fp) / I - CUPS file */
{

• size_t bytes; /* Bytes to write */
• ssize_t bytes; /* Bytes to write */

DEBUG_printf(("cupsFileFlush(fp=%p)\n", fp));
@@ -991,7 +991,7 @@
...) /* I - Additional args as necessary /
{
va_list ap; / Argument list */

• size_t bytes; /* Formatted size */
• ssize_t bytes; /* Formatted size /
  char buf[8192]; / Formatted text */

@@ -1090,7 +1090,7 @@
cupsFilePuts(cups_file_t fp, / I - CUPS file /
const char *s) / I - String to write */
{

• size_t bytes; /* Bytes to write */
• ssize_t bytes; /* Bytes to write */

/*
@@ -1149,8 +1149,8 @@
char buf, / O - Buffer /
size_t bytes) / I - Number of bytes to read */
{

• size_t total, /* Total bytes read */

•   count;          /\* Bytes read */
  

• size_t total; /* Total bytes read */
• ssize_t count; /* Bytes read */

DEBUG_printf(("cupsFileRead(fp=%p, buf=%p, bytes=%ld)\n", fp, buf,
@@ -1274,7 +1274,7 @@
cupsFileSeek(cups_file_t fp, / I - CUPS file /
off_t pos) / I - Position in file */
{

• size_t bytes; /* Number bytes in buffer */
• ssize_t bytes; /* Number bytes in buffer */

DEBUG_printf(("cupsFileSeek(fp=%p, pos=" CUPS_LLFMT ")\n", fp, pos));
@@ -2030,8 +2030,8 @@
const char buf, / I - Buffer /
size_t bytes) / I - Number bytes */
{

• size_t total, /* Total bytes written */

•   count;          /\* Count this time */
  

• size_t total; /* Total bytes written */
• ssize_t count; /* Count this time */

DEBUG_printf(("cups_write(fp=%p, buf=%p, bytes=%ld)\n", fp, buf,