From 51012526181b5b7d2e8a4bb88998609941b0936d Mon Sep 17 00:00:00 2001 From: Zdenek Dohnal Date: Tue, 6 Sep 2022 16:04:43 +0200 Subject: [PATCH] scheduler: Protect MimeDatabase from parallel access (fixes #466) If multiple create_local_bg_thread threads are spawned, they can fight over MimeDatabase (specifically over MimeDatabase->srcs array), ending up in SIGSEGV if one thread uses the array, but the other deletes it. I chose to use RW locks for protecting the pointer - IMHO we benefit from multiple threads being able to read in parallel. --- scheduler/client.c | 4 ++++ scheduler/conf.c | 3 +++ scheduler/ipp.c | 15 +++++++++++++++ scheduler/job.c | 15 +++++++++++++++ scheduler/mime.h | 2 ++ scheduler/printers.c | 16 ++++++++++++++++ 6 files changed, 55 insertions(+) diff --git a/scheduler/client.c b/scheduler/client.c index e7e419f1b2..fb3717a667 100644 --- a/scheduler/client.c +++ b/scheduler/client.c @@ -1052,8 +1052,12 @@ cupsdReadClient(cupsd_client_t *con) /* I - Client to read from */ if ((filename = get_file(con, &filestats, buf, sizeof(buf))) != NULL) { + _cupsRWLockRead(&MimeDatabase->lock); + type = mimeFileType(MimeDatabase, filename, NULL, NULL); + _cupsRWUnlock(&MimeDatabase->lock); + cupsdLogClient(con, CUPSD_LOG_DEBUG, "filename=\"%s\", type=%s/%s", filename, type ? type->super : "", type ? type->type : ""); if (is_cgi(con, filename, &filestats, type)) diff --git a/scheduler/conf.c b/scheduler/conf.c index 535d40f0c5..354cac6e00 100644 --- a/scheduler/conf.c +++ b/scheduler/conf.c @@ -1560,11 +1560,14 @@ cupsdReadConfiguration(void) MimeDatabase = mimeNew(); mimeSetErrorCallback(MimeDatabase, mime_error_cb, NULL); + _cupsRWInit(&MimeDatabase->lock); + _cupsRWLockWrite(&MimeDatabase->lock); MimeDatabase = mimeLoadTypes(MimeDatabase, mimedir); MimeDatabase = mimeLoadTypes(MimeDatabase, ServerRoot); MimeDatabase = mimeLoadFilters(MimeDatabase, mimedir, temp); MimeDatabase = mimeLoadFilters(MimeDatabase, ServerRoot, temp); + _cupsRWUnlock(&MimeDatabase->lock); if (!MimeDatabase) { diff --git a/scheduler/ipp.c b/scheduler/ipp.c index ec950557bd..bea2ad9584 100644 --- a/scheduler/ipp.c +++ b/scheduler/ipp.c @@ -8655,6 +8655,8 @@ print_job(cupsd_client_t *con, /* I - Client connection */ strlcpy(type, "octet-stream", sizeof(type)); } + _cupsRWLockRead(&MimeDatabase->lock); + if (!strcmp(super, "application") && !strcmp(type, "octet-stream")) { /* @@ -8680,6 +8682,8 @@ print_job(cupsd_client_t *con, /* I - Client connection */ else filetype = mimeType(MimeDatabase, super, type); + _cupsRWUnlock(&MimeDatabase->lock); + if (filetype && (!format || (!strcmp(super, "application") && !strcmp(type, "octet-stream")))) @@ -9899,6 +9903,8 @@ send_document(cupsd_client_t *con, /* I - Client connection */ strlcpy(type, "octet-stream", sizeof(type)); } + _cupsRWLockRead(&MimeDatabase->lock); + if (!strcmp(super, "application") && !strcmp(type, "octet-stream")) { /* @@ -9928,6 +9934,8 @@ send_document(cupsd_client_t *con, /* I - Client connection */ else filetype = mimeType(MimeDatabase, super, type); + _cupsRWUnlock(&MimeDatabase->lock); + if (filetype) { /* @@ -11417,6 +11425,8 @@ validate_job(cupsd_client_t *con, /* I - Client connection */ return; } + _cupsRWLockRead(&MimeDatabase->lock); + if ((strcmp(super, "application") || strcmp(type, "octet-stream")) && !mimeType(MimeDatabase, super, type)) { @@ -11427,8 +11437,13 @@ validate_job(cupsd_client_t *con, /* I - Client connection */ format->values[0].string.text); ippAddString(con->response, IPP_TAG_UNSUPPORTED_GROUP, IPP_TAG_MIMETYPE, "document-format", NULL, format->values[0].string.text); + + _cupsRWUnlock(&MimeDatabase->lock); + return; } + + _cupsRWUnlock(&MimeDatabase->lock); } /* diff --git a/scheduler/job.c b/scheduler/job.c index 4912775314..58d1443895 100644 --- a/scheduler/job.c +++ b/scheduler/job.c @@ -585,6 +585,8 @@ cupsdContinueJob(cupsd_job_t *job) /* I - Job */ if (stat(filename, &fileinfo)) fileinfo.st_size = 0; + _cupsRWLockWrite(&MimeDatabase->lock); + if (job->retry_as_raster) { /* @@ -619,6 +621,9 @@ cupsdContinueJob(cupsd_job_t *job) /* I - Job */ abort_state = IPP_JOB_ABORTED; ippSetString(job->attrs, &job->reasons, 0, "document-unprintable-error"); + + _cupsRWUnlock(&MimeDatabase->lock); + goto abort_job; } @@ -705,6 +710,8 @@ cupsdContinueJob(cupsd_job_t *job) /* I - Job */ cupsArrayDelete(filters); filters = prefilters; } + + _cupsRWUnlock(&MimeDatabase->lock); } /* @@ -1916,6 +1923,8 @@ cupsdLoadJob(cupsd_job_t *job) /* I - Job */ * Find all the d##### files... */ + _cupsRWLockRead(&MimeDatabase->lock); + for (fileid = 1; fileid < 10000; fileid ++) { snprintf(jobfile, sizeof(jobfile), "%s/d%05d-%03d", RequestRoot, @@ -1980,6 +1989,8 @@ cupsdLoadJob(cupsd_job_t *job) /* I - Job */ job->filetypes[fileid - 1] = mimeType(MimeDatabase, "application", "vnd.cups-raw"); } + + _cupsRWUnlock(&MimeDatabase->lock); } /* @@ -4484,6 +4495,8 @@ load_job_cache(const char *filename) /* I - job.cache filename */ number --; + _cupsRWLockRead(&MimeDatabase->lock); + job->compressions[number] = compression; job->filetypes[number] = mimeType(MimeDatabase, super, type); @@ -4510,6 +4523,8 @@ load_job_cache(const char *filename) /* I - job.cache filename */ job->filetypes[number] = mimeType(MimeDatabase, "application", "vnd.cups-raw"); } + + _cupsRWUnlock(&MimeDatabase->lock); } else cupsdLogMessage(CUPSD_LOG_ERROR, "Unknown %s directive on line %d of %s.", line, linenum, filename); diff --git a/scheduler/mime.h b/scheduler/mime.h index 7e43ac397c..ea71415b06 100644 --- a/scheduler/mime.h +++ b/scheduler/mime.h @@ -13,6 +13,7 @@ # include # include # include +# include # include @@ -106,6 +107,7 @@ typedef struct _mime_s /**** MIME Database ****/ cups_array_t *srcs; /* Filters sorted by source type */ mime_error_cb_t error_cb; /* Error message callback */ void *error_ctx; /* Pointer for callback */ + _cups_rwlock_t lock; /* Read/write lock for guarding data for background updates */ } mime_t; diff --git a/scheduler/printers.c b/scheduler/printers.c index 4efa613f3f..11dd4b8e1b 100644 --- a/scheduler/printers.c +++ b/scheduler/printers.c @@ -97,8 +97,13 @@ cupsdAddPrinter(const char *name) /* I - Name of printer */ p->state_time = time(NULL); p->accepting = 0; p->shared = DefaultShared; + + _cupsRWLockWrite(&MimeDatabase->lock); + p->filetype = mimeAddType(MimeDatabase, "printer", name); + _cupsRWUnlock(&MimeDatabase->lock); + cupsdSetString(&p->job_sheets[0], "none"); cupsdSetString(&p->job_sheets[1], "none"); @@ -815,6 +820,8 @@ cupsdDeletePrinter( if (p->printers != NULL) free(p->printers); + _cupsRWLockWrite(&MimeDatabase->lock); + delete_printer_filters(p); for (i = 0; i < p->num_reasons; i ++) @@ -828,6 +835,8 @@ cupsdDeletePrinter( mimeDeleteType(MimeDatabase, p->filetype); mimeDeleteType(MimeDatabase, p->prefiltertype); + _cupsRWUnlock(&MimeDatabase->lock); + cupsdFreeStrings(&(p->users)); cupsdFreeQuotas(p); @@ -1470,6 +1479,8 @@ cupsdRenamePrinter( * Rename the printer type... */ + _cupsRWLockWrite(&MimeDatabase->lock); + mimeDeleteType(MimeDatabase, p->filetype); p->filetype = mimeAddType(MimeDatabase, "printer", name); @@ -1479,6 +1490,8 @@ cupsdRenamePrinter( p->prefiltertype = mimeAddType(MimeDatabase, "prefilter", name); } + _cupsRWUnlock(&MimeDatabase->lock); + /* * Rename the printer... */ @@ -2295,6 +2308,7 @@ cupsdSetPrinterAttrs(cupsd_printer_t *p)/* I - Printer to setup */ cupsdCreateCommonData(); _cupsRWLockWrite(&p->lock); + _cupsRWLockWrite(&MimeDatabase->lock); /* * Clear out old filters, if any... @@ -2615,6 +2629,8 @@ cupsdSetPrinterAttrs(cupsd_printer_t *p)/* I - Printer to setup */ add_printer_formats(p); + _cupsRWUnlock(&MimeDatabase->lock); + /* * Add name-default attributes... */ -- 2.47.2