diff options
-rw-r--r-- | source3/include/ntdomain.h | 4 | ||||
-rw-r--r-- | source3/rpc_client/cli_pipe.c | 24 | ||||
-rw-r--r-- | source3/rpc_client/ndr.c | 2 | ||||
-rw-r--r-- | source3/rpc_parse/parse_prs.c | 52 | ||||
-rw-r--r-- | source3/rpc_server/srv_pipe_hnd.c | 57 |
5 files changed, 78 insertions, 61 deletions
diff --git a/source3/include/ntdomain.h b/source3/include/ntdomain.h index 213cb9ff75..25bef47493 100644 --- a/source3/include/ntdomain.h +++ b/source3/include/ntdomain.h @@ -42,7 +42,9 @@ typedef struct _prs_struct { uint32 data_offset; /* Current working offset into data. */ uint32 buffer_size; /* Current allocated size of the buffer. */ uint32 grow_size; /* size requested via prs_grow() calls */ - char *data_p; /* The buffer itself. */ + /* The buffer itself. If "is_dynamic" is true this + * MUST BE TALLOC'ed off mem_ctx. */ + char *data_p; TALLOC_CTX *mem_ctx; /* When unmarshalling, use this.... */ } prs_struct; diff --git a/source3/rpc_client/cli_pipe.c b/source3/rpc_client/cli_pipe.c index e248133de3..d420cbce3c 100644 --- a/source3/rpc_client/cli_pipe.c +++ b/source3/rpc_client/cli_pipe.c @@ -1453,7 +1453,6 @@ static void rpc_api_pipe_trans_done(struct tevent_req *subreq) NTSTATUS status; uint8_t *rdata = NULL; uint32_t rdata_len = 0; - char *rdata_copy; status = cli_api_pipe_recv(subreq, state, &rdata, &rdata_len); TALLOC_FREE(subreq); @@ -1471,16 +1470,11 @@ static void rpc_api_pipe_trans_done(struct tevent_req *subreq) } /* - * Give the memory received from cli_trans as dynamic to the current - * pdu. Duplicating it sucks, but prs_struct doesn't know about talloc - * :-( + * This is equivalent to a talloc_steal - gives rdata to + * the prs_struct state->incoming_frag. */ - rdata_copy = (char *)memdup(rdata, rdata_len); - TALLOC_FREE(rdata); - if (tevent_req_nomem(rdata_copy, req)) { - return; - } - prs_give_memory(&state->incoming_frag, rdata_copy, rdata_len, true); + prs_give_memory(&state->incoming_frag, (char *)rdata, rdata_len, true); + rdata = NULL; /* Ensure we have enough data for a pdu. */ subreq = get_complete_frag_send(state, state->ev, state->cli, @@ -1597,9 +1591,10 @@ static NTSTATUS rpc_api_pipe_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, reply_pdu->mem_ctx = mem_ctx; /* - * Prevent state->incoming_pdu from being freed in - * rpc_api_pipe_state_destructor() + * Prevent state->incoming_pdu from being freed + * when state is freed. */ + talloc_steal(mem_ctx, prs_data_p(reply_pdu)); prs_init_empty(&state->incoming_pdu, state, UNMARSHALL); return NT_STATUS_OK; @@ -2458,9 +2453,10 @@ NTSTATUS rpc_api_pipe_req_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, reply_pdu->mem_ctx = mem_ctx; /* - * Prevent state->req_pdu from being freed in - * rpc_api_pipe_req_state_destructor() + * Prevent state->req_pdu from being freed + * when state is freed. */ + talloc_steal(mem_ctx, prs_data_p(reply_pdu)); prs_init_empty(&state->reply_pdu, state, UNMARSHALL); return NT_STATUS_OK; diff --git a/source3/rpc_client/ndr.c b/source3/rpc_client/ndr.c index bbd78067bf..8e03f2e015 100644 --- a/source3/rpc_client/ndr.c +++ b/source3/rpc_client/ndr.c @@ -103,7 +103,6 @@ static void cli_do_rpc_ndr_done(struct tevent_req *subreq) status = rpc_api_pipe_req_recv(subreq, state, &state->r_ps); TALLOC_FREE(subreq); - prs_mem_free(&state->q_ps); if (!NT_STATUS_IS_OK(status)) { tevent_req_nterror(req, status); return; @@ -126,7 +125,6 @@ NTSTATUS cli_do_rpc_ndr_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx) } ret = prs_data_blob(&state->r_ps, &blob, talloc_tos()); - prs_mem_free(&state->r_ps); if (!ret) { return NT_STATUS_NO_MEMORY; } diff --git a/source3/rpc_parse/parse_prs.c b/source3/rpc_parse/parse_prs.c index 42e6ce1b7b..af1cc3d5b2 100644 --- a/source3/rpc_parse/parse_prs.c +++ b/source3/rpc_parse/parse_prs.c @@ -93,7 +93,7 @@ void prs_debug(prs_struct *ps, int depth, const char *desc, const char *fn_name) * Initialise an expandable parse structure. * * @param size Initial buffer size. If >0, a new buffer will be - * created with malloc(). + * created with talloc(). * * @return False if allocation fails, otherwise True. **/ @@ -112,11 +112,11 @@ bool prs_init(prs_struct *ps, uint32 size, TALLOC_CTX *ctx, bool io) if (size != 0) { ps->buffer_size = size; - if((ps->data_p = (char *)SMB_MALLOC((size_t)size)) == NULL) { - DEBUG(0,("prs_init: malloc fail for %u bytes.\n", (unsigned int)size)); + ps->data_p = talloc_zero_size(ps->mem_ctx, size); + if(ps->data_p == NULL) { + DEBUG(0,("prs_init: talloc fail for %u bytes.\n", (unsigned int)size)); return False; } - memset(ps->data_p, '\0', (size_t)size); ps->is_dynamic = True; /* We own this memory. */ } else if (MARSHALLING(ps)) { /* If size is zero and we're marshalling we should allocate memory on demand. */ @@ -130,14 +130,18 @@ bool prs_init(prs_struct *ps, uint32 size, TALLOC_CTX *ctx, bool io) Delete the memory in a parse structure - if we own it. NOTE: Contrary to the somewhat confusing naming, this function is not - intended for freeing memory allocated by prs_alloc_mem(). That memory - is attached to the talloc context given by ps->mem_ctx. + intended for freeing memory allocated by prs_alloc_mem(). + That memory is also attached to the talloc context given by + ps->mem_ctx, but is only freed when that talloc context is + freed. prs_mem_free() is used to delete "dynamic" memory + allocated in marshalling/unmarshalling. ********************************************************************/ void prs_mem_free(prs_struct *ps) { - if(ps->is_dynamic) - SAFE_FREE(ps->data_p); + if(ps->is_dynamic) { + TALLOC_FREE(ps->data_p); + } ps->is_dynamic = False; ps->buffer_size = 0; ps->data_offset = 0; @@ -184,11 +188,17 @@ TALLOC_CTX *prs_get_mem_context(prs_struct *ps) /******************************************************************* Hand some already allocated memory to a prs_struct. + If "is_dynamic" is true, this is a talloc_steal. + If "is_dynamic" is false, just make ps->data_p a pointer to + the unwritable memory. ********************************************************************/ void prs_give_memory(prs_struct *ps, char *buf, uint32 size, bool is_dynamic) { ps->is_dynamic = is_dynamic; + if (ps->is_dynamic && buf) { + talloc_steal(ps->mem_ctx, buf); + } ps->data_p = buf; ps->buffer_size = size; } @@ -207,14 +217,16 @@ bool prs_set_buffer_size(prs_struct *ps, uint32 newsize) /* newsize == 0 acts as a free and set pointer to NULL */ if (newsize == 0) { - SAFE_FREE(ps->data_p); + TALLOC_FREE(ps->data_p); } else { - ps->data_p = (char *)SMB_REALLOC(ps->data_p, newsize); + ps->data_p = talloc_realloc(ps->mem_ctx, + ps->data_p, + char, + newsize); if (ps->data_p == NULL) { DEBUG(0,("prs_set_buffer_size: Realloc failure for size %u.\n", (unsigned int)newsize)); - DEBUG(0,("prs_set_buffer_size: Reason %s\n",strerror(errno))); return False; } } @@ -261,11 +273,11 @@ bool prs_grow(prs_struct *ps, uint32 extra_space) */ new_size = MAX(128, extra_space); - if((ps->data_p = (char *)SMB_MALLOC(new_size)) == NULL) { - DEBUG(0,("prs_grow: Malloc failure for size %u.\n", (unsigned int)new_size)); + ps->data_p = (char *)talloc_zero_size(ps->mem_ctx, new_size); + if(ps->data_p == NULL) { + DEBUG(0,("prs_grow: talloc failure for size %u.\n", (unsigned int)new_size)); return False; } - memset(ps->data_p, '\0', (size_t)new_size ); } else { /* * If the current buffer size is bigger than the space needed, @@ -276,7 +288,11 @@ bool prs_grow(prs_struct *ps, uint32 extra_space) new_size = MAX(ps->buffer_size*2, ps->buffer_size + extra_space + 64); - if ((ps->data_p = (char *)SMB_REALLOC(ps->data_p, new_size)) == NULL) { + ps->data_p = talloc_realloc(ps->mem_ctx, + ps->data_p, + char, + new_size); + if (ps->data_p == NULL) { DEBUG(0,("prs_grow: Realloc failure for size %u.\n", (unsigned int)new_size)); return False; @@ -305,7 +321,11 @@ bool prs_force_grow(prs_struct *ps, uint32 extra_space) return False; } - if((ps->data_p = (char *)SMB_REALLOC(ps->data_p, new_size)) == NULL) { + ps->data_p = (char *)talloc_realloc(ps->mem_ctx, + ps->data_p, + char, + new_size); + if(ps->data_p == NULL) { DEBUG(0,("prs_force_grow: Realloc failure for size %u.\n", (unsigned int)new_size)); return False; diff --git a/source3/rpc_server/srv_pipe_hnd.c b/source3/rpc_server/srv_pipe_hnd.c index 975f5b823a..a77b9eabc0 100644 --- a/source3/rpc_server/srv_pipe_hnd.c +++ b/source3/rpc_server/srv_pipe_hnd.c @@ -237,24 +237,29 @@ static ssize_t unmarshall_rpc_header(pipes_struct *p) } /**************************************************************************** - Call this to free any talloc'ed memory. Do this before and after processing - a complete PDU. + Call this to free any talloc'ed memory. Do this after processing + a complete incoming and outgoing request (multiple incoming/outgoing + PDU's). ****************************************************************************/ static void free_pipe_context(pipes_struct *p) { - if (p->mem_ctx) { - DEBUG(3, ("free_pipe_context: " - "destroying talloc pool of size %lu\n", - (unsigned long)talloc_total_size(p->mem_ctx))); - talloc_free_children(p->mem_ctx); - } else { - p->mem_ctx = talloc_named(p, 0, "pipe %s %p", - get_pipe_name_from_syntax(talloc_tos(), - &p->syntax), p); - if (p->mem_ctx == NULL) { - p->fault_state = True; - } + prs_mem_free(&p->out_data.frag); + prs_mem_free(&p->out_data.rdata); + prs_mem_free(&p->in_data.data); + + DEBUG(3, ("free_pipe_context: " + "destroying talloc pool of size %lu\n", + (unsigned long)talloc_total_size(p->mem_ctx))); + talloc_free_children(p->mem_ctx); + /* + * Re-initialize to set back to marshalling and set the + * offset back to the start of the buffer. + */ + if(!prs_init(&p->in_data.data, 128, p->mem_ctx, MARSHALL)) { + DEBUG(0, ("free_pipe_context: " + "rps_init failed!\n")); + p->fault_state = True; } } @@ -399,24 +404,10 @@ static bool process_request_pdu(pipes_struct *p, prs_struct *rpc_in_p) * Process the complete data stream here. */ - free_pipe_context(p); - if(pipe_init_outgoing_data(p)) { ret = api_pipe_request(p); } - free_pipe_context(p); - - /* - * We have consumed the whole data stream. Set back to - * marshalling and set the offset back to the start of - * the buffer to re-use it (we could also do a prs_mem_free() - * and then re_init on the next start of PDU. Not sure which - * is best here.... JRA. - */ - - prs_switch_type(&p->in_data.data, MARSHALL); - prs_set_offset(&p->in_data.data, 0); return ret; } @@ -868,6 +859,16 @@ static ssize_t read_from_internal_pipe(struct pipes_struct *p, char *data, p->out_data.current_pdu_sent = 0; prs_mem_free(&p->out_data.frag); } + + if(p->out_data.data_sent_length >= prs_offset(&p->out_data.rdata)) { + /* + * We're completely finished with both outgoing and + * incoming data streams. It's safe to free all temporary + * data from this request. + */ + free_pipe_context(p); + } + return data_returned; } |