The Samba-Bugzilla – Attachment 10551 Details for
Bug 10984
spoolss IDL response marshalling fails when returning error without clearing info
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patchset for master
bso10984_spoolss_marshal_info_on_error_master.patchset (text/plain), 13.51 KB, created by
David Disseldorp
on 2014-12-18 19:14:22 UTC
(
hide
)
Description:
patchset for master
Filename:
MIME Type:
Creator:
David Disseldorp
Created:
2014-12-18 19:14:22 UTC
Size:
13.51 KB
patch
obsolete
>From 4f569f8ac4b8fd03d99f60520b647b3ec3609414 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Thu, 4 Dec 2014 20:03:39 +0100 >Subject: [PATCH 1/7] spoolss: clear JobInfo on GetJob error > >In handling a spoolss GetJob request, the _spoolss_GetJob() handler may >return an immediate error if one of the input parameters is invalid. If >this is done without zeroing the pre-allocated @info pointer, then >api_spoolss_GetJob() will attempt to marshall @info, which in the case >of an @offered value of zero results in a marshalling error: > >ndr_push_error(7): Bad subcontext (PUSH) content_size 64 is larger >than size_is(0) > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 28 ++++++++++++++++++---------- > 1 file changed, 18 insertions(+), 10 deletions(-) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index 1226ec1..c71eb91 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -9484,7 +9484,8 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, > /* that's an [in out] buffer */ > > if (!r->in.buffer && (r->in.offered != 0)) { >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_jinfo_free; > } > > DEBUG(5,("_spoolss_GetJob\n")); >@@ -9492,12 +9493,14 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, > *r->out.needed = 0; > > if (!get_printer_snum(p, r->in.handle, &snum, NULL)) { >- return WERR_BADFID; >+ result = WERR_BADFID; >+ goto err_jinfo_free; > } > > svc_name = lp_const_servicename(snum); > if (svc_name == NULL) { >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_jinfo_free; > } > > result = winreg_get_printer_internal(p->mem_ctx, >@@ -9506,22 +9509,22 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, > svc_name, > &pinfo2); > if (!W_ERROR_IS_OK(result)) { >- return result; >+ goto err_jinfo_free; > } > > pdb = get_print_db_byname(svc_name); > if (pdb == NULL) { > DEBUG(3, ("failed to get print db for svc %s\n", svc_name)); >- TALLOC_FREE(pinfo2); >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_pinfo_free; > } > > sysjob = jobid_to_sysjob_pdb(pdb, r->in.job_id); > release_print_db(pdb); > if (sysjob == -1) { > DEBUG(3, ("no sysjob for spoolss jobid %u\n", r->in.job_id)); >- TALLOC_FREE(pinfo2); >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_pinfo_free; > } > > count = print_queue_status(p->msg_ctx, snum, &queue, &prt_status); >@@ -9551,8 +9554,7 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, > TALLOC_FREE(pinfo2); > > if (!W_ERROR_IS_OK(result)) { >- TALLOC_FREE(r->out.info); >- return result; >+ goto err_jinfo_free; > } > > *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_JobInfo, r->out.info, >@@ -9560,6 +9562,12 @@ WERROR _spoolss_GetJob(struct pipes_struct *p, > r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); > > return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); >+ >+err_pinfo_free: >+ TALLOC_FREE(pinfo2); >+err_jinfo_free: >+ TALLOC_FREE(r->out.info); >+ return result; > } > > /**************************************************************** >-- >2.1.2 > > >From 203a02fdf245714fc37c7834e691de052608db79 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 17 Dec 2014 15:21:33 +0100 >Subject: [PATCH 2/7] spoolss: clear DriverInfo on GetPrinterDriver2 error > >In handling a spoolss GetPrinterDriver2 request, the handler may >return an immediate error if one of the input parameters is invalid. >If this is done without zeroing the pre-allocated @info pointer, then >marshalling of the response will fail. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index c71eb91..9023ab6 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -5686,14 +5686,16 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, > /* that's an [in out] buffer */ > > if (!r->in.buffer && (r->in.offered != 0)) { >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_info_free; > } > > DEBUG(4,("_spoolss_GetPrinterDriver2\n")); > > if (!(printer = find_printer_index_by_hnd(p, r->in.handle))) { > DEBUG(0,("_spoolss_GetPrinterDriver2: invalid printer handle!\n")); >- return WERR_INVALID_PRINTER_NAME; >+ result = WERR_INVALID_PRINTER_NAME; >+ goto err_info_free; > } > > *r->out.needed = 0; >@@ -5701,7 +5703,8 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, > *r->out.server_minor_version = 0; > > if (!get_printer_snum(p, r->in.handle, &snum, NULL)) { >- return WERR_BADFID; >+ result = WERR_BADFID; >+ goto err_info_free; > } > > if (r->in.client_major_version == SPOOLSS_DRIVER_VERSION_2012) { >@@ -5718,8 +5721,7 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, > r->in.architecture, > version); > if (!W_ERROR_IS_OK(result)) { >- TALLOC_FREE(r->out.info); >- return result; >+ goto err_info_free; > } > > *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_DriverInfo, >@@ -5727,6 +5729,10 @@ WERROR _spoolss_GetPrinterDriver2(struct pipes_struct *p, > r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); > > return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); >+ >+err_info_free: >+ TALLOC_FREE(r->out.info); >+ return result; > } > > >-- >2.1.2 > > >From d6c43a21cde1b2b10af10d72646fe09f8db430cd Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 17 Dec 2014 15:29:52 +0100 >Subject: [PATCH 3/7] spoolss: clear FormInfo on GetForm error > >In handling a spoolss GetForm request, the handler may return an >immediate error if one of the input parameters is invalid. If this is >done without zeroing the pre-allocated @info pointer, then marshalling >of the response will fail. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index 9023ab6..9b898d0 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -7853,6 +7853,7 @@ WERROR _spoolss_GetForm(struct pipes_struct *p, > /* that's an [in out] buffer */ > > if (!r->in.buffer && (r->in.offered != 0)) { >+ TALLOC_FREE(r->out.info); > return WERR_INVALID_PARAM; > } > >-- >2.1.2 > > >From 58a23f7efe903586a1dacce9225c2d1af7d86cd7 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 17 Dec 2014 15:54:22 +0100 >Subject: [PATCH 4/7] spoolss: clear info on GetPrintProcessorDirectory error > >If an error is returned without zeroing a pre-allocated @info pointer, >then marshalling of the response will fail. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index 9b898d0..c8c670b 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -10146,7 +10146,8 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, > /* that's an [in out] buffer */ > > if (!r->in.buffer && (r->in.offered != 0)) { >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_info_free; > } > > DEBUG(5,("_spoolss_GetPrintProcessorDirectory: level %d\n", >@@ -10162,7 +10163,8 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, > > snum = find_service(talloc_tos(), "prnproc$", &prnproc_share); > if (!prnproc_share) { >- return WERR_NOMEM; >+ result = WERR_NOMEM; >+ goto err_info_free; > } > if (snum != -1) { > prnproc_share_exists = true; >@@ -10173,8 +10175,7 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, > r->in.environment, > &r->out.info->info1); > if (!W_ERROR_IS_OK(result)) { >- TALLOC_FREE(r->out.info); >- return result; >+ goto err_info_free; > } > > *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_PrintProcessorDirectoryInfo, >@@ -10182,6 +10183,10 @@ WERROR _spoolss_GetPrintProcessorDirectory(struct pipes_struct *p, > r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); > > return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); >+ >+err_info_free: >+ TALLOC_FREE(r->out.info); >+ return result; > } > > /******************************************************************* >-- >2.1.2 > > >From 2c80570d35393d1568ac83bf98587b75083afa45 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 17 Dec 2014 16:47:50 +0100 >Subject: [PATCH 5/7] spoolss: clear info on GetPrinterDriverDirectory error > >If an error is returned without zeroing a pre-allocated @info pointer, >then marshalling of the response will fail. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index c8c670b..c34b04d 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -8544,6 +8544,7 @@ WERROR _spoolss_GetPrinterDriverDirectory(struct pipes_struct *p, > /* that's an [in out] buffer */ > > if (!r->in.buffer && (r->in.offered != 0)) { >+ TALLOC_FREE(r->out.info); > return WERR_INVALID_PARAM; > } > >-- >2.1.2 > > >From 482d8e80ab64d30141e64b608c7bbb3083851dd2 Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 17 Dec 2014 16:54:42 +0100 >Subject: [PATCH 6/7] spoolss: clear PrinterInfo on GetPrinter error > >If an error is returned without zeroing a pre-allocated @info pointer, >then marshalling of the response will fail. > >Bug: https://bugzilla.samba.org/show_bug.cgi?id=10984 > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source3/rpc_server/spoolss/srv_spoolss_nt.c | 19 ++++++++++++------- > 1 file changed, 12 insertions(+), 7 deletions(-) > >diff --git a/source3/rpc_server/spoolss/srv_spoolss_nt.c b/source3/rpc_server/spoolss/srv_spoolss_nt.c >index c34b04d..115af2d 100644 >--- a/source3/rpc_server/spoolss/srv_spoolss_nt.c >+++ b/source3/rpc_server/spoolss/srv_spoolss_nt.c >@@ -4778,17 +4778,20 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, > /* that's an [in out] buffer */ > > if (!r->in.buffer && (r->in.offered != 0)) { >- return WERR_INVALID_PARAM; >+ result = WERR_INVALID_PARAM; >+ goto err_info_free; > } > > *r->out.needed = 0; > > if (Printer == NULL) { >- return WERR_BADFID; >+ result = WERR_BADFID; >+ goto err_info_free; > } > > if (!get_printer_snum(p, r->in.handle, &snum, NULL)) { >- return WERR_BADFID; >+ result = WERR_BADFID; >+ goto err_info_free; > } > > result = winreg_get_printer_internal(p->mem_ctx, >@@ -4797,7 +4800,7 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, > lp_const_servicename(snum), > &info2); > if (!W_ERROR_IS_OK(result)) { >- goto out; >+ goto err_info_free; > } > > switch (r->in.level) { >@@ -4857,12 +4860,10 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, > } > TALLOC_FREE(info2); > >- out: > if (!W_ERROR_IS_OK(result)) { > DEBUG(0, ("_spoolss_GetPrinter: failed to construct printer info level %d - %s\n", > r->in.level, win_errstr(result))); >- TALLOC_FREE(r->out.info); >- return result; >+ goto err_info_free; > } > > *r->out.needed = SPOOLSS_BUFFER_UNION(spoolss_PrinterInfo, >@@ -4870,6 +4871,10 @@ WERROR _spoolss_GetPrinter(struct pipes_struct *p, > r->out.info = SPOOLSS_BUFFER_OK(r->out.info, NULL); > > return SPOOLSS_BUFFER_OK(WERR_OK, WERR_INSUFFICIENT_BUFFER); >+ >+err_info_free: >+ TALLOC_FREE(r->out.info); >+ return result; > } > > /******************************************************************** >-- >2.1.2 > > >From f1d89e3df4448885b0c270427d45fcb25f0d733f Mon Sep 17 00:00:00 2001 >From: David Disseldorp <ddiss@samba.org> >Date: Wed, 3 Dec 2014 18:44:37 +0100 >Subject: [PATCH 7/7] torture/spoolss: issue GetJob after StartDocPrinter > >This reflects Windows XP spoolss client behaviour. This fails if the job >is not yet instantiated on the server, and prior to the bso#10984 fix >resulted in an unsable DCERPC pipe. > >Signed-off-by: David Disseldorp <ddiss@samba.org> >--- > source4/torture/rpc/spoolss.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > >diff --git a/source4/torture/rpc/spoolss.c b/source4/torture/rpc/spoolss.c >index 20e86fc..23f501d 100644 >--- a/source4/torture/rpc/spoolss.c >+++ b/source4/torture/rpc/spoolss.c >@@ -3446,6 +3446,9 @@ static bool test_DoPrintTest_add_one_job_common(struct torture_context *tctx, > torture_assert_werr_ok(tctx, s.out.result, "StartDocPrinter failed"); > > for (i=1; i < 4; i++) { >+ union spoolss_JobInfo ginfo; >+ bool ok; >+ > torture_comment(tctx, "Testing StartPagePrinter: Page[%d], JobId[%d]\n", i, *job_id); > > sp.in.handle = handle; >@@ -3455,6 +3458,11 @@ static bool test_DoPrintTest_add_one_job_common(struct torture_context *tctx, > "dcerpc_spoolss_StartPagePrinter failed"); > torture_assert_werr_ok(tctx, sp.out.result, "StartPagePrinter failed"); > >+ ok = test_GetJob_args(tctx, b, handle, *job_id, 1, &ginfo); >+ if (!ok) { >+ torture_comment(tctx, "test_GetJob failed for JobId[%d]\n", *job_id); >+ } >+ > torture_comment(tctx, "Testing WritePrinter: Page[%d], JobId[%d]\n", i, *job_id); > > w.in.handle = handle; >-- >2.1.2 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=10551">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Actions:
View
Attachments on
bug 10984
: 10551 |
10592
|
10593