Bug 8418 - Core accessing not-valid queues
Summary: Core accessing not-valid queues
Status: NEW
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: Printing (show other bugs)
Version: 3.5.11
Hardware: All All
: P5 normal
Target Milestone: ---
Assignee: Guenther Deschner
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-29 09:53 UTC by Luis Fabiani
Modified: 2021-01-07 22:58 UTC (History)
3 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Luis Fabiani 2011-08-29 09:53:32 UTC
Last friday we had a core with samba 3.0.9 but we've found that the code that produced the core is still available on 3.5.11 and 3.6.0.

The complete stack was this:

 febc2f62 sigacthandler (b, 0, 8046c50) + f2
 --- called from signal handler with signal 11 (SIGSEGV) ---
 08266fb9 Get_Pwnam_alloc (0, 0, 8046f88, 826708e) + 19
 082670a2 Get_Pwnam (0, fec63000, 8046fa8, 8266ce8) + 1e
 08266cfa get_user_home_dir (0) + 1e
 0827cbc8 automount_path (0, 25, 1) + 20
 0827db1c alloc_sub_advanced (0, 842c858, 83c2cb0, 0, 842c758, 842ca58) + cc
 0827ddc2 standard_sub_advanced (0, 842c858, 83c2cb0, 0, 842c758) + 2e
 08297837 print_queue_update (ffffffff, 0, 8516a6c, 829a5ed) + f3
 0829a625 print_queue_status (ffffffff, 0, 0, 819f54e) + 49
 0819f58a update_monitored_printq_cache (0, 0, 840825c, 10f8, 10fc, 8419ae4) + 46
 08127487 timeout_processing (93a80, 8047c8c, 8047c88, 20441) + 32b
 0812788a smbd_process (8047cf0, febcd89b, fec674c0, 840c500, 0, 0) + 1da

Translated to human:
- (rpc_server/srv_spoolss_nt.c) On update_monitored_printq_cache():
   - It goes through all the printers
   - executes "snum = print_queue_snum(printer->sharename);"
   - calls to print_queue_status() with snum. On the error, snum==-1 because it's name was not valid
   - Note: on 3.6.0, this function has been moved to rpc_server/spoolss/srv_spoolss_nt.c

- (printing/printing.c) print_queue_status() receives -1 as 1st parameter (int snum), but it can be valid
- (printing/printing.c) print_queue_update() receives -1 as 1st parameter (int snum)
   - Calls to "standard_sub_advanced(lp_servicename(snum), ...)" [on Samba 3.5, talloc_sub_advanced(ctx, lp_servicename(snum), ...) but the same behavior].
   - lp_servicename(0xFFFFFFFF) returns NULL but it's not checked

- (lib/substitute.c) standard_sub_advanced()/talloc_sub_advanced() receives NULL as 1st parameter  (char* servicename). This value is passed in cascade to the next functions without consequences:
   - (lib/substitute.c) alloc_sub_advanced()
   - (lib/substitute.c) automount_path()
   - (lib/username.c) get_user_home_dir()
   - (lib/username.c) Get_Pwnam() [not existing on 3.5 & 3.6, where get_user_home_dir() calls directly Get_Pwnam_alloc()].
- (lib/username.c) Get_Pwnam_alloc() receives NULL as 2nd parameter (char* user)
   - Get_Pwnam_alloc doesn't check user but tries to access directly to *user and dies.

On the first comment, the proposed solution.
Comment 1 Luis Fabiani 2011-08-29 10:15:14 UTC
Proposed solutions:

(1) Check param user on Get_Pwnam_alloc() [lib/username.c]
- if ( *user == '\0' ) {
+ if ( user == NULL ||  *user == '\0' ) {

(2) Check servicename on print_queue_update() [printing/printing.c]
+ const char* servicename = NULL;
- talloc_sub_advanced(ctx,
-            lp_servicename(snum),
+ servicename = lp_servicename(ctx, snum);
+ if (servicename == NULL)
+    return;
+ talloc_sub_advanced(ctx, servicename,
+            servicename,

(3) Check service number on update_monitored_printq_cache [3.5.11: rpc_server/srv_spoolss_nt.c ; 3.6.0: rpc_server/spoolss/srv_spoolss_nt.c]
{version 3.5.11}
- print_queue_status( snum, NULL, NULL );
+ if (snum != -1)
+    print_queue_status( snum, NULL, NULL );
{version 3.6.0}
- print_queue_status( msg_ctx, snum, NULL, NULL );
+ if (snum != -1)
+    print_queue_status( msg_ctx, snum, NULL, NULL );

Any of these solutions should solve the problem. Probably solution 1 will solve other problems as being the more depth. Don't know about the possible consequences of 2nd solution as it executes other instructions and haven't checked all the possible execution flows.

We're implementing all 3 solutions but, as it's not possible to reproduce the core conditions, we won't be able to send a report. But we think that this modifications (specially (1) and (3)) can be useful.