Bug 13745 - print command %J substitution
print command %J substitution
Status: NEW
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Printing
All All
: P5 regression
: ---
Assigned To: printing-maintainers
Samba QA Contact
Depends on:
  Show dependency treegraph
Reported: 2019-01-15 11:22 UTC by Heinrich Mislik
Modified: 2019-07-29 13:07 UTC (History)
3 users (show)

See Also:

Logleve 10 Log (878.60 KB, text/x-log)
2019-03-15 13:25 UTC, Heinrich Mislik
no flags Details
smb config (322 bytes, text/plain)
2019-03-15 13:26 UTC, Heinrich Mislik
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Heinrich Mislik 2019-01-15 11:22:46 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.


Comment 1 David Disseldorp 2019-01-15 13:52:14 UTC
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.
Comment 2 Jeremy Allison 2019-01-16 00:57:53 UTC
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.
Comment 3 Heinrich Mislik 2019-03-15 13:25:37 UTC
Created attachment 14933 [details]
Logleve 10 Log
Comment 4 Heinrich Mislik 2019-03-15 13:26:48 UTC
Created attachment 14934 [details]
smb config

Comamnd was smbd -d 10 -s smb-x.conf
Comment 5 Franz Sirl 2019-07-29 08:41:15 UTC
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).