The Samba-Bugzilla – Bug 13745
print command %J substitution
Last modified: 2019-07-29 13:07:51 UTC
Samba 4.8 introduced new replacement %J, %j for pathsafe IP address. Unfortunatly this conflicts with with %J in "print command". All submitted print jobs get the client IP address as jobname. This happens because %J is implemented in talloc_sub_basic, which is called before the substitutions of "print command".
Another clash is %p. This works, because it is implemented in talloc_sub_advanced, which is called after the substitutions of "print command". Other commands (lppause, lpresume use %j) may also be affected.
Arg, this looks like a nasty regression from ca66efc24181ba6a7a4c13397af514b0972b4855 .
Any thoughts on how we can resolve this?
IMO we should probably add a special case handler for "print command", lppause and lpresume to ensure that printing specific replacements are handled there before the generic talloc_sub_basic() case.
Can you get me a debug level 10 log showing when the string in 'print command' is getting clobbered by the new call to talloc_sub_basic() ?
I'm hoping we can differentiate by share type, or some other way of deciding this is a print share/string so we can protect the print substitutions.
Created attachment 14933 [details]
Logleve 10 Log
Created attachment 14934 [details]
Comamnd was smbd -d 10 -s smb-x.conf
I've instrumented talloc_sub_basic()/%J with a call to log_stack_trace() with the following outcome (the share is used for generating PDFs):
[2019/07/26 20:03:34.697354, 0] ../../lib/util/fault.c:261(log_stack_trace)
BACKTRACE: 31 stack frames:
#0 /usr/lib64/libsamba-util.so.0(log_stack_trace+0x30) [0x7f8067764460]
#1 /usr/lib64/libsmbconf.so.0(talloc_sub_basic+0x4d1) [0x7f806624da01]
#2 /usr/lib64/libsmbconf.so.0(lp_string+0x3a) [0x7f8066228bca]
#3 /usr/lib64/samba/libsmbd-base-samba4.so(+0x9dfc4) [0x7f806726dfc4]
#4 /usr/lib64/samba/libsmbd-base-samba4.so(print_job_end+0x38b) [0x7f806725392b]
#5 /usr/lib64/samba/libsmbd-base-samba4.so(_spoolss_EndDocPrinter+0x7e) [0x7f80673ffe9e]
#6 /usr/lib64/samba/libsmbd-base-samba4.so(+0x198403) [0x7f8067368403]
#7 /usr/lib64/samba/libsmbd-base-samba4.so(+0x220572) [0x7f80673f0572]
#8 /usr/lib64/samba/libsmbd-base-samba4.so(process_complete_pdu+0x1b65) [0x7f80673f2e95]
#9 /usr/lib64/samba/libsmbd-base-samba4.so(named_pipe_packet_process+0xda) [0x7f80673aa42a]
#10 /usr/lib64/libdcerpc-binding.so.0(+0xc8af) [0x7f805eb8d8af]
#11 /usr/lib64/samba/libsamba-sockets-samba4.so(+0x7219) [0x7f8065fe9219]
#12 /usr/lib64/samba/libsamba-sockets-samba4.so(+0x5fdb) [0x7f8065fe7fdb]
#13 /usr/lib64/libtevent.so.0(tevent_common_invoke_immediate_handler+0x111) [0x7f8063a31e21]
#14 /usr/lib64/libtevent.so.0(tevent_common_loop_immediate+0x1e) [0x7f8063a31e7e]
#15 /usr/lib64/libtevent.so.0(+0xc77b) [0x7f8063a3777b]
#16 /usr/lib64/libtevent.so.0(+0xace7) [0x7f8063a35ce7]
#17 /usr/lib64/libtevent.so.0(_tevent_loop_once+0x9d) [0x7f8063a311dd]
#18 /usr/lib64/libtevent.so.0(tevent_common_loop_wait+0x1b) [0x7f8063a313fb]
#19 /usr/lib64/libtevent.so.0(+0xac87) [0x7f8063a35c87]
#20 /usr/lib64/samba/libsmbd-base-samba4.so(smbd_process+0x76f) [0x7f806731285f]
#21 /usr/sbin/smbd(+0xd970) [0x55e1c7857970]
#22 /usr/lib64/libtevent.so.0(tevent_common_invoke_fd_handler+0x7f) [0x7f8063a319ff]
#23 /usr/lib64/libtevent.so.0(+0xc94a) [0x7f8063a3794a]
#24 /usr/lib64/libtevent.so.0(+0xace7) [0x7f8063a35ce7]
#25 /usr/lib64/libtevent.so.0(_tevent_loop_once+0x9d) [0x7f8063a311dd]
#26 /usr/lib64/libtevent.so.0(tevent_common_loop_wait+0x1b) [0x7f8063a313fb]
#27 /usr/lib64/libtevent.so.0(+0xac87) [0x7f8063a35c87]
#28 /usr/sbin/smbd(main+0x199d) [0x55e1c78529dd]
#29 /lib64/libc.so.6(__libc_start_main+0xea) [0x7f8063691f8a]
#30 /usr/sbin/smbd(_start+0x2a) [0x55e1c7852e5a]
The relevant part of the stacktrace boils down to:
lp_print_command (direct jump to lp_string)
generic_job_submit (via current_printif->job_submit)
There are a few ways to fix this:
1. rename the new and practically unused %j/%J to %k/%K (or any other convenient letter) and mention all currently implemented %-replacements in a single place in the documentation/code (eg. case 'J': /* print job name for print shares */ break; in talloc_sub_basic)
2. split talloc_sub_basic/lp_string/... into print/non-print (or sub/no_sub) versions
3. pass down snum/servicename to talloc_sub_basic
For me there are a few reasons to go with 1.:
- since the new %j/%J are only in distribution use for about half a year (Ubuntu 18.10, RHEL 7.6, SLES15SP1), nearly nobody will make use of it (I guess less than 10 people worldwide)
- the "print" %J is nearly 20 years old and has many users (PDF printing shares)
- the IP %j/%J may also be convenient for print shares
- the revert/change can easily be backported (even to 4.8 since it's a kind of DoS for PDF shares)
- maybe testparm could be changed to warn about %j/%J use in non-print shares
I volunteer to implement a patch for 1. (maybe also for a different solution if it's not too invasive).