Bug 8762 - crash in printer_list.c tdb_pack
crash in printer_list.c tdb_pack
Status: RESOLVED FIXED
Product: Samba 3.6
Classification: Unclassified
Component: Printing
3.6.3
All All
: P5 major
: ---
Assigned To: Karolin Seeger
Samba QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-02-15 14:16 UTC by David Disseldorp
Modified: 2012-05-08 07:01 UTC (History)
0 users

See Also:


Attachments
fix based on v3-6-test (1.08 KB, patch)
2012-02-15 14:47 UTC, David Disseldorp
gd: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2012-02-15 14:16:08 UTC
The crash occurs when packing the printcap information for storage in the
printer_list tdb:

#0  0x00000fff84028220 in .raise () from /lib64/libc.so.6
#1  0x00000fff84029e30 in .abort () from /lib64/libc.so.6
#2  0x0000000042432734 in dump_core () at lib/fault.c:391
#3  0x0000000042448db4 in smb_panic (why=<optimized out>) at lib/util.c:1133
#4  0x0000000042432d80 in fault_report (sig=<optimized out>) at lib/fault.c:53
#5  sig_fault (sig=11) at lib/fault.c:76
#6  <signal handler called>
#7  0x00000fff8407a7e0 in .strlen () from /lib64/libc.so.6
#8  0x00000000424238d0 in tdb_pack_va (buf=0x42b57d72 "", bufsize=1,
fmt=0x428c9af5 "", ap=0xfffdfa64350 "") at lib/util_tdb.c:171
#9  0x00000000424239e8 in tdb_pack (buf=<optimized out>, bufsize=<optimized
out>, fmt=<optimized out>) at lib/util_tdb.c:218
#10 0x000000004240007c in printer_list_set_printer (mem_ctx=<optimized out>,
name=0x42b754c0 "pear_printer", comment=0x42b75880 "pear_printer",
location=<optimized out>, 
    last_refresh=<optimized out>) at printing/printer_list.c:193
#11 0x00000000423f646c in pcap_cache_add (name=0x42b754c0 "pear_printer",
comment=0x42b75880 "pear_printer", location=0x0) at printing/pcap.c:91
#12 0x00000000423f6500 in pcap_cache_replace (pcache=0x42b72fe0) at
printing/pcap.c:116
#13 0x00000000423f9338 in cups_async_callback (event_ctx=<optimized out>,
event=<optimized out>, flags=<optimized out>, p=0x42b6e330) at
printing/print_cups.c:503
#14 0x000000004245e1c4 in run_events_poll (ev=0x42b55460, pollrtn=<optimized
out>, pfds=0x42b70710, num_pfds=6) at lib/events.c:286
#15 0x000000004245e7a0 in s3_event_loop_once (ev=0x42b55460,
location=<optimized out>) at lib/events.c:349
#16 0x000000004245ef90 in _tevent_loop_once (ev=0x42b55460, location=0x42979ec0
"smbd/server.c:844") at ../lib/tevent/tevent.c:494
#17 0x000000004275acbc in smbd_parent_loop (parent=<optimized out>) at
smbd/server.c:844
#18 main (argc=<optimized out>, argv=<optimized out>) at smbd/server.c:1326

The database format was recently changed to store the printcap location field, a bug was introduced with this change. One of the tdb_pack calls is not provided with a location argument in source3/printing/printer_list.c:

 30 #define PL_DATA_FORMAT "ddPPP"
...
193         len = tdb_pack(data.dptr, data.dsize,
194                        PL_DATA_FORMAT, time_h, time_l, name, str);


Fix is a simple one liner:
Index: samba-3.6.3/source3/printing/printer_list.c
===================================================================
--- samba-3.6.3.orig/source3/printing/printer_list.c
+++ samba-3.6.3/source3/printing/printer_list.c
@@ -191,7 +191,7 @@ NTSTATUS printer_list_set_printer(TALLOC
        data.dsize = len;

        len = tdb_pack(data.dptr, data.dsize,
-                      PL_DATA_FORMAT, time_h, time_l, name, str);
+                      PL_DATA_FORMAT, time_h, time_l, name, str, str2);

        status = dbwrap_store_bystring_upper(db, key, data, TDB_REPLACE);

I'll do a quick audit of other calls in printer_list.c and then attach a patch.
Comment 1 David Disseldorp 2012-02-15 14:47:27 UTC
Created attachment 7323 [details]
fix based on v3-6-test
Comment 2 Guenther Deschner 2012-02-15 14:58:41 UTC
Comment on attachment 7323 [details]
fix based on v3-6-test

looks good.

That was probably me. Another reason to hate tdb_pack/-unpack, this would not have happend when using an autogenerated marshalling interface...
Comment 3 David Disseldorp 2012-02-15 15:08:14 UTC
(In reply to comment #2)
> Comment on attachment 7323 [details]
> fix based on v3-6-test
> 
> looks good.

Thanks for the review Günther.

> 
> That was probably me. Another reason to hate tdb_pack/-unpack, this would not
> have happend when using an autogenerated marshalling interface...

Nice, here's hoping tdb2+ heads in that direction.
Comment 4 David Disseldorp 2012-02-15 15:59:45 UTC
Pushed to autobuild as:
s3-printing: fix crash in printer_list_set_printer()

Karolin, please merge for v3-6.
Comment 5 Guenther Deschner 2012-02-16 14:07:14 UTC
David, do you remember, is this in 3.5 as well ?
Comment 6 Guenther Deschner 2012-02-16 14:09:55 UTC
Replying to myself: no, apparently not :-)
Comment 7 Karolin Seeger 2012-02-18 18:47:18 UTC
Pushed to v3-6-test.
Closing out bug report.

Thanks!
Comment 8 Christian Perrier 2012-05-07 17:43:23 UTC
From the bug log, I understand this fix is on its way to 3.6.6. Am I right, Karolin?
Comment 9 Karolin Seeger 2012-05-08 07:01:24 UTC
(In reply to comment #8)
> From the bug log, I understand this fix is on its way to 3.6.6. Am I right,
> Karolin?

Yes, exactly.