StartJob can leak fds and/or memory #921

michaelrsweet · 2004-09-29T21:12:09Z

Version: 1.1-current
CUPS.org User: jlovell

StartJob can leak fds and/or memory.

Saw this when some other process filed the process table with children which prevented cupsd from starting jobs. Eventually the original process was killed but by that time cupsd had leaked too many fds so that it wasn't able to recover.

The attached patch fixes it.

Thanks!

michaelrsweet · 2004-09-30T14:33:57Z

CUPS.org User: mike

Thanks, will rename close_pipe to cupsdClosePipe() so we can use it for the client code, and maybe rename cupsdPipe() to cupsdOpenPipe() while I'm at it...

michaelrsweet · 2004-10-04T19:39:38Z

CUPS.org User: mike

Attached is an updated patch with the new name; I also changed the other places that used similar coding to use cupsdClosePipe() instead...

Please let me know if I missed anything...

michaelrsweet · 2004-10-04T19:39:38Z

CUPS.org User: mike

Fixed in CVS - the anonymous CVS repository will be updated at midnight EST.

michaelrsweet · 2004-10-10T13:52:13Z

"startjob.patch":

Index: job.c

RCS file: /home/anoncvs/cups/scheduler/job.c,v
retrieving revision 1.230
diff -u -d -b -w -r1.230 job.c
--- job.c 9 Sep 2004 15:10:18 -0000 1.230
+++ job.c 29 Sep 2004 21:03:53 -0000
@@ -52,6 +52,7 @@

UpdateJob() - Read a status update from a job's filters.
ipp_length() - Compute the size of the buffer needed to hold
```
            the textual IPP attributes.
```
- * close_pipe() - Close the two ends of a pipe.
start_process() - Start a background process.
set_hold_until() - Set the hold time and update job-hold-until attribute.
*/
@@ -83,6 +84,7 @@

static int ipp_length(ipp_t *ipp);
static void set_time(job_t *job, const char *name);
+static void close_pipe(int *fildes);
static int start_process(const char *command, char *argv[],
char *envp[], int in, int out, int err,
int root, int *pid);
@@ -1364,6 +1366,7 @@
LogMessage(L_ERROR, "Unable to add decompression filter - %s",
strerror(errno));

```
 if (filters != NULL)
```
free(filters);

current->current_file ++;
@@ -1818,6 +1821,13 @@
{
LogMessage(L_EMERG, "Unable to allocate memory for job status buffer - %s",
strerror(errno));

 snprintf(printer->state_message, sizeof(printer->state_message),

         "Unable to allocate memory for job status buffer - %s.", strerror(errno));

```
 if (filters != NULL)
```
```
   free(filters);
```
```
 AddPrinterHistory(printer);
```
CancelJob(current->id, 0);
return;
}
@@ -1840,7 +1850,7 @@

if (filters != NULL)

free(filters);
CancelJob(current->id, 0);
return;
}

@@ -1865,6 +1875,7 @@
if (filters != NULL)
free(filters);

close_pipe(statusfds);
CancelJob(current->id, 0);
return;
}
@@ -1894,7 +1905,24 @@

if (i < (num_filters - 1) ||
strncmp(printer->device_uri, "file:", 5) != 0)

```
 cupsdPipe(filterfds[slot]);
```
{
```
 if (cupsdPipe(filterfds[slot]))
```
```
 {
```
LogMessage(L_ERROR, "Unable to create job filter pipes - %s.",
```
  strerror(errno));
```
snprintf(printer->state_message, sizeof(printer->state_message),

      "Unable to create filter pipes - %s.", strerror(errno));

if (filters != NULL)
```
free(filters);
```
close_pipe(statusfds);
close_pipe(filterfds[!slot]);
AddPrinterHistory(printer);
CancelJob(current->id, 0);
return;
```
 }
```
}
else
{
filterfds[slot][0] = -1;
@@ -1918,6 +1946,7 @@
if (filters != NULL)
free(filters);
close_pipe(statusfds);
CancelJob(current->id, 0);
return;
}
@@ -1934,10 +1963,7 @@
filterfds[slot][1], statusfds[1], 0,
current->procs + i);
if (filterfds[!slot][0] >= 0)
```
 close(filterfds[!slot][0]);
```
if (filterfds[!slot][1] >= 0)
```
 close(filterfds[!slot][1]);
```
close_pipe(filterfds[!slot]);

if (pid == 0)
{
@@ -1952,6 +1978,8 @@
if (filters != NULL)
free(filters);

```
 close_pipe(statusfds);
```
```
 close_pipe(filterfds[slot]);
```
CancelJob(current->id, 0);
return;
}
@@ -2016,10 +2044,7 @@
filterfds[slot][1], statusfds[1], 1,
current->procs + i);
if (filterfds[!slot][0] >= 0)
```
 close(filterfds[!slot][0]);
```
if (filterfds[!slot][1] >= 0)
```
 close(filterfds[!slot][1]);
```
close_pipe(filterfds[!slot]);

if (pid == 0)
{
@@ -2028,8 +2053,12 @@
snprintf(printer->state_message, sizeof(printer->state_message),
"Unable to start backend "%s" - %s.", method, strerror(errno));

```
 AddPrinterHistory(printer);
```
```
 if (filters != NULL)
```
```
   free(filters);
```
```
 close_pipe(statusfds);
```
```
 close_pipe(filterfds[slot]);
```
```
 AddPrinterHistory(printer);
```
CancelJob(current->id, 0);
return;
}
@@ -2044,16 +2073,10 @@
filterfds[slot][0] = -1;
filterfds[slot][1] = -1;
if (filterfds[!slot][0] >= 0)
```
 close(filterfds[!slot][0]);
```
if (filterfds[!slot][1] >= 0)
```
 close(filterfds[!slot][1]);
```
close_pipe(filterfds[!slot]);
}
if (filterfds[slot][0] >= 0)
close(filterfds[slot][0]);
if (filterfds[slot][1] >= 0)
close(filterfds[slot][1]);
close_pipe(filterfds[slot]);

close(statusfds[1]);

@@ -2625,6 +2648,23 @@

/*

* close_pipe() - Close the two ends of a pipe.
*/
+
+static void
+close_pipe(int *fildes)
+{
if (fildes[0] != -1)
close(fildes[0]);

if (fildes[1] != -1)
close(fildes[1]);

fildes[0] = fildes[1] = -1;
+}

+/*

'start_process()' - Start a background process.
*/

michaelrsweet · 2004-10-10T13:52:13Z

"str921.patch":

Index: client.c

RCS file: /development/cvs/cups/scheduler/client.c,v
retrieving revision 1.194
diff -u -r1.194 client.c
--- client.c 12 Sep 2004 18:49:26 -0000 1.194
+++ client.c 4 Oct 2004 19:36:30 -0000
@@ -3277,7 +3277,7 @@

Create a pipe for the output...
*/
if (cupsdPipe(fds))
if (cupsdOpenPipe(fds))
{
ClearString(&query_string);

@@ -3389,8 +3389,7 @@
LogMessage(L_ERROR, "Unable to fork for CGI %s - %s", argv[0],
strerror(errno));

close(fds[0]);
close(fds[1]);
cupsdClosePipe(fds);
pid = 0;
}
else

Index: cupsd.h

RCS file: /development/cvs/cups/scheduler/cupsd.h,v
retrieving revision 1.54
diff -u -r1.54 cupsd.h
--- cupsd.h 9 Sep 2004 15:10:18 -0000 1.54
+++ cupsd.h 4 Oct 2004 19:36:30 -0000
@@ -206,7 +206,8 @@
extern void SetStringf(char **s, const char *f, ...);
extern void StartServer(void);
extern void StopServer(void);
-extern int cupsdPipe(int *fds);
+extern void cupsdClosePipe(int *fds);
+extern int cupsdOpenPipe(int *fds);

/*

Index: dirsvc.c

RCS file: /development/cvs/cups/scheduler/dirsvc.c,v
retrieving revision 1.138
diff -u -r1.138 dirsvc.c
--- dirsvc.c 30 Sep 2004 14:38:45 -0000 1.138
+++ dirsvc.c 4 Oct 2004 19:36:30 -0000
@@ -916,7 +916,7 @@

polling daemon...
*/
if (cupsdPipe(statusfds))
if (cupsdOpenPipe(statusfds))
{
LogMessage(L_ERROR, "Unable to create polling status pipes - %s.",
strerror(errno));

Index: job.c

RCS file: /development/cvs/cups/scheduler/job.c,v
retrieving revision 1.230
diff -u -r1.230 job.c
--- job.c 9 Sep 2004 15:10:18 -0000 1.230
+++ job.c 4 Oct 2004 19:36:30 -0000
@@ -1364,7 +1364,8 @@
LogMessage(L_ERROR, "Unable to add decompression filter - %s",
strerror(errno));

```
 free(filters);
```
```
 if (filters != NULL)
```
```
   free(filters);
```
current->current_file ++;

@@ -1818,6 +1819,14 @@
{
LogMessage(L_EMERG, "Unable to allocate memory for job status buffer - %s",
strerror(errno));

 snprintf(printer->state_message, sizeof(printer->state_message),

     "Unable to allocate memory for job status buffer - %s.",

```
     strerror(errno));
```
```
 if (filters != NULL)
```
```
   free(filters);
```
```
 AddPrinterHistory(printer);
```
CancelJob(current->id, 0);
return;
}
@@ -1829,7 +1838,7 @@
- Now create processes for all of the filters...
  */
if (cupsdPipe(statusfds))
if (cupsdOpenPipe(statusfds))
{
LogMessage(L_ERROR, "Unable to create job status pipes - %s.",
strerror(errno));
@@ -1841,6 +1850,7 @@
if (filters != NULL)
free(filters);
CancelJob(current->id, 0);
return;
}

@@ -1865,6 +1875,7 @@
if (filters != NULL)
free(filters);

cupsdClosePipe(statusfds);
CancelJob(current->id, 0);
return;
}
@@ -1894,7 +1905,24 @@

if (i < (num_filters - 1) ||
strncmp(printer->device_uri, "file:", 5) != 0)

```
 cupsdPipe(filterfds[slot]);
```
{
```
 if (cupsdOpenPipe(filterfds[slot]))
```
```
 {
```
LogMessage(L_ERROR, "Unable to create job filter pipes - %s.",
```
  strerror(errno));
```
snprintf(printer->state_message, sizeof(printer->state_message),

      "Unable to create filter pipes - %s.", strerror(errno));

AddPrinterHistory(printer);

if (filters != NULL)
```
free(filters);
```
cupsdClosePipe(statusfds);
cupsdClosePipe(filterfds[!slot]);
CancelJob(current->id, 0);
return;
```
 }
```
}
else
{
filterfds[slot][0] = -1;
@@ -1918,6 +1946,7 @@
if (filters != NULL)
free(filters);
cupsdClosePipe(statusfds);
CancelJob(current->id, 0);
return;
}
@@ -1934,10 +1963,7 @@
filterfds[slot][1], statusfds[1], 0,
current->procs + i);
if (filterfds[!slot][0] >= 0)
```
 close(filterfds[!slot][0]);
```
if (filterfds[!slot][1] >= 0)
```
 close(filterfds[!slot][1]);
```
cupsdClosePipe(filterfds[!slot]);

if (pid == 0)
{
@@ -1952,6 +1978,8 @@
if (filters != NULL)
free(filters);

```
 cupsdClosePipe(statusfds);
```
```
 cupsdClosePipe(filterfds[slot]);
```
CancelJob(current->id, 0);
return;
}
@@ -2016,10 +2044,7 @@
filterfds[slot][1], statusfds[1], 1,
current->procs + i);
if (filterfds[!slot][0] >= 0)
```
 close(filterfds[!slot][0]);
```
if (filterfds[!slot][1] >= 0)
```
 close(filterfds[!slot][1]);
```
cupsdClosePipe(filterfds[!slot]);

if (pid == 0)
{
@@ -2030,6 +2055,11 @@

 AddPrinterHistory(printer);

```
 if (filters != NULL)
```
```
   free(filters);
```
```
 cupsdClosePipe(statusfds);
```
```
 cupsdClosePipe(filterfds[slot]);
```
CancelJob(current->id, 0);
return;
}
@@ -2044,16 +2074,10 @@
filterfds[slot][0] = -1;
filterfds[slot][1] = -1;
if (filterfds[!slot][0] >= 0)
```
 close(filterfds[!slot][0]);
```
if (filterfds[!slot][1] >= 0)
```
 close(filterfds[!slot][1]);
```
cupsdClosePipe(filterfds[!slot]);
}
if (filterfds[slot][0] >= 0)
close(filterfds[slot][0]);
if (filterfds[slot][1] >= 0)
close(filterfds[slot][1]);
cupsdClosePipe(filterfds[slot]);

close(statusfds[1]);

Index: main.c

RCS file: /development/cvs/cups/scheduler/main.c,v
retrieving revision 1.126
diff -u -r1.126 main.c
--- main.c 9 Sep 2004 15:53:00 -0000 1.126
+++ main.c 4 Oct 2004 19:36:30 -0000
@@ -24,7 +24,8 @@

Contents:
*
main() - Main entry for the CUPS scheduler.
* cupsdPipe() - Create a pipe which is closed on exec.
* cupsdClosePipe() - Close a pipe as necessary.
* cupsdOpenPipe() - Create a pipe which is closed on exec.
CatchChildSignals() - Catch SIGCHLD signals...
HoldSignals() - Hold child and termination signals.
IgnoreChildSignals() - Ignore SIGCHLD signals...
@@ -790,11 +791,36 @@

/*

* 'cupsdPipe()' - Create a pipe which is closed on exec.
* 'cupsdClosePipe()' - Close a pipe as necessary.
/
+
+void
+cupsdClosePipe(int *fds) / I - Pipe file descriptors (2) */
+{
/*
* Close file descriptors as needed...
*/
+
if (fds[0] >= 0)
{
close(fds[0]);
fds[0] = -1;
}
+
if (fds[1] >= 0)
{
close(fds[1]);
fds[1] = -1;
}
+}
+
+
+/*
* 'cupsdOpenPipe()' - Create a pipe which is closed on exec.
*/

int /* O - 0 on success, -1 on error /
-cupsdPipe(int *fds) / O - Pipe file descriptors (2) /
+cupsdOpenPipe(int *fds) / O - Pipe file descriptors (2) /
{
/

Create the pipe...
Index: server.c
RCS file: /development/cvs/cups/scheduler/server.c,v
retrieving revision 1.17
diff -u -r1.17 server.c
--- server.c 9 Sep 2004 15:10:18 -0000 1.17
+++ server.c 4 Oct 2004 19:36:30 -0000
@@ -93,7 +93,7 @@
Create a pipe for CGI processes...
*/
if (cupsdPipe(CGIPipes))
if (cupsdOpenPipe(CGIPipes))
LogMessage(L_ERROR, "StartServer: Unable to create pipes for CGI status!");
else
{
@@ -143,16 +143,12 @@

if (CGIPipes[0] >= 0)
{

close(CGIPipes[0]);

- close(CGIPipes[1]);

 LogMessage(L_DEBUG2, "StopServer: Removing fd %d from InputSet...",
            CGIPipes[0]);

 FD_CLR(CGIPipes[0], InputSet);

CGIPipes[0] = -1;
CGIPipes[1] = -1;
cupsdClosePipe(CGIPipes);
}

/*

michaelrsweet closed this as completed Oct 10, 2004

michaelrsweet added the priority-high label Mar 17, 2016

michaelrsweet added this to the Stable milestone Mar 17, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

StartJob can leak fds and/or memory #921

StartJob can leak fds and/or memory #921

michaelrsweet commented Sep 29, 2004

michaelrsweet commented Sep 30, 2004

michaelrsweet commented Oct 4, 2004

michaelrsweet commented Oct 4, 2004

michaelrsweet commented Oct 10, 2004

free(filters);

michaelrsweet commented Oct 10, 2004

Create the pipe...
Index: server.c

StartJob can leak fds and/or memory #921

StartJob can leak fds and/or memory #921

Comments

michaelrsweet commented Sep 29, 2004

michaelrsweet commented Sep 30, 2004

michaelrsweet commented Oct 4, 2004

michaelrsweet commented Oct 4, 2004

michaelrsweet commented Oct 10, 2004

Index: job.c

free(filters);

michaelrsweet commented Oct 10, 2004

Index: client.c

Index: cupsd.h

Index: dirsvc.c

Index: job.c

Index: main.c

Create the pipe... Index: server.c

- close(CGIPipes[1]);

Create the pipe...
Index: server.c