[PATCHES] Forward smbcontrol messages only from parent

[Jeremy Allison]

On Wed, May 14, 2014 at 02:12:08PM +0200, Christof Schmitt wrote:
> 
> >From 32048b4e1709ef2633a3978e6809d7cb1038abfa Mon Sep 17 00:00:00 2001
> From: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> Date: Mon, 25 Feb 2013 15:38:09 +0100
> Subject: [PATCH 2/4] s3:smbcontrol: Forward MSG_DEBUG only if we are the parent process
> 
> When smbd receives a MSG_DEBUG message the message is forwarded to all
> child processes listed in the 'children' linked list. When forking a new smbd
> child process 'children' still contains the list of all up to now forked
> child processes which causes a message storm between the smbd child processes.
> 
> Fixed by notifying child processes only if we are the smbd parent process.
> 
> Signed-off-by: Ralph Wuerthner <ralph.wuerthner at de.ibm.com>
> Reviewed-by: Christof Schmitt <cs at samba.org>
> ---
>  source3/smbd/server.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/source3/smbd/server.c b/source3/smbd/server.c
> index 37d93af..66c6b9e 100644
> --- a/source3/smbd/server.c
> +++ b/source3/smbd/server.c
> @@ -233,7 +233,9 @@ static void smbd_msg_debug(struct messaging_context *msg_ctx,
>  {
>  	debug_message(msg_ctx, private_data, MSG_DEBUG, server_id, data);
>  
> -	messaging_send_to_children(msg_ctx, MSG_DEBUG, data);
> +	if (am_parent) {
> +		messaging_send_to_children(msg_ctx, MSG_DEBUG, data);
> +	}
>  }

OK, I've looked into this *really* closely, and
I don't see how this can occur in current 4.0.x
and 4.1.x code.

So I'm dialing back my excitement about this
patch :-).

Looking at the fork() code inside source3/smbd/server.c
we have inside smbd_accept_connection():

 585         pid = fork();
 586         if (pid == 0) {
 587                 NTSTATUS status = NT_STATUS_OK;
 588 
 589                 /* Child code ... */
 590                 am_parent = NULL;
 591 
 592                 /*
 593                  * Can't use TALLOC_FREE here. Nulling out the argument to it
 594                  * would overwrite memory we've just freed.
 595                  */
 596                 talloc_free(s->parent);
 597                 s = NULL;

So the global variable 'am_parent' gets set to
NULL in the child.

Now look at messaging_send_to_children() in the
same file. We have:

 196 NTSTATUS messaging_send_to_children(struct messaging_context *msg_ctx,
 197                                     uint32_t msg_type, DATA_BLOB* data)
 198 {
 199         NTSTATUS status;
 200         struct smbd_parent_context *parent = am_parent;
 201         struct smbd_child_pid *child;
 202 
 203         if (parent == NULL) {
 204                 return NT_STATUS_INTERNAL_ERROR;
 205         }
 206 
 207         for (child = parent->children; child != NULL; child = child->next) {
 208                 status = messaging_send(parent->msg_ctx,
 209                                         pid_to_procid(child->pid),
 210                                         msg_type, data);
 ......

The local variable 'parent' gets set to 'am_parent',
so messaging_send_to_children() should always
return NT_STATUS_INTERNAL_ERROR is called within
a child.

Can you add some debug statements to your test
case so I can see how this can happen ?

Jeremy.