Bug 9433 - smbd/spoolss segfault with default devmode = no
smbd/spoolss segfault with default devmode = no
Status: RESOLVED FIXED
Product: Samba 4.0
Classification: Unclassified
Component: printing
unspecified
All All
: P5 normal
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-11-27 14:21 UTC by David Disseldorp
Modified: 2012-11-30 10:40 UTC (History)
1 user (show)

See Also:


Attachments
proposed fix for master and v4-0-test (3.19 KB, patch)
2012-11-27 15:18 UTC, David Disseldorp
asn: review+
ddiss: review? (gd)
Details
proposed fix for v3-6-test (3.19 KB, patch)
2012-11-27 15:46 UTC, David Disseldorp
asn: review+
Details
tagged fix for v3-6-test (3.24 KB, patch)
2012-11-29 13:01 UTC, David Disseldorp
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2012-11-27 14:21:31 UTC
Currently when "default devmode" is explicitly disabled, and a printer is added with a null device mode, spoolssd crashes in copy_devicemode().

Both construct_printer_info2() and construct_printer_info8() code paths currently unconditionally attempt to copy a printers device mode, without checking whether one is present.

This is easily reproducible using the samba3.rpc.spoolss test suite with the following patch:

diff --git a/selftest/target/Samba3.pm b/selftest/target/Samba3.pm
index 5c86612..515db71 100755
--- a/selftest/target/Samba3.pm
+++ b/selftest/target/Samba3.pm
@@ -989,6 +989,7 @@ sub provision($$$$$$)
 [print1]
        copy = tmp
        printable = yes
+       default devmode = no
 
 [print2]
        copy = print1
Comment 1 David Disseldorp 2012-11-27 15:18:52 UTC
Created attachment 8231 [details]
proposed fix for master and v4-0-test
Comment 2 David Disseldorp 2012-11-27 15:46:46 UTC
Created attachment 8232 [details]
proposed fix for v3-6-test

minor change for sys_usleep in the 3.6 branch.
Comment 3 Andreas Schneider 2012-11-27 17:18:17 UTC
Could we add new printer share and add a new test which runs on that share without the default devmode?
Comment 4 David Disseldorp 2012-11-27 17:20:18 UTC
(In reply to comment #3)
> Could we add new printer share and add a new test which runs on that share
> without the default devmode?

Sure, good idea. Patch to follow...
Comment 5 Andreas Schneider 2012-11-29 10:21:24 UTC
Comment on attachment 8231 [details]
proposed fix for master and v4-0-test

Looks fine pushing to autobuild for master.
Comment 6 David Disseldorp 2012-11-29 11:35:13 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Could we add new printer share and add a new test which runs on that share
> > without the default devmode?
> 
> Sure, good idea. Patch to follow...

(In reply to comment #4)
> (In reply to comment #3)
> > Could we add new printer share and add a new test which runs on that share
> > without the default devmode?
> 
> Sure, good idea. Patch to follow...

We hit the segfault when doing a level 2 EnumPrinters where any printer is configured with "default devmode = no" is present, so simply adding the option to [print3] (not currently used explicitly) is sufficient.


Andreas, please review and push if zufrieden, I've snuck in a couple of extra trivial changes:

The following changes since commit b533fbc83617a5ba81d1c53309027be296901314:

  spoolss: fix segfault when "default devmode" is disabled (2012-11-29 11:22:32 +0100)

are available in the git repository at:
  git://git.samba.org/ddiss/samba.git bnc791183_def_devmode_segv_qa

David Disseldorp (3):
      selftest: configure printer with default devmode = no
      rpcclient: fix usage docs for rpcclient adddriver
      s3-printing: add missing carriage return to debug str

 docs-xml/manpages/rpcclient.1.xml |    2 +-
 selftest/target/Samba3.pm         |    1 +
 source3/printing/printing.c       |    2 +-
 source3/rpcclient/cmd_spoolss.c   |    4 ++--
 4 files changed, 5 insertions(+), 4 deletions(-)
Comment 7 David Disseldorp 2012-11-29 13:01:03 UTC
Created attachment 8240 [details]
tagged fix for v3-6-test
Comment 8 Andreas Schneider 2012-11-29 13:03:24 UTC
Comment on attachment 8240 [details]
tagged fix for v3-6-test

Looks good.
Comment 9 David Disseldorp 2012-11-29 13:27:31 UTC
Karolin, could you please cherry-pick 2e12deedcfdc5ce3637a125b083b0f00b208bf61 to the v4-0-test branch, and apply "tagged fix for v3-6-test" to the v3-6-test branch. Thanks!
Comment 10 Karolin Seeger 2012-11-30 08:23:26 UTC
Pushed to v3-6-test and autobuild-v4-0-test.
Comment 11 Karolin Seeger 2012-11-30 10:40:02 UTC
Pushed to v4-0-test.
Closing out bug report.

Thanks!