michaelrsweet commented 17 years ago

Version: 1.4-feature CUPS.org User: mcknight

The problem showed up specifically in cups/ipp.c in both ippWriteIO() and ippReadIO(). There, cups creates a buffer of 32 K on the stack. This sounds little in a world with a few gigs of RAM but there are stack limits and some operating systems set them to quite conservative values. Worse even with pthreads where every thread has another (normally significantly smaller) stack limit.

We noticed that on a gnome application on OpenBSD that was linked against the cups library. Clicking the print button made the application crash.

The pthreads stack limits were set to 64k per thread and with quite some stack backtrace of previous function calls and all their local variables, cups touched the limit requesting another 32k and the process got killed by the operating system.

Especially a library does not know in which circumstances it is executed later and should be conservative about its expectations regarding system ressources.

I append the patch that fixed that very problem but this bug report is rather meant as a general advice to not put large data structures on the stack.

michaelrsweet commented 17 years ago

CUPS.org User: mike

I'm re-flagging this as a request for enhancement. At this point, it is doubtful we will incorporate this change before CUPS 1.4, and it will certainly need some cleanup and serious testing.

michaelrsweet commented 17 years ago

CUPS.org User: mike

Considering for 1.4...

michaelrsweet commented 16 years ago

CUPS.org User: mike

Fixed in Subversion repository.

The actual implementation maintains a linked list of buffers so we don't allocate memory every time.

michaelrsweet commented 16 years ago

"patch-cups_ipp_c":

$OpenBSD$ --- cups/ipp.c.orig Mon Jan 3 20:29:45 2005 +++ cups/ipp.c Fri May 18 16:05:29 2007 @@ -856,6 +856,12 @@ ippReadFile(int fd, /* I - HTTP dat

/*

* We allocate the buffer on the heap. Some platforms have very low stack
* limits, especially when run under pthreads or similar.
/ +#define BUFSIZE 32786 / Data buffer size / + +/
- 'ippReadIO()' - Read data for an IPP request. */

@@ -867,11 +873,12 @@ ippReadIO(void src, / I - Data ipp_t ipp) / I - IPP data / { int n; / Length of data */

unsigned char buffer[32768], /* Data buffer */
unsigned char buffer, / Data buffer base pointer / *bufptr; / Pointer into buffer _/ ipp_attributet *attr; / Current attribute _/ ipp_tagt tag; / Current tag _/ ipp_valuet *value; / Current value */
ipp_state_t retval; /* Return value */

DEBUG_printf(("ippReadIO(%p, %p, %d, %p, %p)\n", src, cb, blocking, @@ -880,6 +887,10 @@ ippReadIO(void src, / I - Data if (src == NULL || ipp == NULL) return (IPP_ERROR);
buffer = (unsigned char*) malloc(BUFSIZE);
if (!buffer)
return (IPP_ERROR); + switch (ipp->state) { case IPP_IDLE : @@ -895,7 +906,8 @@ ippReadIO(void src, / I - Data if ((n = (*cb)(src, buffer, 8)) < 8) { DEBUG_printf(("ippReadIO: Unable to read header (%d bytes read)!\n", n));
return (n == 0 ? IPP_IDLE : IPP_ERROR);
retval = (n == 0 ? IPP_IDLE : IPP_ERROR);
```
goto cleanup_retval;
```
}

/* @@ -906,7 +918,7 @@ ippReadIO(void src, / I - Data { DEBUG_printf(("ippReadIO: version number (%d.%d) is bad.\n", buffer[0], buffer[1]));
return (IPP_ERROR);
```
goto cleanup_error;
```
}

/ @@ -979,15 +991,15 @@ ippReadIO(void src, / I - Data if ((cb)(src, buffer, 2) < 2) { DEBUG_puts("ippReadIO: unable to read name length!");
return (IPP_ERROR);

goto cleanup_error;

}

 n = (buffer[0] << 8) | buffer[1];

```
   if (n > (sizeof(buffer) - 1))
```
if (n > (BUFSIZE - 1)) { DEBUG_printf(("ippReadIO: bad name length %d!\n", n));
return (IPP_ERROR);

goto cleanup_error;

}

 DEBUG_printf(("ippReadIO: name length = %d\n", n));

@@ -999,7 +1011,7 @@ ippReadIO(void src, / I - Data */

   if (ipp->current == NULL)

return (IPP_ERROR);

  goto cleanup_error;

   attr = ipp->current;

@@ -1023,11 +1035,11 @@ ippReadIO(void src, / I - Data

      if (tag != IPP_TAG_STRING &&
          (tag < IPP_TAG_TEXTLANG || tag > IPP_TAG_MIMETYPE))

return (IPP_ERROR);
goto cleanup_error; } else if (attr->value_tag != tag && tag != IPP_TAG_END_COLLECTION)
return (IPP_ERROR);
```
  goto cleanup_error;

  /*
* Finally, reallocate the attribute array as needed...
```
@@ -1047,7 +1059,7 @@ ippReadIO(void src, / I - Data if ((temp = realloc(attr, sizeof(ipp_attribute_t) + (attr->num_values + IPP_MAX_VALUES - 1) * sizeof(ipp_value_t))) == NULL)
return (IPP_ERROR);
```
    goto cleanup_error;

    /*
* Reset pointers in the list...
```
@@ -1072,7 +1084,7 @@ ippReadIO(void src, / I - Data if (n) { DEBUG_puts("ippReadIO: member name not empty!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

attr = ipp->current = _ipp_add_attr(ipp, IPP_MAX_VALUES);
```
@@ -1090,7 +1102,7 @@ ippReadIO(void src, / I - Data if ((*cb)(src, buffer, n) < n) { DEBUG_puts("ippReadIO: unable to read name!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

buffer[n] = '\0';
```
@@ -1109,7 +1121,7 @@ ippReadIO(void src, / I - Data if ((*cb)(src, buffer, 2) < 2) { DEBUG_puts("ippReadIO: unable to read value length!");
return (IPP_ERROR);
```
goto cleanup_error;
```
}

n = (buffer[0] << 8) | buffer[1]; @@ -1122,7 +1134,7 @@ ippReadIO(void src, / I - Data if ((*cb)(src, buffer, 4) < 4) { DEBUG_puts("ippReadIO: Unable to read integer value!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

n = (((((buffer[0] << 8) | buffer[1]) << 8) | buffer[2]) << 8) |
```
@@ -1134,7 +1146,7 @@ ippReadIO(void src, / I - Data if ((*cb)(src, buffer, 1) < 1) { DEBUG_puts("ippReadIO: Unable to read boolean value!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

       value->boolean = buffer[0];
```
@@ -1153,7 +1165,7 @@ ippReadIO(void src, / I - Data if ((cb)(src, (ipp_uchar_t )value->string.text, n) < n) { DEBUG_puts("ippReadIO: Unable to read string value!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

DEBUG_printf(("ippReadIO: value = \'%s\'\n",
```
@@ -1163,14 +1175,14 @@ ippReadIO(void src, / I - Data if ((*cb)(src, value->date, 11) < 11) { DEBUG_puts("ippReadIO: Unable to date integer value!");
return (IPP_ERROR);
goto cleanup_error; } break; case IPP_TAG_RESOLUTION : if ((*cb)(src, buffer, 9) < 9) { DEBUG_puts("ippReadIO: Unable to read resolution value!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

       value->resolution.xres =
```
@@ -1186,7 +1198,7 @@ ippReadIO(void src, / I - Data if ((*cb)(src, buffer, 8) < 8) { DEBUG_puts("ippReadIO: Unable to read range value!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

       value->range.lower =
```
@@ -1198,16 +1210,16 @@ ippReadIO(void src, / I - Data break; case IPP_TAG_TEXTLANG : case IPP_TAG_NAMELANG :
if (n > sizeof(buffer) || n < 4)
if (n > BUFSIZE || n < 4) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));
return (IPP_ERROR);

  goto cleanup_error;
}

  if ((*cb)(src, buffer, n) < n)
{
    DEBUG_puts("ippReadIO: Unable to read string w/language value!");

return (IPP_ERROR);
```
  goto cleanup_error;
}

       bufptr = buffer;
```
@@ -1248,13 +1260,13 @@ ippReadIO(void src, / I - Data if (n > 0) { DEBUG_puts("ippReadIO: begCollection tag with value length > 0!");
return (IPP_ERROR);

  goto cleanup_error;
}

if (ippReadIO(src, cb, 1, ipp, value->collection) == IPP_ERROR)
{
    DEBUG_puts("ippReadIO: Unable to read collection value!");

return (IPP_ERROR);
goto cleanup_error; } break;

@@ -1262,12 +1274,13 @@ ippReadIO(void src, / I - Data if (n > 0) { DEBUG_puts("ippReadIO: endCollection tag with value length > 0!");

return (IPP_ERROR);

  goto cleanup_error;
}

  DEBUG_puts("ippReadIO: endCollection tag...");

```
return (ipp->state = IPP_DATA);
```
retval = ipp->state = IPP_DATA;
```
goto cleanup_retval;

   case IPP_TAG_MEMBERNAME :
 /*
```
@@ -1280,7 +1293,7 @@ ippReadIO(void src, / I - Data if ((cb)(src, (ipp_uchar_t )attr->name, n) < n) { DEBUG_puts("ippReadIO: Unable to read member name value!");
return (IPP_ERROR);
```
  goto cleanup_error;
}

DEBUG_printf(("ippReadIO: member name = \"%s\"\n", attr->name));
```
@@ -1294,7 +1307,7 @@ ippReadIO(void src, / I - Data if ((*cb)(src, value->unknown.data, n) < n) { DEBUG_puts("ippReadIO: Unable to read unsupported value!");
return (IPP_ERROR);
```
    goto cleanup_error;
}
}
else
```
@@ -1320,7 +1333,16 @@ ippReadIO(void src, / I - Data break; /* anti-compiler-warning-code */ }
free(buffer); return (ipp->state); + +cleanup_error:
free(buffer);
return (IPP_ERROR); + +cleanup_retval:
free(buffer);
return retval; }

@@ -1406,6 +1428,12 @@ ippWriteFile(int fd, /* I - HTTP da

/*

* We allocate the buffer on the heap. Some platforms have very low stack
* limits, especially when run under pthreads or similar.
/ +#define BUFSIZE 32786 / Data buffer size / + +/
- 'ippWriteIO()' - Write data for an IPP request. */

@@ -1418,7 +1446,7 @@ ippWriteIO(void dst, / I - Des { int i; /* Looping var / int n; / Length of data */

unsigned char buffer[32768], /* Data buffer */
unsigned char buffer, / Data buffer base pointer / *bufptr; / Pointer into buffer _/ ipp_attributet *attr; / Current attribute _/ ipp_valuet *value; / Current value / @@ -1430,6 +1458,10 @@ ippWriteIO(void *dst, / I - Des if (dst == NULL || ipp == NULL) return (IPP_ERROR);
buffer = (unsigned char*) malloc(BUFSIZE);
if (!buffer)
return (IPP_ERROR); + switch (ipp->state) { case IPP_IDLE : @@ -1461,7 +1493,7 @@ ippWriteIO(void dst, / I - Des if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP header...");
return (IPP_ERROR);
goto cleanup_error; } }

@@ -1530,8 +1562,8 @@ ippWriteIO(void dst, / I - Des * overflow the buffer... */

if ((n = strlen(attr->name)) > (sizeof(buffer) - 4))
return (IPP_ERROR);
if ((n = strlen(attr->name)) > (BUFSIZE - 4))
```
  goto cleanup_error;

  /*
* Write the value tag, name length, and name string...
```
@@ -1553,8 +1585,8 @@ ippWriteIO(void dst, / I - Des * overflow the buffer... */

     if ((n = strlen(attr->name)) > (sizeof(buffer) - 7))

return (IPP_ERROR);
if ((n = strlen(attr->name)) > (BUFSIZE - 7))
```
  goto cleanup_error;

  /*
* Write the member name tag, name length, name string, value tag,
```
@@ -1592,12 +1624,12 @@ ippWriteIO(void dst, / I - Des i < attr->num_values; i ++, value ++) {
if ((sizeof(buffer) - (bufptr - buffer)) < 9)
if ((BUFSIZE - (bufptr - buffer)) < 9) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -1636,12 +1668,12 @@ ippWriteIO(void dst, / I - Des i < attr->num_values; i ++, value ++) {
if ((sizeof(buffer) - (bufptr - buffer)) < 6)
if ((BUFSIZE - (bufptr - buffer)) < 6) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -1695,12 +1727,12 @@ ippWriteIO(void dst, / I - Des attr->value_tag)); DEBUG_printf(("ippWrite: writing name = 0, \'\'\n"));

             if ((sizeof(buffer) - (bufptr - buffer)) < 3)

if ((BUFSIZE - (bufptr - buffer)) < 3) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
        goto cleanup_error;
        }

    bufptr = buffer;
```
@@ -1716,18 +1748,18 @@ ippWriteIO(void dst, / I - Des else n = 0;

           if (n > (sizeof(buffer) - 2))

return (IPP_ERROR);
if (n > (BUFSIZE - 2))

    goto cleanup_error;

         DEBUG_printf(("ippWrite: writing string = %d, \'%s\'\n", n,
              value->string.text));

           if ((int)(sizeof(buffer) - (bufptr - buffer)) < (n + 2))

if ((int)(BUFSIZE - (bufptr - buffer)) < (n + 2)) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -1759,12 +1791,12 @@ ippWriteIO(void dst, / I - Des i < attr->num_values; i ++, value ++) {
if ((sizeof(buffer) - (bufptr - buffer)) < 16)
if ((BUFSIZE - (bufptr - buffer)) < 16) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -1802,12 +1834,12 @@ ippWriteIO(void dst, / I - Des i < attr->num_values; i ++, value ++) {
if ((sizeof(buffer) - (bufptr - buffer)) < 14)
if ((BUFSIZE - (bufptr - buffer)) < 14) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -1853,12 +1885,12 @@ ippWriteIO(void dst, / I - Des i < attr->num_values; i ++, value ++) {
if ((sizeof(buffer) - (bufptr - buffer)) < 13)
if ((BUFSIZE - (bufptr - buffer)) < 13) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -1910,12 +1942,12 @@ ippWriteIO(void dst, / I - Des * values with a zero-length name... */

             if ((sizeof(buffer) - (bufptr - buffer)) < 3)

if ((BUFSIZE - (bufptr - buffer)) < 3) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
        goto cleanup_error;
        }

    bufptr = buffer;
```
@@ -1944,15 +1976,15 @@ ippWriteIO(void dst, / I - Des if (value->string.text != NULL) n += strlen(value->string.text);

           if (n > (sizeof(buffer) - 2))

return (IPP_ERROR);
if (n > (BUFSIZE - 2))
```
    goto cleanup_error;
```

           if ((int)(sizeof(buffer) - (bufptr - buffer)) < (n + 2))

if ((int)(BUFSIZE - (bufptr - buffer)) < (n + 2)) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -2008,12 +2040,12 @@ ippWriteIO(void dst, / I - Des * value... */

           if ((sizeof(buffer) - (bufptr - buffer)) < 5)

if ((BUFSIZE - (bufptr - buffer)) < 5) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -2041,7 +2073,7 @@ ippWriteIO(void dst, / I - Des if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);

    goto cleanup_error;
    }

bufptr = buffer;

@@ -2053,7 +2085,7 @@ ippWriteIO(void dst, / I - Des value->collection->state = IPP_IDLE;

if (ippWriteIO(dst, cb, 1, ipp, value->collection) == IPP_ERROR)

return (IPP_ERROR);
goto cleanup_error; } break;

@@ -2069,12 +2101,12 @@ ippWriteIO(void dst, / I - Des * values with a zero-length name... */

if ((sizeof(buffer) - (bufptr - buffer)) < 3)
if ((BUFSIZE - (bufptr - buffer)) < 3) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);

    goto cleanup_error;
        }

    bufptr = buffer;

@@ -2094,15 +2126,15 @@ ippWriteIO(void dst, / I - Des

         n = value->unknown.length;

           if (n > (sizeof(buffer) - 2))

return (IPP_ERROR);
if (n > (BUFSIZE - 2))
```
    goto cleanup_error;
```

           if ((int)(sizeof(buffer) - (bufptr - buffer)) < (n + 2))

if ((int)(BUFSIZE - (bufptr - buffer)) < (n + 2)) { if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
      goto cleanup_error;
      }

  bufptr = buffer;
```
@@ -2129,7 +2161,7 @@ ippWriteIO(void dst, / I - Des if ((*cb)(dst, buffer, bufptr - buffer) < 0) { DEBUG_puts("ippWrite: Could not write IPP attribute...");
return (IPP_ERROR);
```
goto cleanup_error;
```
}
```
 DEBUG_printf(("ippWrite: wrote %d bytes\n", bufptr - buffer));
```
@@ -2167,7 +2199,7 @@ ippWriteIO(void dst, / I - Des if ((*cb)(dst, buffer, n) < 0) { DEBUG_puts("ippWrite: Could not write IPP end-tag...");
return (IPP_ERROR);
```
goto cleanup_error;
```
}

ipp->state = IPP_DATA; @@ -2181,7 +2213,12 @@ ippWriteIO(void dst, / I - Des break; /* anti-compiler-warning-code */ }
free(buffer); return (ipp->state); + +cleanup_error:
free(buffer);
return (IPP_ERROR); }

michaelrsweet commented 16 years ago

"str2388.patch":

Index: cups/ipp.c

--- cups/ipp.c (revision 7988) +++ cups/ipp.c (working copy) @@ -75,6 +75,8 @@

Local functions... */

+static unsigned char _ipp_buffer_get(void); +static void ipp_buffer_release(unsigned char b); static size_t ipp_length(ipp_t ipp, int collection); static ssize_t ipp_read_http(http_t http, ipp_uchar_t buffer, size_t length); @@ -1064,8 +1066,7 @@ ippt *ipp) / I - IPP data / { int n; / Length of data */

unsigned char buffer[IPP_MAX_LENGTH + 1],
/* Data buffer */
unsigned char buffer, / Data buffer _/ string[IPP_MAXNAME], / Small string buffer / *bufptr; / Pointer into buffer */ @@ -1082,6 +1083,12 @@ if (!src || !ipp) return (IPP_ERROR);
if ((buffer = ipp_buffer_get()) == NULL)
{
DEBUG_puts("ippReadIO: Unable to get read buffer!");
return (IPP_ERROR);
} + switch (ipp->state) { case IPP_IDLE : @@ -1097,6 +1104,7 @@ if ((*cb)(src, buffer, 8) < 8) { DEBUG_puts("ippReadIO: Unable to read header!");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1108,6 +1116,7 @@ { DEBUG_printf(("ippReadIO: version number (%d.%d) is bad.\n", buffer[0], buffer[1]));

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1144,7 +1153,11 @@ for (;;) { if ((*cb)(src, buffer, 1) < 1)

{
DEBUG_puts("ippReadIO: Callback returned EOF/error");
ipp_buffer_release(buffer); return (IPP_ERROR);
```
}
```
DEBUG_printf(("ippReadIO: ipp->current=%p, ipp->prev=%p\n", ipp->current, ipp->prev)); @@ -1194,14 +1207,16 @@ if ((*cb)(src, buffer, 2) < 2) { DEBUG_puts("ippReadIO: unable to read name length!");

ipp_buffer_release(buffer);
return (IPP_ERROR);

}

 n = (buffer[0] << 8) | buffer[1];

```
   if (n > (sizeof(buffer) - 1))
```
if (n >= IPP_BUF_SIZE) { DEBUG_printf(("ippReadIO: bad name length %d!\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1215,7 +1230,11 @@ */

         if (ipp->current == NULL)

{
DEBUG_puts("ippReadIO: Attribute without name and no current");
ipp_buffer_release(buffer); return (IPP_ERROR);
```
}

   attr      = ipp->current;
value_tag = (ipp_tag_t)(attr->value_tag & IPP_TAG_MASK);
```
@@ -1247,6 +1266,7 @@ DEBUG_printf(("ippReadIO: 1setOf value tag %x(%s) != %x(%s)\n", value_tag, ippTagString(value_tag), tag, ippTagString(tag)));
ipp_buffer_release(buffer); return (IPP_ERROR); } } @@ -1255,6 +1275,7 @@ DEBUG_printf(("ippReadIO: value tag %x(%s) != %x(%s)\n", value_tag, ippTagString(value_tag), tag, ippTagString(tag)));
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1279,7 +1300,11 @@ if ((temp = realloc(attr, sizeof(ipp_attribute_t) + (attr->num_values + IPP_MAX_VALUES - 1) * sizeof(ipp_value_t))) == NULL)

{
DEBUG_puts("ippReadIO: Unable to resize attribute");
ipp_buffer_release(buffer); return (IPP_ERROR);
```
  }

     if (temp != attr)
{
```
@@ -1305,6 +1330,7 @@ if (n) { DEBUG_puts("ippReadIO: member name not empty!");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1329,6 +1355,7 @@ if ((*cb)(src, buffer, n) < n) { DEBUG_puts("ippReadIO: unable to read name!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1340,6 +1367,7 @@ if ((attr = ipp->current = _ippAddAttr(ipp, 1)) == NULL) { DEBUG_puts("ippReadIO: unable to allocate attribute!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1362,6 +1390,7 @@ if ((*cb)(src, buffer, 2) < 2) { DEBUG_puts("ippReadIO: unable to read value length!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1375,12 +1404,14 @@ if (n != 4) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, buffer, 4) < 4)
{
    DEBUG_puts("ippReadIO: Unable to read integer value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1394,12 +1425,14 @@ if (n != 1) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, buffer, 1) < 1)
{
    DEBUG_puts("ippReadIO: Unable to read boolean value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1423,15 +1456,17 @@ case IPP_TAG_CHARSET : case IPP_TAG_LANGUAGE : case IPP_TAG_MIMETYPE :

if (n >= sizeof(buffer))
if (n >= IPP_BUF_SIZE) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

if ((*cb)(src, buffer, n) < n)
{
DEBUG_puts("ippReadIO: unable to read name!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1445,12 +1480,14 @@ if (n != 11) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, value->date, 11) < 11)
{
    DEBUG_puts("ippReadIO: Unable to date integer value!");

ipp_buffer_release(buffer); return (IPP_ERROR); } break; @@ -1459,12 +1496,14 @@ if (n != 9) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, buffer, 9) < 9)
{
    DEBUG_puts("ippReadIO: Unable to read resolution value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1482,12 +1521,14 @@ if (n != 8) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, buffer, 8) < 8)
{
    DEBUG_puts("ippReadIO: Unable to read range value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1501,15 +1542,17 @@

    case IPP_TAG_TEXTLANG :
    case IPP_TAG_NAMELANG :

if (n >= sizeof(buffer) || n < 4)
if (n >= IPP_BUF_SIZE || n < 4) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, buffer, n) < n)
{
    DEBUG_puts("ippReadIO: Unable to read string w/language value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1527,10 +1570,11 @@

    n = (bufptr[0] << 8) | bufptr[1];

if ((bufptr + 2 + n) >= (buffer + sizeof(buffer)) ||
if ((bufptr + 2 + n) >= (buffer + IPP_BUF_SIZE) || n >= sizeof(string)) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1542,9 +1586,10 @@ bufptr += 2 + n; n = (bufptr[0] << 8) | bufptr[1];

if ((bufptr + 2 + n) >= (buffer + sizeof(buffer)))
if ((bufptr + 2 + n) >= (buffer + IPP_BUF_SIZE)) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1563,17 +1608,21 @@ { DEBUG_puts("ippReadIO: begCollection tag with value length " "> 0!");

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

if (ippReadIO(src, cb, 1, ipp, value->collection) == IPP_ERROR)
{
    DEBUG_puts("ippReadIO: Unable to read collection value!");

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}
       break;

   case IPP_TAG_END_COLLECTION :

```
ipp_buffer_release(buffer);
```
+ if (n > 0) { DEBUG_puts("ippReadIO: endCollection tag with value length " @@ -1582,7 +1631,6 @@ }
```
  DEBUG_puts("ippReadIO: endCollection tag...");
```
```
return (ipp->state = IPP_DATA);

   case IPP_TAG_MEMBERNAME :
```
@@ -1591,15 +1639,17 @@ * we need to carry over... */
```
if (n >= sizeof(buffer))
```
if (n >= IPP_BUF_SIZE) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

  if ((*cb)(src, buffer, n) < n)
{
    DEBUG_puts("ippReadIO: Unable to read member name value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1621,12 +1671,14 @@ if (n > IPP_MAX_LENGTH) { DEBUG_printf(("ippReadIO: bad value length %d!\n", n));

  ipp_buffer_release(buffer);
return (IPP_ERROR);
}

if (!value)
{
DEBUG_puts("ippReadIO: NULL value!");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -1636,12 +1688,14 @@ if ((value->unknown.data = malloc(n)) == NULL) { DEBUG_puts("ippReadIO: Unable to allocate value");

    ipp_buffer_release(buffer);
  return (IPP_ERROR);
}

    if ((*cb)(src, value->unknown.data, n) < n)
{
      DEBUG_puts("ippReadIO: Unable to read unsupported value!");

```
    ipp_buffer_release(buffer);
  return (IPP_ERROR);
}
}
```
@@ -1669,6 +1723,7 @@ }

DEBUG_printf(("ippReadIO: returning ipp->state=%d!\n", ipp->state));
ipp_buffer_release(buffer);

return (ipp->state); } @@ -1773,8 +1828,7 @@ { int i; /* Looping var / int n; / Length of data */
unsigned char buffer[IPP_MAX_LENGTH + 2],
/* Data buffer + length bytes */
unsigned char buffer, / Data buffer / *bufptr; / Pointer into buffer _/ ipp_attributet *attr; / Current attribute _/ ipp_valuet *value; / Current value */ @@ -1786,6 +1840,12 @@ if (!dst || !ipp) return (IPP_ERROR);
if ((buffer = ipp_buffer_get()) == NULL)
{
DEBUG_puts("ippWriteIO: Unable to get write buffer");
return (IPP_ERROR);
} + switch (ipp->state) { case IPP_IDLE : @@ -1817,6 +1877,7 @@ if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP header...");
```
ipp_buffer_release(buffer);
return (IPP_ERROR);
```
} } @@ -1891,8 +1952,12 @@ * overflow the buffer... */

     if ((n = (int)strlen(attr->name)) > (sizeof(buffer) - 4))

if ((n = (int)strlen(attr->name)) > (IPP_BUF_SIZE - 4))
{
DEBUG_printf(("ippWriteIO: Attribute name too long (%d)\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR);

}

  /*
* Write the value tag, name length, and name string...

@@ -1916,8 +1981,12 @@ * overflow the buffer... */

     if ((n = (int)strlen(attr->name)) > (sizeof(buffer) - 7))

if ((n = (int)strlen(attr->name)) > (IPP_BUF_SIZE - 7))
{
DEBUG_printf(("ippWriteIO: Attribute name too long (%d)\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR);

}

  /*
* Write the member name tag, name length, name string, value tag,

@@ -1957,12 +2026,13 @@ i < attr->num_values; i ++, value ++) {

if ((sizeof(buffer) - (bufptr - buffer)) < 9)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 9) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2002,12 +2072,13 @@ i < attr->num_values; i ++, value ++) {

if ((sizeof(buffer) - (bufptr - buffer)) < 6)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 6) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2062,12 +2133,13 @@ ippTagString(attr->value_tag))); DEBUG_printf(("ippWriteIO: writing name=0,\"\"\n"));

if ((sizeof(buffer) - (bufptr - buffer)) < 3)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 3) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2084,18 +2156,23 @@ else n = 0;

if (n > (sizeof(buffer) - 2))
if (n > (IPP_BUF_SIZE - 2))
{
DEBUG_printf(("ippWriteIO: String too long (%d)\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR);

  }

         DEBUG_printf(("ippWriteIO: writing string=%d,\"%s\"\n", n,
              value->string.text));

           if ((int)(sizeof(buffer) - (bufptr - buffer)) < (n + 2))

if ((int)(IPP_BUF_SIZE - (bufptr - buffer)) < (n + 2)) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2128,12 +2205,13 @@ i < attr->num_values; i ++, value ++) {

if ((sizeof(buffer) - (bufptr - buffer)) < 16)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 16) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2172,13 +2250,14 @@ i < attr->num_values; i ++, value ++) {

if ((sizeof(buffer) - (bufptr - buffer)) < 14)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 14) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
return (IPP_ERROR);
ipp_buffer_release(buffer);
```
      return (IPP_ERROR);
      }

  bufptr = buffer;
```
@@ -2224,12 +2303,13 @@ i < attr->num_values; i ++, value ++) {
if ((sizeof(buffer) - (bufptr - buffer)) < 13)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 13) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2282,12 +2362,13 @@ * values with a zero-length name... */

if ((sizeof(buffer) - (bufptr - buffer)) < 3)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 3) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2317,15 +2398,21 @@ if (value->string.text != NULL) n += (int)strlen(value->string.text);

if (n > (sizeof(buffer) - 2))
if (n > (IPP_BUF_SIZE - 2))
{
DEBUG_printf(("ippWriteIO: text/nameWithLanguage value "
"too long (%d)\n", n));
ipp_buffer_release(buffer); return (IPP_ERROR);
```
           }
```

           if ((int)(sizeof(buffer) - (bufptr - buffer)) < (n + 2))

if ((int)(IPP_BUF_SIZE - (bufptr - buffer)) < (n + 2)) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2382,12 +2469,13 @@ * value... */

if ((sizeof(buffer) - (bufptr - buffer)) < 5)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 5) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2417,6 +2505,7 @@ { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2428,8 +2517,13 @@

               value->collection->state = IPP_IDLE;

if (ippWriteIO(dst, cb, 1, ipp, value->collection) == IPP_ERROR)
if (ippWriteIO(dst, cb, 1, ipp,
value->collection) == IPP_ERROR)
{
DEBUG_puts("ippWriteIO: Unable to write collection value");
ipp_buffer_release(buffer); return (IPP_ERROR);
} } break;

@@ -2445,12 +2539,13 @@ * values with a zero-length name... */

if ((sizeof(buffer) - (bufptr - buffer)) < 3)
if ((IPP_BUF_SIZE - (bufptr - buffer)) < 3) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2471,15 +2566,21 @@

               n = value->unknown.length;

if (n > (sizeof(buffer) - 2))
if (n > (IPP_BUF_SIZE - 2))
{
DEBUG_printf(("ippWriteIO: Data length too long (%d)\n",
n));
ipp_buffer_release(buffer); return (IPP_ERROR);
```
  }
```

           if ((int)(sizeof(buffer) - (bufptr - buffer)) < (n + 2))

if ((int)(IPP_BUF_SIZE - (bufptr - buffer)) < (n + 2)) { if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP " "attribute...");
ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2507,6 +2608,7 @@ if ((*cb)(dst, buffer, (int)(bufptr - buffer)) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP attribute...");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2546,6 +2648,7 @@ if ((*cb)(dst, buffer, n) < 0) { DEBUG_puts("ippWriteIO: Could not write IPP end-tag...");

ipp_buffer_release(buffer); return (IPP_ERROR); }

@@ -2560,6 +2663,8 @@ break; /* anti-compiler-warning-code */ }

ipp_buffer_release(buffer); + return (ipp->state); }

@@ -2684,6 +2789,47 @@

/*

* 'ipp_buffer_get()' - Get a read/write buffer.
/ + +static unsigned char * / O - Buffer */ +ipp_buffer_get(void) +{
_ipp_buffer_t buffer; / Current buffer */
_cups_globals_t *cg = _cupsGlobals();
/* Global data */ + +
for (buffer = cg->ipp_buffers; buffer; buffer = buffer->next)
if (!buffer->used)
{
buffer->used = 1;
return (buffer->d);
} +
if ((buffer = malloc(sizeof(_ipp_buffer_t))) == NULL)
return (NULL); +
buffer->used = 1;
buffer->next = cg->ipp_buffers;
cg->ipp_buffers = buffer; +
return (buffer->d); +} + + +/*
* 'ipp_buffer_release()' - Release a read/write buffer.
_/ + +static void +ipp_bufferrelease(unsigned char *b) / I - Buffer to release */ +{
((_ipp_buffert )b)->used = 0; +} + + +/_
- 'ipp_length()' - Compute the length of an IPP message or collection value. */

Index: cups/ipp-private.h

--- cups/ipp-private.h (revision 7988) +++ cups/ipp-private.h (working copy) @@ -35,9 +35,24 @@

/*

* Constants...
*/ + +# define IPP_BUF_SIZE (IPP_MAX_LENGTH + 2)
/* Size of buffer / + + +/
- Structures... */

+typedef struct _ipp_buffer_s /\ Read/write buffer **/ +{

unsigned char d[IPP_BUF_SIZE];/* Data buffer */
struct _ipp_buffer_s next; / Next buffer in list */
int used; /* Is this buffer used? _/ +} _ipp_buffert; + typedef struct /* Attribute mapping data */ { int multivalue; / Option has multiple values? _/ Index: cups/globals.c

--- cups/globals.c (revision 7988) +++ cups/globals.c (working copy) @@ -144,6 +144,8 @@ globalsdestructor(void *value) / I - Data to free / { int i; / Looping var */
_ipp_buffer_t buffer, / Current IPP read/write buffer */
next; / Next buffer _/ _cups_globalst *cg; / Global data */

@@ -159,6 +161,12 @@

cupsFreeOptions(cg->cupsd_num_settings, cg->cupsd_settings);

for (buffer = cg->ipp_buffers; buffer; buffer = next)
{
next = buffer->next;
free(buffer);
} + free(value); }

Index: cups/globals.h

--- cups/globals.h (revision 7988) +++ cups/globals.h (working copy) @@ -85,6 +85,7 @@

/* ipp.c _/ ipp_uchar_t ippdate[11]; / RFC-1903 date/time data */

_ipp_buffer_t _ippbuffers; / Buffer list */

/* ipp-support.c _/ int ippport; / IPP port number */

apple / cups

cups should not create large chunks of memory on the stack #2388

Index: cups/ipp.c

Index: cups/ipp-private.h

int used; /* Is this buffer used? _/ +} _ipp_buffert; + typedef struct /* Attribute mapping data **/ { int multivalue; /* Option has multiple values? _/ Index: cups/globals.c

Index: cups/globals.h

apple / cups

cups should not create large chunks of memory on the stack #2388

Index: cups/ipp.c

Index: cups/ipp-private.h

int used; /* Is this buffer used? _/ +} _ipp_buffert; + typedef struct /* Attribute mapping data */ { int multivalue; / Option has multiple values? _/ Index: cups/globals.c

Index: cups/globals.h

int used; /* Is this buffer used? _/ +} _ipp_buffert; + typedef struct /* Attribute mapping data **/ { int multivalue; /* Option has multiple values? _/ Index: cups/globals.c