From c66143b3750e7a12960b7d0fbe5f6c1f1663a7c9 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 21 Jun 2012 17:12:23 +0200 Subject: [PATCH 01/17] torture: add test for smbd print job spooling Clients can print by performing file IO on a printer share, rather than issuing spoolss RPCs. This commit attempts to reproduce bug 8719. --- source4/torture/rpc/spoolss.c | 73 +++++++++++++++++++++++++++++++++++++++++ 1 files changed, 73 insertions(+), 0 deletions(-) diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c index 75adaf4..217eae9 100644 --- a/source4/torture/rpc/spoolss.c +++ b/source4/torture/rpc/spoolss.c @@ -35,9 +35,12 @@ #include "libcli/libcli.h" #include "libcli/raw/raw_proto.h" #include "libcli/resolve/resolve.h" +#include "libcli/smb2/smb2.h" +#include "libcli/smb2/smb2_calls.h" #include "lib/cmdline/popt_common.h" #include "system/filesys.h" #include "torture/ndr/ndr.h" +#include "torture/smb2/proto.h" #define TORTURE_WELLKNOWN_PRINTER "torture_wkn_printer" #define TORTURE_PRINTER "torture_printer" @@ -7726,6 +7729,75 @@ static bool test_print_test_extended(struct torture_context *tctx, return ret; } +/* use smbd file IO to spool a print job */ +static bool test_print_test_smbd(struct torture_context *tctx, + void *private_data) +{ + struct torture_printer_context *t = + (struct torture_printer_context *)talloc_get_type_abort(private_data, struct torture_printer_context); + struct dcerpc_pipe *p = t->spoolss_pipe; + struct dcerpc_binding_handle *b = p->binding_handle; + NTSTATUS status; + uint32_t count; + union spoolss_JobInfo *info = NULL; + int i; + + struct smb2_tree *tree; + struct smb2_handle job_h; + struct cli_credentials *credentials = cmdline_credentials; + struct smbcli_options options; + TALLOC_CTX *mem_ctx = talloc_new(tctx); + const char *share = t->info2.printername; + + torture_comment(tctx, "Testing smbd job spooling\n"); + lpcfg_smbcli_options(tctx->lp_ctx, &options); + + status = smb2_connect_ext(mem_ctx, + torture_setting_string(tctx, "host", NULL), + lpcfg_smb_ports(tctx->lp_ctx), + share, + lpcfg_resolve_context(tctx->lp_ctx), + credentials, + 0, + &tree, + tctx->ev, + &options, + lpcfg_socket_options(tctx->lp_ctx), + lpcfg_gensec_settings(tctx, tctx->lp_ctx)); + if (!NT_STATUS_IS_OK(status)) { + printf("Failed to connect to SMB2 printer %s - %s\n", + share, nt_errstr(status)); + return false; + } + + status = torture_smb2_testfile(tree, "smbd_spooler_job", &job_h); + torture_assert_ntstatus_ok(tctx, status, "smbd spool job create"); + + status = smb2_util_write(tree, job_h, "exciting print job data", 0, + sizeof("exciting print job data")); + torture_assert_ntstatus_ok(tctx, status, "smbd spool job write"); + + /* check back end spoolss job was created */ + torture_assert(tctx, + test_EnumJobs_args(tctx, b, &t->handle, 1, &count, &info), + "EnumJobs level 1 failed"); + + for (i = 0; i < count; i++) { + if (!strcmp(info[i].info1.document_name, "smbd_spooler_job")) { + break; + } + } + torture_assert(tctx, (i != count), "smbd_spooler_job not found"); + + status = smb2_util_close(tree, job_h); + torture_assert_ntstatus_ok(tctx, status, "smbd spool job close"); + + /* disconnect from printer share */ + talloc_free(mem_ctx); + + return true; +} + static bool test_printer_sd(struct torture_context *tctx, void *private_data) { @@ -7907,6 +7979,7 @@ void torture_tcase_printer(struct torture_tcase *tcase) torture_tcase_add_simple_test(tcase, "csetprinter", test_csetprinter); torture_tcase_add_simple_test(tcase, "print_test", test_print_test); torture_tcase_add_simple_test(tcase, "print_test_extended", test_print_test_extended); + torture_tcase_add_simple_test(tcase, "print_test_smbd", test_print_test_smbd); torture_tcase_add_simple_test(tcase, "printer_info", test_printer_info); torture_tcase_add_simple_test(tcase, "sd", test_printer_sd); torture_tcase_add_simple_test(tcase, "dm", test_printer_dm); -- 1.7.1 From ab01dcb41932ab4d322be50e441faed74304a26a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 26 Jan 2012 15:28:34 +0100 Subject: [PATCH 02/17] s3-printing: store print jobid as part of struct printjob Printing code in some places relies upon the spool-file format to retrieve the print jobid. By storing the jobid as part of struct printjob, and hence in the printing TDB, we can move away from this ugly behaviour. --- source3/include/printing.h | 3 ++- source3/printing/printing.c | 29 +++++++++++++++++------------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index bba7d53..2d6925c 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -68,6 +68,7 @@ typedef struct { /* Information for print jobs */ struct printjob { pid_t pid; /* which process launched the job */ + uint32_t jobid; /* the spoolss print job identifier */ int sysjob; /* the system (lp) job number */ int fd; /* file descriptor of open file if open */ time_t starttime; /* when the job started spooling */ @@ -123,7 +124,7 @@ extern struct printif iprint_printif; #ifndef PRINT_SPOOL_PREFIX #define PRINT_SPOOL_PREFIX "smbprn." #endif -#define PRINT_DATABASE_VERSION 7 +#define PRINT_DATABASE_VERSION 8 #ifdef AIX #define DEFAULT_PRINTING PRINT_AIX diff --git a/source3/printing/printing.c b/source3/printing/printing.c index f15bd4f..75fce1a 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -373,18 +373,20 @@ done: unpack a pjob from a tdb buffer ***********************************************************************/ -static int unpack_pjob( uint8 *buf, int buflen, struct printjob *pjob ) +static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob) { int len = 0; int used; - uint32 pjpid, pjsysjob, pjfd, pjstarttime, pjstatus; + uint32 pjpid, pjjobid, pjsysjob, pjfd, pjstarttime, pjstatus; uint32 pjsize, pjpage_count, pjspooled, pjsmbjob; - if ( !buf || !pjob ) + if (!buf || !pjob) { return -1; + } - len += tdb_unpack(buf+len, buflen-len, "dddddddddfffff", + len += tdb_unpack(buf+len, buflen-len, "ddddddddddfffff", &pjpid, + &pjjobid, &pjsysjob, &pjfd, &pjstarttime, @@ -399,8 +401,9 @@ static int unpack_pjob( uint8 *buf, int buflen, struct printjob *pjob ) pjob->clientmachine, pjob->queuename); - if ( len == -1 ) + if (len == -1) { return -1; + } used = unpack_devicemode(NULL, buf+len, buflen-len, &pjob->devmode); if (used == -1) { @@ -410,6 +413,7 @@ static int unpack_pjob( uint8 *buf, int buflen, struct printjob *pjob ) len += used; pjob->pid = pjpid; + pjob->jobid = pjjobid; pjob->sysjob = pjsysjob; pjob->fd = pjfd; pjob->starttime = pjstarttime; @@ -463,6 +467,7 @@ static struct printjob *print_job_find(const char *sharename, uint32 jobid) DEBUG(10,("print_job_find: returning system job %d for jobid %u.\n", (int)pjob.sysjob, (unsigned int)jobid )); + SMB_ASSERT(pjob.jobid == jobid); return &pjob; } @@ -489,9 +494,7 @@ static int unixjob_traverse_fn(TDB_CONTEXT *the_tdb, TDB_DATA key, return 0; if (state->sysjob == pjob->sysjob) { - uint32 jobid = IVAL(key.dptr,0); - - state->sysjob_to_jobid_value = jobid; + state->sysjob_to_jobid_value = pjob->jobid; return 1; } @@ -737,8 +740,9 @@ static bool pjob_store(struct tevent_context *ev, do { len = 0; buflen = newlen; - len += tdb_pack(buf+len, buflen-len, "dddddddddfffff", + len += tdb_pack(buf+len, buflen-len, "ddddddddddfffff", (uint32)pjob->pid, + (uint32)pjob->jobid, (uint32)pjob->sysjob, (uint32)pjob->fd, (uint32)pjob->starttime, @@ -875,6 +879,7 @@ static void print_unix_job(struct tevent_context *ev, ZERO_STRUCT(pj); pj.pid = (pid_t)-1; + pj.jobid = jobid; pj.sysjob = q->job; pj.fd = -1; pj.starttime = old_pj ? old_pj->starttime : q->time; @@ -921,11 +926,10 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void if ( key.dsize != sizeof(jobid) ) return 0; - jobid = IVAL(key.dptr, 0); - if ( unpack_pjob( data.dptr, data.dsize, &pjob ) == -1 ) + if (unpack_pjob(data.dptr, data.dsize, &pjob) == -1) return 0; talloc_free(pjob.devmode); - + jobid = pjob.jobid; if (!pjob.smbjob) { /* remove a unix job if it isn't in the system queue any more */ @@ -2825,6 +2829,7 @@ WERROR print_job_start(const struct auth_serversupplied_info *server_info, ZERO_STRUCT(pjob); pjob.pid = sys_getpid(); + pjob.jobid = jobid; pjob.sysjob = -1; pjob.fd = -1; pjob.starttime = time(NULL); -- 1.7.1 From 9e1f2f37ecd052d9d42d6275aa005d04a2569f99 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 30 Jan 2012 13:44:33 +0100 Subject: [PATCH 03/17] s3-printing: remove print_parse_jobid() from print_cups.c The spoolss print job identifier is now passed to the cups layer via struct printjob, therefore it is no longer necessary to parse the job filename to determine it. --- source3/printing/print_cups.c | 12 +----------- 1 files changed, 1 insertions(+), 11 deletions(-) diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c index ff19de2..ed2cb38 100644 --- a/source3/printing/print_cups.c +++ b/source3/printing/print_cups.c @@ -871,7 +871,6 @@ static int cups_job_submit(int snum, struct printjob *pjob) char *cupsoptions = NULL; char *filename = NULL; size_t size; - uint32_t jobid = (uint32_t)-1; DEBUG(5,("cups_job_submit(%d, %p)\n", snum, pjob)); @@ -933,21 +932,12 @@ static int cups_job_submit(int snum, struct printjob *pjob) "job-originating-host-name", NULL, pjob->clientmachine); - /* Get the jobid from the filename. */ - jobid = print_parse_jobid(pjob->filename); - if (jobid == (uint32_t)-1) { - DEBUG(0,("cups_job_submit: failed to parse jobid from name %s\n", - pjob->filename )); - jobid = 0; - } - if (!push_utf8_talloc(frame, &jobname, pjob->jobname, &size)) { goto out; } new_jobname = talloc_asprintf(frame, "%s%.8u %s", PRINT_SPOOL_PREFIX, - (unsigned int)jobid, - jobname); + pjob->jobid, jobname); if (new_jobname == NULL) { goto out; } -- 1.7.1 From 9270772d34b54fc974ac78eb78142ac34a7aa00f Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 27 Jan 2012 12:33:27 +0100 Subject: [PATCH 04/17] s3-printing: rename queue->job sysjob Print jobs maintain two job identifiers, the jobid allocated by the spoolss layer (pj->jobid), and the job identifier defined by the printing backend (pj->sysjob). Printer job queues currently only contain a single job identifier variable (queue->job), the variable is sometimes representative of the spoolss layer job identifier, and more often representative of the printing backend id. This change renames the queue job identifier from queue->job to queue->sysjob, in preparation for a change to only store the printing backend identifier. --- source3/include/printing.h | 2 +- source3/printing/lpq_parse.c | 22 +++++++++++----------- source3/printing/print_cups.c | 2 +- source3/printing/print_iprint.c | 2 +- source3/printing/printing.c | 26 +++++++++++++------------- source3/rpc_server/spoolss/srv_spoolss_nt.c | 16 ++++++++-------- 6 files changed, 35 insertions(+), 35 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index 2d6925c..793a581 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -46,7 +46,7 @@ enum { }; typedef struct _print_queue_struct { - int job; /* normally the UNIX jobid -- see note in + int sysjob; /* normally the UNIX jobid -- see note in printing.c:traverse_fn_delete() */ int size; int page_count; diff --git a/source3/printing/lpq_parse.c b/source3/printing/lpq_parse.c index 16b9b09..2a14dfa 100644 --- a/source3/printing/lpq_parse.c +++ b/source3/printing/lpq_parse.c @@ -164,7 +164,7 @@ static bool parse_lpq_bsd(char *line,print_queue_struct *buf,bool first) return False; } - buf->job = atoi(tok[JOBTOK]); + buf->sysjob = atoi(tok[JOBTOK]); buf->size = atoi(tok[TOTALTOK]); buf->status = strequal(tok[RANKTOK],"active")?LPQ_PRINTING:LPQ_QUEUED; buf->time = time(NULL); @@ -281,7 +281,7 @@ static bool parse_lpq_lprng(char *line,print_queue_struct *buf,bool first) return False; } - buf->job = atoi(tokarr[LPRNG_JOBTOK]); + buf->sysjob = atoi(tokarr[LPRNG_JOBTOK]); buf->size = atoi(tokarr[LPRNG_TOTALTOK]); if (strequal(tokarr[LPRNG_RANKTOK],"active")) { @@ -384,7 +384,7 @@ static bool parse_lpq_aix(char *line,print_queue_struct *buf,bool first) } } - buf->job = atoi(tok[1]); + buf->sysjob = atoi(tok[1]); buf->status = strequal(tok[0],"HELD")?LPQ_PAUSED:LPQ_QUEUED; buf->priority = 0; buf->time = time(NULL); @@ -420,7 +420,7 @@ static bool parse_lpq_aix(char *line,print_queue_struct *buf,bool first) } } - buf->job = atoi(tok[3]); + buf->sysjob = atoi(tok[3]); buf->status = strequal(tok[2],"RUNNING")?LPQ_PRINTING:LPQ_QUEUED; buf->priority = 0; buf->time = time(NULL); @@ -511,7 +511,7 @@ static bool parse_lpq_hpux(char *line, print_queue_struct *buf, bool first) /* fill things from header line */ buf->time = jobtime; - buf->job = jobid; + buf->sysjob = jobid; buf->status = jobstat; buf->priority = jobprio; if (jobuser) { @@ -651,7 +651,7 @@ static bool parse_lpq_sysv(char *line,print_queue_struct *buf,bool first) tok[2] = p+1; } - buf->job = atoi(tok[1]); + buf->sysjob = atoi(tok[1]); buf->size = atoi(tok[3]); if (count > 7 && strequal(tok[7],"on")) { buf->status = LPQ_PRINTING; @@ -726,7 +726,7 @@ static bool parse_lpq_qnx(char *line,print_queue_struct *buf,bool first) } } - buf->job = atoi(tok[2]); + buf->sysjob = atoi(tok[2]); buf->size = atoi(tok[4]); buf->status = strequal(tok[3],"active")?LPQ_PRINTING:LPQ_QUEUED; buf->priority = 0; @@ -806,7 +806,7 @@ static bool parse_lpq_plp(char *line,print_queue_struct *buf,bool first) } } - buf->job = atoi(tok[4]); + buf->sysjob = atoi(tok[4]); buf->size = atoi(tok[7]); if (strchr_m(tok[7],'K')) { @@ -896,7 +896,7 @@ static bool parse_lpq_nt(char *line,print_queue_struct *buf,bool first) parse_line->space3 = '\0'; trim_char(parse_line->jobname, '\0', ' '); - buf->job = atoi(parse_line->jobid); + buf->sysjob = atoi(parse_line->jobid); buf->priority = 0; buf->size = atoi(parse_line->size); buf->time = time(NULL); @@ -957,7 +957,7 @@ static bool parse_lpq_os2(char *line,print_queue_struct *buf,bool first) } /* Get the jobid */ - buf->job = atoi(parse_line->jobid); + buf->sysjob = atoi(parse_line->jobid); /* Get the job name */ parse_line->space2[0] = '\0'; @@ -1023,7 +1023,7 @@ static bool parse_lpq_vlp(char *line,print_queue_struct *buf,bool first) while(next_token_talloc(frame, &cline, &tok, NULL)) { switch (toknum) { case 0: - buf->job = atoi(tok); + buf->sysjob = atoi(tok); break; case 1: buf->size = atoi(tok); diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c index ed2cb38..8f0cd88 100644 --- a/source3/printing/print_cups.c +++ b/source3/printing/print_cups.c @@ -1243,7 +1243,7 @@ static int cups_queue_get(const char *sharename, continue; } - temp->job = job_id; + temp->sysjob = job_id; temp->size = job_k_octets * 1024; temp->status = job_status == IPP_JOB_PENDING ? LPQ_QUEUED : job_status == IPP_JOB_STOPPED ? LPQ_PAUSED : diff --git a/source3/printing/print_iprint.c b/source3/printing/print_iprint.c index 1392cba..9e741c1 100644 --- a/source3/printing/print_iprint.c +++ b/source3/printing/print_iprint.c @@ -1162,7 +1162,7 @@ static int iprint_queue_get(const char *sharename, continue; } - temp->job = job_id; + temp->sysjob = job_id; temp->size = job_k_octets * 1024; temp->status = job_status == IPP_JOB_PENDING ? LPQ_QUEUED : job_status == IPP_JOB_STOPPED ? LPQ_PAUSED : diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 75fce1a..d44405b 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -870,7 +870,7 @@ static void print_unix_job(struct tevent_context *ev, struct printjob pj, *old_pj; if (jobid == (uint32)-1) - jobid = q->job + UNIX_JOB_START; + jobid = q->sysjob + UNIX_JOB_START; /* Preserve the timestamp on an existing unix print job */ @@ -880,7 +880,7 @@ static void print_unix_job(struct tevent_context *ev, pj.pid = (pid_t)-1; pj.jobid = jobid; - pj.sysjob = q->job; + pj.sysjob = q->sysjob; pj.fd = -1; pj.starttime = old_pj ? old_pj->starttime : q->time; pj.status = q->status; @@ -935,7 +935,7 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void /* remove a unix job if it isn't in the system queue any more */ for (i=0;iqcount;i++) { - uint32 u_jobid = (ts->queue[i].job + UNIX_JOB_START); + uint32 u_jobid = (ts->queue[i].sysjob + UNIX_JOB_START); if (jobid == u_jobid) break; } @@ -1036,7 +1036,7 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void FIXME!!! This is the only place where queue->job represents the SMB jobid --jerry */ - ts->queue[i].job = jobid; + ts->queue[i].sysjob = jobid; ts->queue[i].size = pjob.size; ts->queue[i].page_count = pjob.page_count; ts->queue[i].status = pjob.status; @@ -1187,7 +1187,7 @@ static void store_queue_struct(struct tdb_print_db *pdb, struct traverse_struct qcount++; data.dsize += tdb_pack(NULL, 0, "ddddddff", - (uint32)queue[i].job, + (uint32)queue[i].sysjob, (uint32)queue[i].size, (uint32)queue[i].page_count, (uint32)queue[i].status, @@ -1207,7 +1207,7 @@ static void store_queue_struct(struct tdb_print_db *pdb, struct traverse_struct continue; len += tdb_pack(data.dptr + len, data.dsize - len, "ddddddff", - (uint32)queue[i].job, + (uint32)queue[i].sysjob, (uint32)queue[i].size, (uint32)queue[i].page_count, (uint32)queue[i].status, @@ -1403,7 +1403,7 @@ static void print_queue_update_internal( struct tevent_context *ev, continue; } - pjob->sysjob = queue[i].job; + pjob->sysjob = queue[i].sysjob; /* don't reset the status on jobs to be deleted */ @@ -3080,7 +3080,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, &qtime, queue[i].fs_user, queue[i].fs_file); - queue[i].job = qjob; + queue[i].sysjob = qjob; queue[i].size = qsize; queue[i].page_count = qpage_count; queue[i].status = qstatus; @@ -3104,7 +3104,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, continue; } - queue[total_count].job = jobid; + queue[total_count].sysjob = jobid; queue[total_count].size = pjob->size; queue[total_count].page_count = pjob->page_count; queue[total_count].status = pjob->status; @@ -3122,7 +3122,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, bool found = false; for (j = 0; j < total_count; j++) { - if (queue[j].job == jobid) { + if (queue[j].sysjob == jobid) { found = true; break; } @@ -3143,7 +3143,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, continue; } - queue[j].job = jobid; + queue[j].sysjob = jobid; queue[j].size = pjob->size; queue[j].page_count = pjob->page_count; queue[j].status = pjob->status; @@ -3345,11 +3345,11 @@ WERROR print_queue_purge(const struct auth_serversupplied_info *server_info, for (i=0;ijob); + SETUP_SPOOLSS_NOTIFY_DATA_INTEGER(data, queue->sysjob); } /******************************************************************* @@ -3685,7 +3685,7 @@ static WERROR printer_notify_info(struct pipes_struct *p, &queue[j], info, pinfo2, snum, &option_type, - queue[j].job, + queue[j].sysjob, mem_ctx); } @@ -6783,7 +6783,7 @@ static WERROR fill_job_info1(TALLOC_CTX *mem_ctx, t = gmtime(&queue->time); - r->job_id = queue->job; + r->job_id = queue->sysjob; r->printer_name = talloc_strdup(mem_ctx, lp_servicename(snum)); W_ERROR_HAVE_NO_MEMORY(r->printer_name); @@ -6824,7 +6824,7 @@ static WERROR fill_job_info2(TALLOC_CTX *mem_ctx, t = gmtime(&queue->time); - r->job_id = queue->job; + r->job_id = queue->sysjob; r->printer_name = talloc_strdup(mem_ctx, lp_servicename(snum)); W_ERROR_HAVE_NO_MEMORY(r->printer_name); @@ -6877,10 +6877,10 @@ static WERROR fill_job_info3(TALLOC_CTX *mem_ctx, int position, int snum, struct spoolss_PrinterInfo2 *pinfo2) { - r->job_id = queue->job; + r->job_id = queue->sysjob; r->next_job_id = 0; if (next_queue) { - r->next_job_id = next_queue->job; + r->next_job_id = next_queue->sysjob; } r->reserved = 0; @@ -9028,7 +9028,7 @@ static WERROR getjob_level_1(TALLOC_CTX *mem_ctx, bool found = false; for (i=0; i Date: Mon, 30 Jan 2012 13:35:21 +0100 Subject: [PATCH 05/17] s3-printing: remove print_parse_jobid() calls from printing.c In all cases the spoolss layer job id can be determinded from the printing subsystem allocated job identifier (sysjob). --- source3/printing/printing.c | 48 ++++++++++++++++++++++++------------------ 1 files changed, 27 insertions(+), 21 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index d44405b..2fe3d2e 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -501,6 +501,18 @@ static int unixjob_traverse_fn(TDB_CONTEXT *the_tdb, TDB_DATA key, return 0; } +static uint32 sysjob_to_jobid_pdb(struct tdb_print_db *pdb, int sysjob) +{ + struct unixjob_traverse_state state; + + state.sysjob = sysjob; + state.sysjob_to_jobid_value = (uint32)-1; + + tdb_traverse(pdb->tdb, unixjob_traverse_fn, &state); + + return state.sysjob_to_jobid_value; +} + /**************************************************************************** This is a *horribly expensive call as we have to iterate through all the current printer tdb's. Don't do this often ! JRA. @@ -933,11 +945,10 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void if (!pjob.smbjob) { /* remove a unix job if it isn't in the system queue any more */ - for (i=0;iqcount;i++) { - uint32 u_jobid = (ts->queue[i].sysjob + UNIX_JOB_START); - if (jobid == u_jobid) + if (ts->queue[i].sysjob == pjob.sysjob) { break; + } } if (i == ts->qcount) { DEBUG(10,("traverse_fn_delete: pjob %u deleted due to !smbjob\n", @@ -968,16 +979,12 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void /* this check only makes sense for jobs submitted from Windows clients */ - if ( pjob.smbjob ) { + if (pjob.smbjob) { for (i=0;iqcount;i++) { - uint32 curr_jobid; - if ( pjob.status == LPQ_DELETED ) continue; - curr_jobid = print_parse_jobid(ts->queue[i].fs_file); - - if (jobid == curr_jobid) { + if (ts->queue[i].sysjob == pjob.sysjob) { /* try to clean up any jobs that need to be deleted */ @@ -1032,11 +1039,8 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void return 0; } - /* Save the pjob attributes we will store. - FIXME!!! This is the only place where queue->job - represents the SMB jobid --jerry */ - - ts->queue[i].sysjob = jobid; + /* Save the pjob attributes we will store. */ + ts->queue[i].sysjob = pjob.sysjob; ts->queue[i].size = pjob.size; ts->queue[i].page_count = pjob.page_count; ts->queue[i].status = pjob.status; @@ -1321,11 +1325,11 @@ done: main work for updating the lpq cache for a printer queue ****************************************************************************/ -static void print_queue_update_internal( struct tevent_context *ev, - struct messaging_context *msg_ctx, - const char *sharename, - struct printif *current_printif, - char *lpq_command, char *lprm_command ) +static void print_queue_update_internal(struct tevent_context *ev, + struct messaging_context *msg_ctx, + const char *sharename, + struct printif *current_printif, + char *lpq_command, char *lprm_command) { int i, qcount; print_queue_struct *queue = NULL; @@ -1383,8 +1387,7 @@ static void print_queue_update_internal( struct tevent_context *ev, jcdata = get_jobs_added_data(pdb); for (i=0; isysjob = queue[i].sysjob; /* don't reset the status on jobs to be deleted */ -- 1.7.1 From 71a2f1877895516ec810706dd74ca5307e30376a Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 30 Jan 2012 16:05:21 +0100 Subject: [PATCH 06/17] s3-printing: remove redundant variable set --- source3/printing/printing.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 2fe3d2e..d41c606 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -1408,9 +1408,6 @@ static void print_queue_update_internal(struct tevent_context *ev, continue; } - /* FIXME this is already confirmed by sysjob_to_jobid_pdb() */ - pjob->sysjob = queue[i].sysjob; - /* don't reset the status on jobs to be deleted */ if ( pjob->status != LPQ_DELETING ) -- 1.7.1 From 3e79f7a020b0e0e20c7d818e0728c030f080fc02 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Mon, 30 Jan 2012 17:35:28 +0100 Subject: [PATCH 07/17] s3-printing: remove print_parse_jobid() With all callers fixed, it is now safe to remove. --- source3/include/printing.h | 1 - source3/printing/lpq_parse.c | 19 ------------------- 2 files changed, 0 insertions(+), 20 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index 793a581..f749358 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -246,7 +246,6 @@ void printing_end(void); bool parse_lpq_entry(enum printing_types printing_type,char *line, print_queue_struct *buf, print_status_struct *status,bool first); -uint32_t print_parse_jobid(const char *fname); /* The following definitions come from printing/printing_db.c */ diff --git a/source3/printing/lpq_parse.c b/source3/printing/lpq_parse.c index 2a14dfa..06790d8 100644 --- a/source3/printing/lpq_parse.c +++ b/source3/printing/lpq_parse.c @@ -1152,22 +1152,3 @@ bool parse_lpq_entry(enum printing_types printing_type,char *line, return ret; } -/**************************************************************************** - Parse a file name from the system spooler to generate a jobid. -****************************************************************************/ - -uint32_t print_parse_jobid(const char *fname) -{ - int jobid; - const char *p = strstr_m(fname,PRINT_SPOOL_PREFIX); - - if (!p) { - return (uint32_t)-1; - } - p += strlen(PRINT_SPOOL_PREFIX); - jobid = atoi(p); - if (jobid <= 0) { - return (uint32_t)-1; - } - return (uint32_t)jobid; -} -- 1.7.1 From 384d4ec88318b17af36a624693e643b7cbbe5ba3 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 1 Feb 2012 13:21:04 +0100 Subject: [PATCH 08/17] s3-spoolss: remove duplicate "." in smbd spooler path --- source3/printing/printspoolss.c | 11 ++++++++--- 1 files changed, 8 insertions(+), 3 deletions(-) diff --git a/source3/printing/printspoolss.c b/source3/printing/printspoolss.c index 3d8b0d8..23464d5 100644 --- a/source3/printing/printspoolss.c +++ b/source3/printing/printspoolss.c @@ -82,7 +82,8 @@ NTSTATUS print_spool_open(files_struct *fsp, } } - /* Ok, now we have to open an actual file. + /* + * Ok, now we have to open an actual file. * Here is the reason: * We want to write the spool job to this file in * smbd for scalability reason (and also because @@ -92,9 +93,13 @@ NTSTATUS print_spool_open(files_struct *fsp, * to spoolss in output_file so it can monitor and * take over once we call EndDocPrinter(). * Of course we will not start writing until - * StartDocPrinter() actually gives the ok. */ + * StartDocPrinter() actually gives the ok. + * smbd spooler files do not include a print jobid + * path component, as the jobid is only known after + * calling StartDocPrinter(). + */ - pf->filename = talloc_asprintf(pf, "%s/%s.XXXXXX", + pf->filename = talloc_asprintf(pf, "%s/%sXXXXXX", lp_pathname(SNUM(fsp->conn)), PRINT_SPOOL_PREFIX); if (!pf->filename) { -- 1.7.1 From 3db4fc7c93027fe12025581ab6cefd7e8e69ed2b Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 8 Feb 2012 13:45:40 +0100 Subject: [PATCH 09/17] s3-printing: fix potential print db refcount leak --- source3/printing/printing.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index d41c606..f928557 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -820,8 +820,8 @@ static bool pjob_store(struct tevent_context *ev, } } - release_print_db(pdb); done: + release_print_db(pdb); SAFE_FREE( old_data.dptr ); SAFE_FREE( buf ); -- 1.7.1 From a4a8bd335167c9c9aeb152ccc1010f30c13b8f51 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 8 Feb 2012 15:01:15 +0100 Subject: [PATCH 10/17] s3-printing: clean up print_job_pause/resume interface Currently both return a bool and sometimes set a werr pointer argument, always return werror instead. --- source3/include/printing.h | 8 ++-- source3/printing/printing.c | 47 ++++++++++++++++----------- source3/rpc_server/spoolss/srv_spoolss_nt.c | 12 ++---- 3 files changed, 36 insertions(+), 31 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index f749358..0c749ba 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -204,12 +204,12 @@ bool print_job_get_name(TALLOC_CTX *mem_ctx, const char *sharename, uint32_t job WERROR print_job_delete(const struct auth_serversupplied_info *server_info, struct messaging_context *msg_ctx, int snum, uint32_t jobid); -bool print_job_pause(const struct auth_serversupplied_info *server_info, +WERROR print_job_pause(const struct auth_serversupplied_info *server_info, struct messaging_context *msg_ctx, - int snum, uint32 jobid, WERROR *errcode); -bool print_job_resume(const struct auth_serversupplied_info *server_info, + int snum, uint32 jobid); +WERROR print_job_resume(const struct auth_serversupplied_info *server_info, struct messaging_context *msg_ctx, - int snum, uint32 jobid, WERROR *errcode); + int snum, uint32 jobid); ssize_t print_job_write(struct tevent_context *ev, struct messaging_context *msg_ctx, int snum, uint32 jobid, const char *buf, size_t size); diff --git a/source3/printing/printing.c b/source3/printing/printing.c index f928557..174f628 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -2333,27 +2333,30 @@ pause, or resume print job. User name: %s. Printer name: %s.", Pause a job. ****************************************************************************/ -bool print_job_pause(const struct auth_serversupplied_info *server_info, +WERROR print_job_pause(const struct auth_serversupplied_info *server_info, struct messaging_context *msg_ctx, - int snum, uint32 jobid, WERROR *errcode) + int snum, uint32 jobid) { const char* sharename = lp_const_servicename(snum); struct printjob *pjob; int ret = -1; struct printif *current_printif = get_printer_fns( snum ); + WERROR werr; pjob = print_job_find(sharename, jobid); if (!pjob || !server_info) { DEBUG(10, ("print_job_pause: no pjob or user for jobid %u\n", (unsigned int)jobid )); - return False; + werr = WERR_INVALID_PARAM; + goto err_out; } if (!pjob->spooled || pjob->sysjob == -1) { DEBUG(10, ("print_job_pause: not spooled or bad sysjob = %d for jobid %u\n", (int)pjob->sysjob, (unsigned int)jobid )); - return False; + werr = WERR_INVALID_PARAM; + goto err_out; } if (!is_owner(server_info, lp_const_servicename(snum), jobid) && @@ -2369,16 +2372,16 @@ pause, or resume print job. User name: %s. Printer name: %s.", lp_printername(snum) ); /* END_ADMIN_LOG */ - *errcode = WERR_ACCESS_DENIED; - return False; + werr = WERR_ACCESS_DENIED; + goto err_out; } /* need to pause the spooled entry */ ret = (*(current_printif->job_pause))(snum, pjob); if (ret != 0) { - *errcode = WERR_INVALID_PARAM; - return False; + werr = WERR_INVALID_PARAM; + goto err_out; } /* force update the database */ @@ -2390,42 +2393,45 @@ pause, or resume print job. User name: %s. Printer name: %s.", JOB_STATUS_PAUSED); /* how do we tell if this succeeded? */ - - return True; + werr = WERR_OK; +err_out: + return werr; } /**************************************************************************** Resume a job. ****************************************************************************/ -bool print_job_resume(const struct auth_serversupplied_info *server_info, +WERROR print_job_resume(const struct auth_serversupplied_info *server_info, struct messaging_context *msg_ctx, - int snum, uint32 jobid, WERROR *errcode) + int snum, uint32 jobid) { const char *sharename = lp_const_servicename(snum); struct printjob *pjob; int ret; struct printif *current_printif = get_printer_fns( snum ); + WERROR werr; pjob = print_job_find(sharename, jobid); if (!pjob || !server_info) { DEBUG(10, ("print_job_resume: no pjob or user for jobid %u\n", (unsigned int)jobid )); - return False; + werr = WERR_INVALID_PARAM; + goto err_out; } if (!pjob->spooled || pjob->sysjob == -1) { DEBUG(10, ("print_job_resume: not spooled or bad sysjob = %d for jobid %u\n", (int)pjob->sysjob, (unsigned int)jobid )); - return False; + werr = WERR_INVALID_PARAM; + goto err_out; } if (!is_owner(server_info, lp_const_servicename(snum), jobid) && !print_access_check(server_info, msg_ctx, snum, JOB_ACCESS_ADMINISTER)) { DEBUG(3, ("resume denied by security descriptor\n")); - *errcode = WERR_ACCESS_DENIED; /* BEGIN_ADMIN_LOG */ sys_adminlog( LOG_ERR, @@ -2434,14 +2440,15 @@ pause, or resume print job. User name: %s. Printer name: %s.", uidtoname(server_info->utok.uid), lp_printername(snum) ); /* END_ADMIN_LOG */ - return False; + werr = WERR_ACCESS_DENIED; + goto err_out; } ret = (*(current_printif->job_resume))(snum, pjob); if (ret != 0) { - *errcode = WERR_INVALID_PARAM; - return False; + werr = WERR_INVALID_PARAM; + goto err_out; } /* force update the database */ @@ -2452,7 +2459,9 @@ pause, or resume print job. User name: %s. Printer name: %s.", notify_job_status(server_event_context(), msg_ctx, sharename, jobid, JOB_STATUS_QUEUED); - return True; + werr = WERR_OK; +err_out: + return werr; } /**************************************************************************** diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index 0e520fb..e0f5be5 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -7189,17 +7189,13 @@ WERROR _spoolss_SetJob(struct pipes_struct *p, } break; case SPOOLSS_JOB_CONTROL_PAUSE: - if (print_job_pause(session_info, p->msg_ctx, - snum, r->in.job_id, &errcode)) { - errcode = WERR_OK; - } + errcode = print_job_pause(session_info, p->msg_ctx, + snum, r->in.job_id); break; case SPOOLSS_JOB_CONTROL_RESTART: case SPOOLSS_JOB_CONTROL_RESUME: - if (print_job_resume(session_info, p->msg_ctx, - snum, r->in.job_id, &errcode)) { - errcode = WERR_OK; - } + errcode = print_job_resume(session_info, p->msg_ctx, + snum, r->in.job_id); break; case 0: errcode = WERR_OK; -- 1.7.1 From 91a31f0303cf79dec89c3bef95aadd3153f54d62 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 8 Feb 2012 16:55:40 +0100 Subject: [PATCH 11/17] s3-printing: return talloced print jobs print_job_find() currently returns print jobs to callers via a statically allocated variable, this is particularly messy as the device mode is talloced under the static variable. This change adds or passes a talloc context to all callers, giving them ownership of the returned print job. --- source3/include/printing.h | 4 +- source3/printing/printing.c | 291 +++++++++++++++++++++++++++------------- source4/torture/rpc/spoolss.c | 23 ++-- 3 files changed, 210 insertions(+), 108 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index 0c749ba..a49d6a5 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -196,7 +196,9 @@ bool print_notify_register_pid(int snum); bool print_notify_deregister_pid(int snum); bool print_job_exists(const char* sharename, uint32 jobid); char *print_job_fname(const char* sharename, uint32 jobid); -struct spoolss_DeviceMode *print_job_devmode(const char* sharename, uint32 jobid); +struct spoolss_DeviceMode *print_job_devmode(TALLOC_CTX *mem_ctx, + const char *sharename, + uint32 jobid); bool print_job_set_name(struct tevent_context *ev, struct messaging_context *msg_ctx, const char *sharename, uint32 jobid, const char *name); diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 174f628..c300c8b 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -373,6 +373,7 @@ done: unpack a pjob from a tdb buffer ***********************************************************************/ +/* FIXME talloc ctx */ static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob) { int len = 0; @@ -431,9 +432,11 @@ static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob) Useful function to find a print job in the database. ****************************************************************************/ -static struct printjob *print_job_find(const char *sharename, uint32 jobid) +static struct printjob *print_job_find(TALLOC_CTX *mem_ctx, + const char *sharename, + uint32 jobid) { - static struct printjob pjob; + struct printjob *pjob; uint32_t tmp; TDB_DATA ret; struct tdb_print_db *pdb = get_print_db_byname(sharename); @@ -449,27 +452,30 @@ static struct printjob *print_job_find(const char *sharename, uint32 jobid) release_print_db(pdb); if (!ret.dptr) { - DEBUG(10,("print_job_find: failed to find jobid %u.\n", (unsigned int)jobid )); + DEBUG(10, ("print_job_find: failed to find jobid %u.\n", + jobid)); return NULL; } - talloc_free(pjob.devmode); - - ZERO_STRUCT( pjob ); - - if ( unpack_pjob( ret.dptr, ret.dsize, &pjob ) == -1 ) { - DEBUG(10,("print_job_find: failed to unpack jobid %u.\n", (unsigned int)jobid )); - SAFE_FREE(ret.dptr); - return NULL; + pjob = talloc_zero(mem_ctx, struct printjob); + if (pjob == NULL) { + goto err_out; } - SAFE_FREE(ret.dptr); + if (unpack_pjob(ret.dptr, ret.dsize, pjob) == -1) { + DEBUG(10, ("failed to unpack jobid %u.\n", jobid)); + talloc_free(pjob); + pjob = NULL; + goto err_out; + } DEBUG(10,("print_job_find: returning system job %d for jobid %u.\n", - (int)pjob.sysjob, (unsigned int)jobid )); - SMB_ASSERT(pjob.jobid == jobid); + pjob->sysjob, jobid)); + SMB_ASSERT(pjob->jobid == jobid); - return &pjob; +err_out: + SAFE_FREE(ret.dptr); + return pjob; } /* Convert a unix jobid to a smb jobid */ @@ -840,19 +846,21 @@ static void pjob_delete(struct tevent_context *ev, struct printjob *pjob; uint32 job_status = 0; struct tdb_print_db *pdb; - - pdb = get_print_db_byname( sharename ); - - if (!pdb) + TALLOC_CTX *tmp_ctx = talloc_new(ev); + if (tmp_ctx == NULL) { return; + } - pjob = print_job_find( sharename, jobid ); + pdb = get_print_db_byname(sharename); + if (!pdb) { + goto err_out; + } + pjob = print_job_find(tmp_ctx, sharename, jobid); if (!pjob) { - DEBUG(5, ("pjob_delete: we were asked to delete nonexistent job %u\n", - (unsigned int)jobid)); - release_print_db(pdb); - return; + DEBUG(5, ("we were asked to delete nonexistent job %u\n", + jobid)); + goto err_release; } /* We must cycle through JOB_STATUS_DELETING and @@ -866,8 +874,11 @@ static void pjob_delete(struct tevent_context *ev, tdb_delete(pdb->tdb, print_key(jobid, &tmp)); remove_from_jobs_added(sharename, jobid); - release_print_db( pdb ); rap_jobid_delete(sharename, jobid); +err_release: + release_print_db(pdb); +err_out: + talloc_free(tmp_ctx); } /**************************************************************************** @@ -880,13 +891,18 @@ static void print_unix_job(struct tevent_context *ev, uint32 jobid) { struct printjob pj, *old_pj; + TALLOC_CTX *tmp_ctx = talloc_new(ev); + if (tmp_ctx == NULL) { + return; + } - if (jobid == (uint32)-1) + if (jobid == (uint32)-1) { jobid = q->sysjob + UNIX_JOB_START; + } /* Preserve the timestamp on an existing unix print job */ - old_pj = print_job_find(sharename, jobid); + old_pj = print_job_find(tmp_ctx, sharename, jobid); ZERO_STRUCT(pj); @@ -910,6 +926,7 @@ static void print_unix_job(struct tevent_context *ev, fstrcpy(pj.queuename, old_pj ? old_pj->queuename : sharename ); pjob_store(ev, msg_ctx, sharename, jobid, &pj); + talloc_free(tmp_ctx); } @@ -1341,8 +1358,9 @@ static void print_queue_update_internal(struct tevent_context *ev, TDB_DATA jcdata; fstring keystr, cachestr; struct tdb_print_db *pdb = get_print_db_byname(sharename); + TALLOC_CTX *tmp_ctx = talloc_new(ev); - if (!pdb) { + if ((pdb == NULL) || (tmp_ctx == NULL)) { return; } @@ -1383,7 +1401,6 @@ static void print_queue_update_internal(struct tevent_context *ev, fill in any system job numbers as we go */ - jcdata = get_jobs_added_data(pdb); for (i=0; ispooled || pjob->pid != sys_getpid()) return NULL; return pjob->filename; @@ -2071,12 +2089,14 @@ char *print_job_fname(const char* sharename, uint32 jobid) has not been spooled. ****************************************************************************/ -struct spoolss_DeviceMode *print_job_devmode(const char* sharename, uint32 jobid) +struct spoolss_DeviceMode *print_job_devmode(TALLOC_CTX *mem_ctx, + const char *sharename, + uint32 jobid) { - struct printjob *pjob = print_job_find(sharename, jobid); - - if ( !pjob ) + struct printjob *pjob = print_job_find(mem_ctx, sharename, jobid); + if (pjob == NULL) { return NULL; + } return pjob->devmode; } @@ -2090,13 +2110,23 @@ bool print_job_set_name(struct tevent_context *ev, const char *sharename, uint32 jobid, const char *name) { struct printjob *pjob; + bool ret; + TALLOC_CTX *tmp_ctx = talloc_new(ev); + if (tmp_ctx == NULL) { + return false; + } - pjob = print_job_find(sharename, jobid); - if (!pjob || pjob->pid != sys_getpid()) - return False; + pjob = print_job_find(tmp_ctx, sharename, jobid); + if (!pjob || pjob->pid != sys_getpid()) { + ret = false; + goto err_out; + } fstrcpy(pjob->jobname, name); - return pjob_store(ev, msg_ctx, sharename, jobid, pjob); + ret = pjob_store(ev, msg_ctx, sharename, jobid, pjob); +err_out: + talloc_free(tmp_ctx); + return ret; } /**************************************************************************** @@ -2107,17 +2137,12 @@ bool print_job_get_name(TALLOC_CTX *mem_ctx, const char *sharename, uint32_t job { struct printjob *pjob; - pjob = print_job_find(sharename, jobid); + pjob = print_job_find(mem_ctx, sharename, jobid); if (!pjob || pjob->pid != sys_getpid()) { return false; } - *name = talloc_strdup(mem_ctx, pjob->jobname); - if (!*name) { - return false; - } - - return true; + return pjob->jobname; } @@ -2189,19 +2214,29 @@ static bool print_job_delete1(struct tevent_context *ev, int snum, uint32 jobid) { const char* sharename = lp_const_servicename(snum); - struct printjob *pjob = print_job_find(sharename, jobid); + struct printjob *pjob; int result = 0; struct printif *current_printif = get_printer_fns( snum ); + bool ret; + TALLOC_CTX *tmp_ctx = talloc_new(ev); + if (tmp_ctx == NULL) { + return false; + } - if (!pjob) - return False; + pjob = print_job_find(tmp_ctx, sharename, jobid); + if (!pjob) { + ret = false; + goto err_out; + } /* * If already deleting just return. */ - if (pjob->status == LPQ_DELETING) - return True; + if (pjob->status == LPQ_DELETING) { + ret = true; + goto err_out; + } /* Hrm - we need to be able to cope with deleting a job before it has reached the spooler. Just mark it as LPQ_DELETING and @@ -2231,8 +2266,10 @@ static bool print_job_delete1(struct tevent_context *ev, struct tdb_print_db *pdb = get_print_db_byname(sharename); int njobs = 1; - if (!pdb) - return False; + if (!pdb) { + ret = false; + goto err_out; + } pjob_delete(ev, msg_ctx, sharename, jobid); /* Ensure we keep a rough count of the number of total jobs... */ tdb_change_int32_atomic(pdb->tdb, "INFO/total_jobs", &njobs, -1); @@ -2242,7 +2279,10 @@ static bool print_job_delete1(struct tevent_context *ev, remove_from_jobs_added( sharename, jobid ); - return (result == 0); + ret = (result == 0); +err_out: + talloc_free(tmp_ctx); + return ret; } /**************************************************************************** @@ -2253,12 +2293,23 @@ static bool is_owner(const struct auth_serversupplied_info *server_info, const char *servicename, uint32 jobid) { - struct printjob *pjob = print_job_find(servicename, jobid); + struct printjob *pjob; + bool ret; + TALLOC_CTX *tmp_ctx = talloc_new(server_info); + if (tmp_ctx == NULL) { + return false; + } - if (!pjob || !server_info) - return False; + pjob = print_job_find(tmp_ctx, servicename, jobid); + if (!pjob || !server_info) { + ret = false; + goto err_out; + } - return strequal(pjob->user, server_info->sanitized_username); + ret = strequal(pjob->user, server_info->sanitized_username); +err_out: + talloc_free(tmp_ctx); + return ret; } /**************************************************************************** @@ -2272,7 +2323,11 @@ WERROR print_job_delete(const struct auth_serversupplied_info *server_info, const char* sharename = lp_const_servicename(snum); struct printjob *pjob; bool owner; - char *fname; + WERROR werr; + TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); + if (tmp_ctx == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } owner = is_owner(server_info, lp_const_servicename(snum), jobid); @@ -2292,7 +2347,8 @@ pause, or resume print job. User name: %s. Printer name: %s.", lp_printername(snum) ); /* END_ADMIN_LOG */ - return WERR_ACCESS_DENIED; + werr = WERR_ACCESS_DENIED; + goto err_out; } /* @@ -2302,18 +2358,20 @@ pause, or resume print job. User name: %s. Printer name: %s.", * spool file & return. */ - fname = print_job_fname(sharename, jobid); - if (fname != NULL) { - /* remove the spool file */ - DEBUG(10, ("print_job_delete: " - "Removing spool file [%s]\n", fname)); - if (unlink(fname) == -1) { - return map_werror_from_unix(errno); + pjob = print_job_find(tmp_ctx, sharename, jobid); + if (!pjob || pjob->spooled || pjob->pid != getpid()) { + DEBUG(10, ("Skipping spool file removal for job %u\n", jobid)); + } else { + DEBUG(10, ("Removing spool file [%s]\n", pjob->filename)); + if (unlink(pjob->filename) == -1) { + werr = map_werror_from_unix(errno); + goto err_out; } } if (!print_job_delete1(server_event_context(), msg_ctx, snum, jobid)) { - return WERR_ACCESS_DENIED; + werr = WERR_ACCESS_DENIED; + goto err_out; } /* force update the database and say the delete failed if the @@ -2321,12 +2379,16 @@ pause, or resume print job. User name: %s. Printer name: %s.", print_queue_update(msg_ctx, snum, True); - pjob = print_job_find(sharename, jobid); + pjob = print_job_find(tmp_ctx, sharename, jobid); if (pjob && (pjob->status != LPQ_DELETING)) { - return WERR_ACCESS_DENIED; + werr = WERR_ACCESS_DENIED; + goto err_out; } + werr = WERR_PRINTER_HAS_JOBS_QUEUED; - return WERR_PRINTER_HAS_JOBS_QUEUED; +err_out: + talloc_free(tmp_ctx); + return werr; } /**************************************************************************** @@ -2342,9 +2404,12 @@ WERROR print_job_pause(const struct auth_serversupplied_info *server_info, int ret = -1; struct printif *current_printif = get_printer_fns( snum ); WERROR werr; + TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); + if (tmp_ctx == NULL) { + return WERR_NOT_ENOUGH_MEMORY; + } - pjob = print_job_find(sharename, jobid); - + pjob = print_job_find(tmp_ctx, sharename, jobid); if (!pjob || !server_info) { DEBUG(10, ("print_job_pause: no pjob or user for jobid %u\n", (unsigned int)jobid )); @@ -2395,6 +2460,7 @@ pause, or resume print job. User name: %s. Printer name: %s.", /* how do we tell if this succeeded? */ werr = WERR_OK; err_out: + talloc_free(tmp_ctx); return werr; } @@ -2411,9 +2477,11 @@ WERROR print_job_resume(const struct auth_serversupplied_info *server_info, int ret; struct printif *current_printif = get_printer_fns( snum ); WERROR werr; + TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); + if (tmp_ctx == NULL) + return WERR_NOT_ENOUGH_MEMORY; - pjob = print_job_find(sharename, jobid); - + pjob = print_job_find(tmp_ctx, sharename, jobid); if (!pjob || !server_info) { DEBUG(10, ("print_job_resume: no pjob or user for jobid %u\n", (unsigned int)jobid )); @@ -2461,6 +2529,7 @@ pause, or resume print job. User name: %s. Printer name: %s.", werr = WERR_OK; err_out: + talloc_free(tmp_ctx); return werr; } @@ -2475,26 +2544,36 @@ ssize_t print_job_write(struct tevent_context *ev, const char* sharename = lp_const_servicename(snum); ssize_t return_code; struct printjob *pjob; + TALLOC_CTX *tmp_ctx = talloc_new(ev); + if (tmp_ctx == NULL) { + return -1; + } - pjob = print_job_find(sharename, jobid); + pjob = print_job_find(tmp_ctx, sharename, jobid); + if (!pjob) { + return_code = -1; + goto err_out; + } - if (!pjob) - return -1; /* don't allow another process to get this info - it is meaningless */ - if (pjob->pid != sys_getpid()) - return -1; + if (pjob->pid != sys_getpid()) { + return_code = -1; + goto err_out; + } /* if SMBD is spooling this can't be allowed */ if (pjob->status == PJOB_SMBD_SPOOLING) { - return -1; + return_code = -1; + goto err_out; } return_code = write_data(pjob->fd, buf, size); - - if (return_code>0) { + if (return_code > 0) { pjob->size += size; pjob_store(ev, msg_ctx, sharename, jobid, pjob); } +err_out: + talloc_free(tmp_ctx); return return_code; } @@ -2906,16 +2985,24 @@ void print_job_endpage(struct messaging_context *msg_ctx, { const char* sharename = lp_const_servicename(snum); struct printjob *pjob; - - pjob = print_job_find(sharename, jobid); - if (!pjob) + TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); + if (tmp_ctx == NULL) { return; + } + + pjob = print_job_find(tmp_ctx, sharename, jobid); + if (!pjob) { + goto err_out; + } /* don't allow another process to get this info - it is meaningless */ - if (pjob->pid != sys_getpid()) - return; + if (pjob->pid != sys_getpid()) { + goto err_out; + } pjob->page_count++; pjob_store(server_event_context(), msg_ctx, sharename, jobid, pjob); +err_out: + talloc_free(tmp_ctx); } /**************************************************************************** @@ -2931,17 +3018,22 @@ NTSTATUS print_job_end(struct messaging_context *msg_ctx, int snum, struct printjob *pjob; int ret; SMB_STRUCT_STAT sbuf; - struct printif *current_printif = get_printer_fns( snum ); + struct printif *current_printif = get_printer_fns(snum); NTSTATUS status = NT_STATUS_UNSUCCESSFUL; + TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); + if (tmp_ctx == NULL) { + return NT_STATUS_NO_MEMORY; + } - pjob = print_job_find(sharename, jobid); - + pjob = print_job_find(tmp_ctx, sharename, jobid); if (!pjob) { - return NT_STATUS_PRINT_CANCELLED; + status = NT_STATUS_PRINT_CANCELLED; + goto err_out; } if (pjob->spooled || pjob->pid != sys_getpid()) { - return NT_STATUS_ACCESS_DENIED; + status = NT_STATUS_ACCESS_DENIED; + goto err_out; } if (close_type == NORMAL_CLOSE || close_type == SHUTDOWN_CLOSE) { @@ -3021,6 +3113,8 @@ fail: pjob->fd = -1; unlink(pjob->filename); pjob_delete(server_event_context(), msg_ctx, sharename, jobid); +err_out: + talloc_free(tmp_ctx); return status; } @@ -3043,6 +3137,10 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, int max_reported_jobs = lp_max_reported_jobs(snum); bool ret = False; const char* sharename = lp_servicename(snum); + TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); + if (tmp_ctx == NULL) { + return false; + } /* make sure the database is up to date */ if (print_cache_expired(lp_const_servicename(snum), True)) @@ -3109,7 +3207,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, jobid = IVAL(cgdata.dptr, i*4); DEBUG(5,("get_stored_queue_info: added job = %u\n", (unsigned int)jobid)); - pjob = print_job_find(lp_const_servicename(snum), jobid); + pjob = print_job_find(tmp_ctx, lp_const_servicename(snum), jobid); if (!pjob) { DEBUG(5,("get_stored_queue_info: failed to find added job = %u\n", (unsigned int)jobid)); remove_from_jobs_added(sharename, jobid); @@ -3125,6 +3223,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, fstrcpy(queue[total_count].fs_user, pjob->user); fstrcpy(queue[total_count].fs_file, pjob->jobname); total_count++; + talloc_free(pjob); } /* Update the changed jobids. */ @@ -3146,7 +3245,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, DEBUG(5,("get_stored_queue_info: changed job: %u\n", (unsigned int) jobid)); - pjob = print_job_find(sharename, jobid); + pjob = print_job_find(tmp_ctx, sharename, jobid); if (pjob == NULL) { DEBUG(5,("get_stored_queue_info: failed to find " "changed job = %u\n", @@ -3163,6 +3262,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, queue[j].time = pjob->starttime; fstrcpy(queue[j].fs_user, pjob->user); fstrcpy(queue[j].fs_file, pjob->jobname); + talloc_free(pjob); DEBUG(5,("get_stored_queue_info: updated queue[%u], jobid: %u, jobname: %s\n", (unsigned int) j, (unsigned int) jobid, pjob->jobname)); @@ -3190,6 +3290,7 @@ static bool get_stored_queue_info(struct messaging_context *msg_ctx, SAFE_FREE(data.dptr); SAFE_FREE(cgdata.dptr); + talloc_free(tmp_ctx); return ret; } diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c index 217eae9..a724aee 100644 --- a/source4/torture/rpc/spoolss.c +++ b/source4/torture/rpc/spoolss.c @@ -7752,18 +7752,17 @@ static bool test_print_test_smbd(struct torture_context *tctx, torture_comment(tctx, "Testing smbd job spooling\n"); lpcfg_smbcli_options(tctx->lp_ctx, &options); - status = smb2_connect_ext(mem_ctx, - torture_setting_string(tctx, "host", NULL), - lpcfg_smb_ports(tctx->lp_ctx), - share, - lpcfg_resolve_context(tctx->lp_ctx), - credentials, - 0, - &tree, - tctx->ev, - &options, - lpcfg_socket_options(tctx->lp_ctx), - lpcfg_gensec_settings(tctx, tctx->lp_ctx)); + status = smb2_connect(tctx, + torture_setting_string(tctx, "host", NULL), + lpcfg_smb_ports(tctx->lp_ctx), + share, + lpcfg_resolve_context(tctx->lp_ctx), + credentials, + &tree, + tctx->ev, + &options, + lpcfg_socket_options(tctx->lp_ctx), + lpcfg_gensec_settings(tctx, tctx->lp_ctx)); if (!NT_STATUS_IS_OK(status)) { printf("Failed to connect to SMB2 printer %s - %s\n", share, nt_errstr(status)); -- 1.7.1 From 2255b131b73c63725248b8de1c950ec943ff1092 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 8 Feb 2012 17:57:02 +0100 Subject: [PATCH 12/17] s3-printing: pass a talloc ctx to unpack_pjob Rather than allocating the devicemode on a null context. --- source3/printing/printing.c | 38 +++++++++++++++------------ source3/rpc_server/spoolss/srv_spoolss_nt.c | 2 +- 2 files changed, 22 insertions(+), 18 deletions(-) diff --git a/source3/printing/printing.c b/source3/printing/printing.c index c300c8b..8ce2561 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -373,8 +373,8 @@ done: unpack a pjob from a tdb buffer ***********************************************************************/ -/* FIXME talloc ctx */ -static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob) +static int unpack_pjob(TALLOC_CTX *mem_ctx, uint8 *buf, int buflen, + struct printjob *pjob) { int len = 0; int used; @@ -406,7 +406,7 @@ static int unpack_pjob(uint8 *buf, int buflen, struct printjob *pjob) return -1; } - used = unpack_devicemode(NULL, buf+len, buflen-len, &pjob->devmode); + used = unpack_devicemode(mem_ctx, buf+len, buflen-len, &pjob->devmode); if (used == -1) { return -1; } @@ -462,7 +462,7 @@ static struct printjob *print_job_find(TALLOC_CTX *mem_ctx, goto err_out; } - if (unpack_pjob(ret.dptr, ret.dsize, pjob) == -1) { + if (unpack_pjob(mem_ctx, ret.dptr, ret.dsize, pjob) == -1) { DEBUG(10, ("failed to unpack jobid %u.\n", jobid)); talloc_free(pjob); pjob = NULL; @@ -797,30 +797,32 @@ static bool pjob_store(struct tevent_context *ev, /* Send notify updates for what has changed */ - if ( ret ) { + if (ret) { bool changed = false; struct printjob old_pjob; - if ( old_data.dsize ) - { - if ( unpack_pjob( old_data.dptr, old_data.dsize, &old_pjob ) != -1 ) - { - pjob_store_notify(server_event_context(), + if (old_data.dsize) { + TALLOC_CTX *tmp_ctx = talloc_new(ev); + if (tmp_ctx == NULL) + goto done; + + len = unpack_pjob(tmp_ctx, old_data.dptr, + old_data.dsize, &old_pjob); + if (len != -1 ) { + pjob_store_notify(ev, msg_ctx, sharename, jobid, &old_pjob, pjob, &changed); - talloc_free(old_pjob.devmode); - if (changed) { add_to_jobs_changed(pdb, jobid); } } + talloc_free(tmp_ctx); - } - else { + } else { /* new job */ - pjob_store_notify(server_event_context(), msg_ctx, + pjob_store_notify(ev, msg_ctx, sharename, jobid, NULL, pjob, &changed); } @@ -939,6 +941,7 @@ struct traverse_struct { struct printif *print_if; struct tevent_context *ev; struct messaging_context *msg_ctx; + TALLOC_CTX *mem_ctx; }; /**************************************************************************** @@ -955,7 +958,7 @@ static int traverse_fn_delete(TDB_CONTEXT *t, TDB_DATA key, TDB_DATA data, void if ( key.dsize != sizeof(jobid) ) return 0; - if (unpack_pjob(data.dptr, data.dsize, &pjob) == -1) + if (unpack_pjob(ts->mem_ctx, data.dptr, data.dsize, &pjob) == -1) return 0; talloc_free(pjob.devmode); jobid = pjob.jobid; @@ -1436,7 +1439,6 @@ static void print_queue_update_internal(struct tevent_context *ev, } SAFE_FREE(jcdata.dptr); - talloc_free(tmp_ctx); /* now delete any queued entries that don't appear in the system queue */ @@ -1450,6 +1452,7 @@ static void print_queue_update_internal(struct tevent_context *ev, tstruct.print_if = current_printif; tstruct.ev = ev; tstruct.msg_ctx = msg_ctx; + tstruct.mem_ctx = tmp_ctx; tdb_traverse(pdb->tdb, traverse_fn_delete, (void *)&tstruct); @@ -1457,6 +1460,7 @@ static void print_queue_update_internal(struct tevent_context *ev, store_queue_struct(pdb, &tstruct); SAFE_FREE(tstruct.queue); + talloc_free(tmp_ctx); DEBUG(10,("print_queue_update_internal: printer %s INFO/total_jobs = %d\n", sharename, tstruct.total_jobs )); diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c index e0f5be5..8868a98 100644 --- a/source3/rpc_server/spoolss/srv_spoolss_nt.c +++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c @@ -9077,7 +9077,7 @@ static WERROR getjob_level_2(TALLOC_CTX *mem_ctx, * a failure condition */ - devmode = print_job_devmode(lp_const_servicename(snum), jobid); + devmode = print_job_devmode(mem_ctx, lp_const_servicename(snum), jobid); if (!devmode) { result = spoolss_create_default_devmode(mem_ctx, pinfo2->printername, -- 1.7.1 From eef01a7fba3f2fc825fbafda767861f788f95533 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 8 Feb 2012 17:03:06 +0100 Subject: [PATCH 13/17] s3-printing: remove unused print_job_fname() --- source3/include/printing.h | 1 - source3/printing/printing.c | 17 +---------------- 2 files changed, 1 insertions(+), 17 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index a49d6a5..149e84c 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -195,7 +195,6 @@ uint32 sysjob_to_jobid(int unix_jobid); bool print_notify_register_pid(int snum); bool print_notify_deregister_pid(int snum); bool print_job_exists(const char* sharename, uint32 jobid); -char *print_job_fname(const char* sharename, uint32 jobid); struct spoolss_DeviceMode *print_job_devmode(TALLOC_CTX *mem_ctx, const char *sharename, uint32 jobid); diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 8ce2561..9ac808a 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -2073,22 +2073,7 @@ bool print_job_exists(const char* sharename, uint32 jobid) } /**************************************************************************** - Give the filename used for a jobid. - Only valid for the process doing the spooling and when the job - has not been spooled. -****************************************************************************/ - -char *print_job_fname(const char* sharename, uint32 jobid) -{ - struct printjob *pjob = print_job_find(NULL, sharename, jobid); - if (!pjob || pjob->spooled || pjob->pid != sys_getpid()) - return NULL; - return pjob->filename; -} - - -/**************************************************************************** - Give the filename used for a jobid. + Return the device mode asigned to a specific print job. Only valid for the process doing the spooling and when the job has not been spooled. ****************************************************************************/ -- 1.7.1 From aa8925accd57c9c3638bf721252a6596d494253e Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Wed, 8 Feb 2012 18:47:11 +0100 Subject: [PATCH 14/17] s3-printing: pass lpq command to job_submit Currently the generic print backend does not fill the printing backend job identifier (sysjob) on submission of a new job. The sysjob identifier is required to correctly map jobs in the printer queue to corresponding spoolss print jobs. Passing the lpq command to job_submit allows the generic print backend to check the printer queue for the new job following submission. This behaviour will come in a later commit. --- source3/include/printing.h | 4 +++- source3/printing/print_cups.c | 4 +++- source3/printing/print_generic.c | 4 +++- source3/printing/print_iprint.c | 4 +++- source3/printing/printing.c | 26 +++++++++++++++++++++++++- 5 files changed, 37 insertions(+), 5 deletions(-) diff --git a/source3/include/printing.h b/source3/include/printing.h index 149e84c..c511fa2 100644 --- a/source3/include/printing.h +++ b/source3/include/printing.h @@ -101,7 +101,9 @@ struct printif int (*job_delete)(const char *sharename, const char *lprm_command, struct printjob *pjob); int (*job_pause)(int snum, struct printjob *pjob); int (*job_resume)(int snum, struct printjob *pjob); - int (*job_submit)(int snum, struct printjob *pjob); + int (*job_submit)(int snum, struct printjob *pjob, + enum printing_types printing_type, + char *lpq_command); }; extern struct printif generic_printif; diff --git a/source3/printing/print_cups.c b/source3/printing/print_cups.c index 8f0cd88..9f23866 100644 --- a/source3/printing/print_cups.c +++ b/source3/printing/print_cups.c @@ -852,7 +852,9 @@ static int cups_job_resume(int snum, struct printjob *pjob) * 'cups_job_submit()' - Submit a job for printing. */ -static int cups_job_submit(int snum, struct printjob *pjob) +static int cups_job_submit(int snum, struct printjob *pjob, + enum printing_types printing_type, + char *lpq_cmd) { TALLOC_CTX *frame = talloc_stackframe(); int ret = 1; /* Return value */ diff --git a/source3/printing/print_generic.c b/source3/printing/print_generic.c index b925bed..228636a 100644 --- a/source3/printing/print_generic.c +++ b/source3/printing/print_generic.c @@ -142,7 +142,9 @@ static int generic_job_resume(int snum, struct printjob *pjob) Submit a file for printing - called from print_job_end() ****************************************************************************/ -static int generic_job_submit(int snum, struct printjob *pjob) +static int generic_job_submit(int snum, struct printjob *pjob, + enum printing_types printing_type, + char *lpq_cmd) { int ret = -1; char *current_directory = NULL; diff --git a/source3/printing/print_iprint.c b/source3/printing/print_iprint.c index 9e741c1..5ca77f9 100644 --- a/source3/printing/print_iprint.c +++ b/source3/printing/print_iprint.c @@ -722,7 +722,9 @@ static int iprint_job_resume(int snum, struct printjob *pjob) * 'iprint_job_submit()' - Submit a job for printing. */ -static int iprint_job_submit(int snum, struct printjob *pjob) +static int iprint_job_submit(int snum, struct printjob *pjob, + enum printing_types printing_type, + char *lpq_cmd) { int ret = 1; /* Return value */ http_t *http = NULL; /* HTTP connection to server */ diff --git a/source3/printing/printing.c b/source3/printing/printing.c index 9ac808a..98b6e89 100644 --- a/source3/printing/printing.c +++ b/source3/printing/printing.c @@ -3009,6 +3009,7 @@ NTSTATUS print_job_end(struct messaging_context *msg_ctx, int snum, SMB_STRUCT_STAT sbuf; struct printif *current_printif = get_printer_fns(snum); NTSTATUS status = NT_STATUS_UNSUCCESSFUL; + char *lpq_cmd; TALLOC_CTX *tmp_ctx = talloc_new(msg_ctx); if (tmp_ctx == NULL) { return NT_STATUS_NO_MEMORY; @@ -3076,8 +3077,31 @@ NTSTATUS print_job_end(struct messaging_context *msg_ctx, int snum, return NT_STATUS_OK; } - ret = (*(current_printif->job_submit))(snum, pjob); + /* don't strip out characters like '$' from the printername */ + lpq_cmd = talloc_string_sub2(tmp_ctx, + lp_lpqcommand(snum), + "%p", + lp_printername(snum), + false, false, false); + if (lpq_cmd == NULL) { + status = NT_STATUS_PRINT_CANCELLED; + goto fail; + } + lpq_cmd = talloc_sub_advanced(tmp_ctx, + lp_servicename(snum), + current_user_info.unix_name, + "", + current_user.ut.gid, + get_current_username(), + current_user_info.domain, + lpq_cmd); + if (lpq_cmd == NULL) { + status = NT_STATUS_PRINT_CANCELLED; + goto fail; + } + ret = (*(current_printif->job_submit))(snum, pjob, + current_printif->type, lpq_cmd); if (ret) { status = NT_STATUS_PRINT_CANCELLED; goto fail; -- 1.7.1 From c3cef25b599caab4f581fe898d52416d6fcc60d7 Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Thu, 9 Feb 2012 12:08:27 +0100 Subject: [PATCH 15/17] s3-printing: fill print_generic sysjob id on job submission Change the generic print backend to fill the printing backend job identifier (sysjob) on submission of a new job. This is needed to ensure correct mapping of spoolss jobs and entries in the backend print queue. This and the last 13 commits attempt to address bug 8719. --- source3/printing/print_generic.c | 145 +++++++++++++++++++++++--------------- 1 files changed, 88 insertions(+), 57 deletions(-) diff --git a/source3/printing/print_generic.c b/source3/printing/print_generic.c index 228636a..aac3892 100644 --- a/source3/printing/print_generic.c +++ b/source3/printing/print_generic.c @@ -139,6 +139,62 @@ static int generic_job_resume(int snum, struct printjob *pjob) } /**************************************************************************** +get the current list of queued jobs +****************************************************************************/ +static int generic_queue_get(const char *printer_name, + enum printing_types printing_type, + char *lpq_command, + print_queue_struct **q, + print_status_struct *status) +{ + char **qlines; + int fd; + int numlines, i, qcount; + print_queue_struct *queue = NULL; + + /* never do substitution when running the 'lpq command' since we can't + get it rigt when using the background update daemon. Make the caller + do it before passing off the command string to us here. */ + + print_run_command(-1, printer_name, False, lpq_command, &fd, NULL); + + if (fd == -1) { + DEBUG(5,("generic_queue_get: Can't read print queue status for printer %s\n", + printer_name )); + return 0; + } + + numlines = 0; + qlines = fd_lines_load(fd, &numlines,0,NULL); + close(fd); + + /* turn the lpq output into a series of job structures */ + qcount = 0; + ZERO_STRUCTP(status); + if (numlines && qlines) { + queue = SMB_MALLOC_ARRAY(print_queue_struct, numlines+1); + if (!queue) { + TALLOC_FREE(qlines); + *q = NULL; + return 0; + } + memset(queue, '\0', sizeof(print_queue_struct)*(numlines+1)); + + for (i=0; isysjob = -1; + ret = generic_queue_get(lp_printername(snum), printing_type, lpq_cmd, + &q, &status); + if (ret > 0) { + int i; + for (i = 0; i < ret; i++) { + if (strcmp(q[i].fs_file, p) == 0) { + pjob->sysjob = q[i].sysjob; + DEBUG(5, ("new job %u (%s) matches sysjob %d\n", + pjob->jobid, jobname, pjob->sysjob)); + break; + } + } + SAFE_FREE(q); + ret = 0; + } + if (pjob->sysjob == -1) { + DEBUG(0, ("failed to get sysjob for job %u (%s), tracking as " + "Unix job\n", pjob->jobid, jobname)); + } + out: @@ -214,63 +302,6 @@ static int generic_job_submit(int snum, struct printjob *pjob, return ret; } - -/**************************************************************************** -get the current list of queued jobs -****************************************************************************/ -static int generic_queue_get(const char *printer_name, - enum printing_types printing_type, - char *lpq_command, - print_queue_struct **q, - print_status_struct *status) -{ - char **qlines; - int fd; - int numlines, i, qcount; - print_queue_struct *queue = NULL; - - /* never do substitution when running the 'lpq command' since we can't - get it rigt when using the background update daemon. Make the caller - do it before passing off the command string to us here. */ - - print_run_command(-1, printer_name, False, lpq_command, &fd, NULL); - - if (fd == -1) { - DEBUG(5,("generic_queue_get: Can't read print queue status for printer %s\n", - printer_name )); - return 0; - } - - numlines = 0; - qlines = fd_lines_load(fd, &numlines,0,NULL); - close(fd); - - /* turn the lpq output into a series of job structures */ - qcount = 0; - ZERO_STRUCTP(status); - if (numlines && qlines) { - queue = SMB_MALLOC_ARRAY(print_queue_struct, numlines+1); - if (!queue) { - TALLOC_FREE(qlines); - *q = NULL; - return 0; - } - memset(queue, '\0', sizeof(print_queue_struct)*(numlines+1)); - - for (i=0; i Date: Thu, 21 Jun 2012 15:49:55 +0200 Subject: [PATCH 16/17] s3-printing: use euid for vlp job tracking vlp can be called by print_run_command as root with euids set appropriately, vlp should use this to track the job owner. --- source3/printing/tests/vlp.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/source3/printing/tests/vlp.c b/source3/printing/tests/vlp.c index b220506..f02e0c5 100644 --- a/source3/printing/tests/vlp.c +++ b/source3/printing/tests/vlp.c @@ -237,7 +237,8 @@ static int print_command(int argc, char **argv) slprintf(job.jobname, sizeof(job.jobname) - 1, "%s", argv[2]); - if (!(pw = getpwuid(getuid()))) { + if (!(pw = getpwuid(geteuid()))) { + printf("getpwuid failed\n"); return 1; } -- 1.7.1 From 04f2d2e4e97463a8aeab3ba5197d5d9efbfabf0f Mon Sep 17 00:00:00 2001 From: David Disseldorp Date: Fri, 22 Jun 2012 18:49:50 +0200 Subject: [PATCH 17/17] s3-torture: Use static printer for smbd spooler test --- source4/torture/rpc/spoolss.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c index a724aee..fd065fc 100644 --- a/source4/torture/rpc/spoolss.c +++ b/source4/torture/rpc/spoolss.c @@ -54,6 +54,7 @@ #define TORTURE_DRIVER_TIMESTAMPS "torture_driver_timestamps" #define TORTURE_DRIVER_DELETER "torture_driver_deleter" #define TORTURE_DRIVER_DELETERIN "torture_driver_deleterin" +#define TORTURE_PRINTER_STATIC1 "print1" #define TOP_LEVEL_PRINT_KEY "SOFTWARE\\Microsoft\\Windows NT\\CurrentVersion\\Print" #define TOP_LEVEL_PRINT_PRINTERS_KEY TOP_LEVEL_PRINT_KEY "\\Printers" @@ -7747,7 +7748,14 @@ static bool test_print_test_smbd(struct torture_context *tctx, struct cli_credentials *credentials = cmdline_credentials; struct smbcli_options options; TALLOC_CTX *mem_ctx = talloc_new(tctx); - const char *share = t->info2.printername; + /* + * Do not test against the dynamically added printers, printing via + * smbd means that a different spoolss process may handle the + * OpenPrinter request to the one that handled the AddPrinter request. + * This currently leads to an ugly race condition where one process + * sees the new printer and one doesn't. + */ + const char *share = TORTURE_PRINTER_STATIC1; torture_comment(tctx, "Testing smbd job spooling\n"); lpcfg_smbcli_options(tctx->lp_ctx, &options); -- 1.7.1