From 5d3bb5b18575e6485562caa5f9fb9fb945b8a9c9 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Jun 2011 08:33:09 +0200 Subject: [PATCH 1/3] s3:rpc_server/svcctl: don't allocate return values on a temporary stackframe And always initialize the whole return structure. This caused samba3.posix_s3.rpc.svcctl to be flakey. metze Autobuild-User: Stefan Metzmacher Autobuild-Date: Thu Jun 16 11:34:34 CEST 2011 on sn-devel-104 (cherry picked from commit 48de3e51eacbd1051f79dc99aaac8a4ec988fde5) --- source3/rpc_server/svcctl/srv_svcctl_nt.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-) diff --git a/source3/rpc_server/svcctl/srv_svcctl_nt.c b/source3/rpc_server/svcctl/srv_svcctl_nt.c index bf7ade8..1894ae2 100644 --- a/source3/rpc_server/svcctl/srv_svcctl_nt.c +++ b/source3/rpc_server/svcctl/srv_svcctl_nt.c @@ -668,17 +668,18 @@ WERROR _svcctl_QueryServiceStatusEx(struct pipes_struct *p, /******************************************************************** ********************************************************************/ -static WERROR fill_svc_config(TALLOC_CTX *ctx, +static WERROR fill_svc_config(TALLOC_CTX *mem_ctx, struct messaging_context *msg_ctx, struct auth_serversupplied_info *session_info, const char *name, struct QUERY_SERVICE_CONFIG *config) { - TALLOC_CTX *mem_ctx = talloc_stackframe(); const char *result = NULL; /* now fill in the individual values */ + ZERO_STRUCTP(config); + config->displayname = svcctl_lookup_dispname(mem_ctx, msg_ctx, session_info, @@ -720,9 +721,6 @@ static WERROR fill_svc_config(TALLOC_CTX *ctx, else config->start_type = SVCCTL_DEMAND_START; - - talloc_free(mem_ctx); - return WERR_OK; } -- 1.7.4.1 From df9d3063148b5a9c936b10a8ea84396afcd40c60 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Jun 2011 12:34:42 +0200 Subject: [PATCH 2/3] s3:rpc_server/svcctl: fix valgrind bugs in _svcctl_QueryServiceConfig2W() r->out.buffer needs to stay in its size, as it will be marshalled completely. As it's preallocated and initialized with zeros, we just need to copy the payload into it. If we always marshall the return buffer, we already have the needed buffer size and don't need to call ndr_size_* functions. metze (cherry picked from commit 11683ccf3e68606ecb1cdfa455f7921b119803c6) --- source3/rpc_server/svcctl/srv_svcctl_nt.c | 14 +++++--------- 1 files changed, 5 insertions(+), 9 deletions(-) diff --git a/source3/rpc_server/svcctl/srv_svcctl_nt.c b/source3/rpc_server/svcctl/srv_svcctl_nt.c index 1894ae2..36e5832 100644 --- a/source3/rpc_server/svcctl/srv_svcctl_nt.c +++ b/source3/rpc_server/svcctl/srv_svcctl_nt.c @@ -775,7 +775,8 @@ WERROR _svcctl_QueryServiceConfig2W(struct pipes_struct *p, struct svcctl_QueryServiceConfig2W *r) { SERVICE_INFO *info = find_service_info_by_hnd( p, r->in.handle ); - uint32 buffer_size; + uint32_t buffer_size; + DATA_BLOB blob = data_blob_null; /* perform access checks */ @@ -795,7 +796,6 @@ WERROR _svcctl_QueryServiceConfig2W(struct pipes_struct *p, struct SERVICE_DESCRIPTION desc_buf; const char *description; enum ndr_err_code ndr_err; - DATA_BLOB blob; description = svcctl_lookup_description(p->mem_ctx, p->msg_ctx, @@ -810,9 +810,6 @@ WERROR _svcctl_QueryServiceConfig2W(struct pipes_struct *p, return WERR_INVALID_PARAM; } - buffer_size = ndr_size_SERVICE_DESCRIPTION(&desc_buf, 0); - r->out.buffer = blob.data; - break; } break; @@ -820,7 +817,6 @@ WERROR _svcctl_QueryServiceConfig2W(struct pipes_struct *p, { struct SERVICE_FAILURE_ACTIONS actions; enum ndr_err_code ndr_err; - DATA_BLOB blob; /* nothing to say...just service the request */ @@ -832,9 +828,6 @@ WERROR _svcctl_QueryServiceConfig2W(struct pipes_struct *p, return WERR_INVALID_PARAM; } - buffer_size = ndr_size_SERVICE_FAILURE_ACTIONS(&actions, 0); - r->out.buffer = blob.data; - break; } break; @@ -843,12 +836,15 @@ WERROR _svcctl_QueryServiceConfig2W(struct pipes_struct *p, return WERR_UNKNOWN_LEVEL; } + buffer_size = blob.length; buffer_size += buffer_size % 4; *r->out.needed = (buffer_size > r->in.offered) ? buffer_size : r->in.offered; if (buffer_size > r->in.offered) return WERR_INSUFFICIENT_BUFFER; + memcpy(r->out.buffer, blob.data, blob.length); + return WERR_OK; } -- 1.7.4.1 From 34b5929aad3e088cbd46f55473454b3653defda5 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Thu, 16 Jun 2011 12:47:22 +0200 Subject: [PATCH 3/3] s3:rpc_server/svcctl: fix valgrind bug in _svcctl_QueryServiceObjectSecurity() r->out.buffer needs to stay in its size, as it will be marshalled completely. As it's preallocated and initialized with zeros, we just need to copy the payload into it, even if it's smaller than the offered buffer size. metze Autobuild-User: Stefan Metzmacher Autobuild-Date: Thu Jun 16 14:15:47 CEST 2011 on sn-devel-104 (cherry picked from commit 67512152c007bb186e4fd8dac5d1aab89bce0689) --- source3/rpc_server/svcctl/srv_svcctl_nt.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/source3/rpc_server/svcctl/srv_svcctl_nt.c b/source3/rpc_server/svcctl/srv_svcctl_nt.c index 36e5832..044dd15 100644 --- a/source3/rpc_server/svcctl/srv_svcctl_nt.c +++ b/source3/rpc_server/svcctl/srv_svcctl_nt.c @@ -936,7 +936,7 @@ WERROR _svcctl_QueryServiceObjectSecurity(struct pipes_struct *p, } *r->out.needed = len; - r->out.buffer = buffer; + memcpy(r->out.buffer, buffer, len); return WERR_OK; } -- 1.7.4.1