Re: [SCM] Samba Shared Repository - branch master updated - release-4-0-0alpha8-494-geba2eb4

2009-07-21 Thread Jeremy Allison
On Tue, Jul 21, 2009 at 08:22:46AM +0200, Stefan (metze) Metzmacher wrote:
> Hi Jeremy,
> 
> > Move the initialization of smbd_server_conn from smbd/process,
> > after the accept and fork, to smbd_init_globals(), so it's
> > done immediately on server startup. This is needed as some
> > messages are sent to all active smbd processes (including
> > the master listening daemon). If it gets a message that
> > forces it to scan it's current connections (ie. conn_find())
> > then it discovers that sconn->smb1.tcons.Connections dereferences
> > null (as sconn == NULL in the parent) and crashes. Yes,
> > I could fix all cases where sconn is used and explicitly
> > check for NULL but this fix is easier. It means that
> > the smbd_event_context() is initialized in the master
> > daemon and then re-initialized after fork, but that
> > should be being done correctly in every fork call anyway.
> > Without this change the previous fix 
> > 6a9e0039100b57f9626e87defec6720c476b9789
> > still panics in the reproducible test case for bug
> > 6564, as this is one case where such a message
> > (MSG_SMB_CONF_UPDATED) is sent to the parent. Metze
> > please check. This change passes valgrind.
> > Jeremy.
> 
> I think it is thte wrong approach, smbd_server_connection represents
> the state of a connected client/a smbd child process.
> I added it so that we strictly separate code that runs in the parent
> from code that runs in the child.
> 
> I'd really like to understand what the problem is.
> From reading the code I can't find where we hit any panic.

Oh, I thought the above description was good enough, sorry :-).

> Can you send me the backtrace please?

Sure, I'll reproduce and send it. I'm glad you're looking
at it, it's an ugly fix but I couldn't see an elegant way
of fixing it without filling the code with "if (!sconn) return"
statements everywhere.

Jeremy.


Re: [SCM] Samba Shared Repository - branch master updated - release-4-0-0alpha8-494-geba2eb4

2009-07-20 Thread Stefan (metze) Metzmacher
Hi Jeremy,

> Move the initialization of smbd_server_conn from smbd/process,
> after the accept and fork, to smbd_init_globals(), so it's
> done immediately on server startup. This is needed as some
> messages are sent to all active smbd processes (including
> the master listening daemon). If it gets a message that
> forces it to scan it's current connections (ie. conn_find())
> then it discovers that sconn->smb1.tcons.Connections dereferences
> null (as sconn == NULL in the parent) and crashes. Yes,
> I could fix all cases where sconn is used and explicitly
> check for NULL but this fix is easier. It means that
> the smbd_event_context() is initialized in the master
> daemon and then re-initialized after fork, but that
> should be being done correctly in every fork call anyway.
> Without this change the previous fix 
> 6a9e0039100b57f9626e87defec6720c476b9789
> still panics in the reproducible test case for bug
> 6564, as this is one case where such a message
> (MSG_SMB_CONF_UPDATED) is sent to the parent. Metze
> please check. This change passes valgrind.
> Jeremy.

I think it is thte wrong approach, smbd_server_connection represents
the state of a connected client/a smbd child process.
I added it so that we strictly separate code that runs in the parent
from code that runs in the child.

I'd really like to understand what the problem is.
From reading the code I can't find where we hit any panic.
Can you send me the backtrace please?

metze



signature.asc
Description: OpenPGP digital signature


[SCM] Samba Shared Repository - branch master updated - release-4-0-0alpha8-494-geba2eb4

2009-07-17 Thread Jeremy Allison
The branch, master has been updated
   via  eba2eb45e208e6b3091c01ff1d40fd966e72a044 (commit)
   via  55b4231c773ef17b8e628f33d6c3c9d5335df9da (commit)
   via  6a9e0039100b57f9626e87defec6720c476b9789 (commit)
  from  33251da8611ddc98c3ae73d62601218e5b784e63 (commit)

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -
commit eba2eb45e208e6b3091c01ff1d40fd966e72a044
Author: Jeremy Allison 
Date:   Fri Jul 17 18:05:10 2009 -0700

Fix a typo reading uninitialized memory. Caught by valgrind.
Jeremy.

commit 55b4231c773ef17b8e628f33d6c3c9d5335df9da
Author: Jeremy Allison 
Date:   Fri Jul 17 17:57:48 2009 -0700

Move the initialization of smbd_server_conn from smbd/process,
after the accept and fork, to smbd_init_globals(), so it's
done immediately on server startup. This is needed as some
messages are sent to all active smbd processes (including
the master listening daemon). If it gets a message that
forces it to scan it's current connections (ie. conn_find())
then it discovers that sconn->smb1.tcons.Connections dereferences
null (as sconn == NULL in the parent) and crashes. Yes,
I could fix all cases where sconn is used and explicitly
check for NULL but this fix is easier. It means that
the smbd_event_context() is initialized in the master
daemon and then re-initialized after fork, but that
should be being done correctly in every fork call anyway.
Without this change the previous fix 
6a9e0039100b57f9626e87defec6720c476b9789
still panics in the reproducible test case for bug
6564, as this is one case where such a message
(MSG_SMB_CONF_UPDATED) is sent to the parent. Metze
please check. This change passes valgrind.
Jeremy.

commit 6a9e0039100b57f9626e87defec6720c476b9789
Author: Jeremy Allison 
Date:   Fri Jul 17 17:36:26 2009 -0700

Fix bug #6564 - SetPrinter fails (panics) as non root.
Missing become_root()/unbecome_root() around reload_services.
Jeremy.

---

Summary of changes:
 source3/lib/system.c|4 +---
 source3/rpc_server/srv_spoolss_nt.c |4 
 source3/smbd/globals.c  |5 +
 source3/smbd/process.c  |5 -
 source3/smbd/server.c   |1 +
 5 files changed, 11 insertions(+), 8 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source3/lib/system.c b/source3/lib/system.c
index ffc236e..6a4f5d5 100644
--- a/source3/lib/system.c
+++ b/source3/lib/system.c
@@ -458,8 +458,6 @@ static struct timespec calc_create_time_stat_ex(const 
struct stat_ex *st)
 
 static void get_create_timespec(const struct stat *pst, struct stat_ex *dst)
 {
-   struct timespec ret;
-
if (S_ISDIR(pst->st_mode) && lp_fake_dir_create_times()) {
dst->st_ex_btime.tv_sec = 315493200L;  /* 1/1/1980 */
dst->st_ex_btime.tv_nsec = 0;
@@ -483,7 +481,7 @@ static void get_create_timespec(const struct stat *pst, 
struct stat_ex *dst)
/* Deal with systems that don't initialize birthtime correctly.
 * Pointed out by SATOH Fumiyasu .
 */
-   if (null_timespec(ret)) {
+   if (null_timespec(dst->st_ex_btime)) {
dst->st_ex_btime = calc_create_time_stat(pst);
dst->st_ex_calculated_birthtime = true;
}
diff --git a/source3/rpc_server/srv_spoolss_nt.c 
b/source3/rpc_server/srv_spoolss_nt.c
index 48ac103..9dc1a26 100644
--- a/source3/rpc_server/srv_spoolss_nt.c
+++ b/source3/rpc_server/srv_spoolss_nt.c
@@ -309,7 +309,9 @@ static WERROR delete_printer_hook(TALLOC_CTX *ctx, 
NT_USER_TOKEN *token, const c
return WERR_BADFID; /* What to return here? */
 
/* go ahead and re-read the services immediately */
+   become_root();
reload_services(false);
+   unbecome_root();
 
if ( lp_servicenumber( sharename )  < 0 )
return WERR_ACCESS_DENIED;
@@ -6034,7 +6036,9 @@ bool add_printer_hook(TALLOC_CTX *ctx, NT_USER_TOKEN 
*token, NT_PRINTER_INFO_LEV
}
 
/* reload our services immediately */
+   become_root();
reload_services(false);
+   unbecome_root();
 
numlines = 0;
/* Get lines and convert them back to dos-codepage */
diff --git a/source3/smbd/globals.c b/source3/smbd/globals.c
index 15550ed..317304a 100644
--- a/source3/smbd/globals.c
+++ b/source3/smbd/globals.c
@@ -153,4 +153,9 @@ void smbd_init_globals(void)
ZERO_STRUCT(conn_ctx_stack);
 
ZERO_STRUCT(sec_ctx_stack);
+
+   smbd_server_conn = talloc_zero(smbd_event_context(), struct 
smbd_server_connection);
+   if (!smbd_server_conn) {
+   exit_server("failed to create smbd_server_connection");
+   }
 }
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index b26bc15..c2065ca 100644
--- a/sourc