diff options
author | Andrew Tridgell <tridge@samba.org> | 2005-04-01 11:24:52 +0000 |
---|---|---|
committer | Gerald (Jerry) Carter <jerry@samba.org> | 2007-10-10 13:11:23 -0500 |
commit | 772f31797d22a8ef78e7257bbbd0cb36196af03a (patch) | |
tree | 800e3dcea7abd37e125e8738ed5085aa31ec88ac | |
parent | 7288298b65b4040d112f06def3c9836aba9ddbaf (diff) | |
download | samba-772f31797d22a8ef78e7257bbbd0cb36196af03a.tar.gz samba-772f31797d22a8ef78e7257bbbd0cb36196af03a.tar.bz2 samba-772f31797d22a8ef78e7257bbbd0cb36196af03a.zip |
r6165: fixed up the userinfo composite code. Fixes include:
- talloc should always be done in the right context. For example, when creating
the userinfo_state structure, place it inside the composite
structure, not directly on the pipe. If this isn't done then
correct cleanup can't happen on errors (as cleanup destroys the top
level composite context only)
- define private structures like userinfo_state in the userinfo.c
code, not in the public header
- only keep the parameters we need in the state structure. For
example, the domain_handle is only needed in the first call, so we
don't need to keep it around in the state structure, but the level is
needed in later calls, so we need to keep it
- always initialise [out,ref] parameters in RPC calls. The [ref] part
means that the call assumes the pointer it has been given is
valid. If you don't initialise it then you will get a segv on
recv. This is why the code was dying.
- don't use internal strucrure elements like the pipe
pipe->conn->pending outside of the internal rpc implementation. That
is an internal list, trying to use it from external code will cause crashes.
- rpc calls assume that rpc call strucrures remain valid for the
duration of the call. This means you need to keep the structures
(such as "struct samr_Close") in the userinfo_state strucrure,
otherwise it will go out of scope during the async processing
- need to remember to change c->state to SMBCLI_REQUEST_DONE when the
request has finished in the close handler, otherwise it will loop
forever trying to close
Mimir, please look at the diff carefully for more detailed info on the fixes
(This used to be commit 01ea1e7762e214e87e74d6f28d6efeb6cdea9736)
-rw-r--r-- | source4/libnet/composite.h | 9 | ||||
-rw-r--r-- | source4/libnet/userinfo.c | 106 | ||||
-rw-r--r-- | source4/torture/libnet/userinfo.c | 9 |
3 files changed, 53 insertions, 71 deletions
diff --git a/source4/libnet/composite.h b/source4/libnet/composite.h index f4f5c456fe..86127a19b3 100644 --- a/source4/libnet/composite.h +++ b/source4/libnet/composite.h @@ -22,8 +22,6 @@ composite function definitions */ -enum userinfo_stage { USERINFO_OPENUSER, USERINFO_GETUSER, USERINFO_CLOSEUSER }; - struct rpc_composite_userinfo { struct { struct policy_handle domain_handle; @@ -34,10 +32,3 @@ struct rpc_composite_userinfo { union samr_UserInfo info; } out; }; - -struct userinfo_state { - enum userinfo_stage stage; - struct dcerpc_pipe *pipe; - struct rpc_request *req; - struct rpc_composite_userinfo io; -}; diff --git a/source4/libnet/userinfo.c b/source4/libnet/userinfo.c index a61610ed21..16c9c0beb9 100644 --- a/source4/libnet/userinfo.c +++ b/source4/libnet/userinfo.c @@ -30,37 +30,43 @@ static void userinfo_handler(struct rpc_request *req); +enum userinfo_stage { USERINFO_OPENUSER, USERINFO_GETUSER, USERINFO_CLOSEUSER }; + +struct userinfo_state { + enum userinfo_stage stage; + struct dcerpc_pipe *pipe; + struct rpc_request *req; + struct policy_handle user_handle; + uint16_t level; + struct samr_OpenUser openuser; + struct samr_QueryUserInfo queryuserinfo; + struct samr_Close samrclose; + union samr_UserInfo *info; +}; /** * Stage 1: Open user policy handle in SAM server. */ - static NTSTATUS userinfo_openuser(struct composite_context *c, - struct rpc_composite_userinfo *io) + struct userinfo_state *s) { - struct userinfo_state *s = talloc_get_type(c->private, struct userinfo_state); - struct rpc_request *req = s->req; - struct samr_OpenUser *rep; - struct samr_QueryUserInfo r; - /* receive samr_OpenUser reply */ - c->status = dcerpc_ndr_request_recv(req); + c->status = dcerpc_ndr_request_recv(s->req); NT_STATUS_NOT_OK_RETURN(c->status); - rep = (struct samr_OpenUser*)req->ndr.struct_ptr; /* prepare parameters for QueryUserInfo call */ - r.in.user_handle = rep->out.user_handle; - r.in.level = io->in.level; + s->queryuserinfo.in.user_handle = &s->user_handle; + s->queryuserinfo.in.level = s->level; /* queue rpc call, set event handling and new state */ - s->req = dcerpc_samr_QueryUserInfo_send(s->pipe, c, &r); + s->req = dcerpc_samr_QueryUserInfo_send(s->pipe, c, &s->queryuserinfo); if (s->req == NULL) goto failure; s->req->async.callback = userinfo_handler; s->req->async.private = c; s->stage = USERINFO_GETUSER; - return rep->out.result; + return NT_STATUS_OK; failure: return NT_STATUS_UNSUCCESSFUL; @@ -72,53 +78,42 @@ failure: */ static NTSTATUS userinfo_getuser(struct composite_context *c, - struct rpc_composite_userinfo *io) + struct userinfo_state *s) { - struct userinfo_state *s = talloc_get_type(c->private, struct userinfo_state); - struct rpc_request *req = s->pipe->conn->pending; - struct samr_QueryUserInfo *rep; - struct samr_Close r; - /* receive samr_QueryUserInfo reply */ - c->status = dcerpc_ndr_request_recv(req); + c->status = dcerpc_ndr_request_recv(s->req); NT_STATUS_NOT_OK_RETURN(c->status); - rep = (struct samr_QueryUserInfo*)req->ndr.struct_ptr; + + s->info = talloc_steal(s, s->queryuserinfo.out.info); /* prepare arguments for Close call */ - r.in.handle = rep->in.user_handle; + s->samrclose.in.handle = &s->user_handle; + s->samrclose.out.handle = &s->user_handle; /* queue rpc call, set event handling and new state */ - s->req = dcerpc_samr_Close_send(s->pipe, c, &r); + s->req = dcerpc_samr_Close_send(s->pipe, c, &s->samrclose); s->req->async.callback = userinfo_handler; s->req->async.private = c; s->stage = USERINFO_CLOSEUSER; - /* copying result of composite call */ - io->out.info = *rep->out.info; - - return rep->out.result; + return NT_STATUS_OK; } /** * Stage3: Close policy handle associated with opened user. */ - static NTSTATUS userinfo_closeuser(struct composite_context *c, - struct rpc_composite_userinfo *io) + struct userinfo_state *s) { - struct userinfo_state *s = talloc_get_type(c->private, struct userinfo_state); - struct rpc_request *req = s->pipe->conn->pending; - struct samr_Close *rep; - /* receive samr_Close reply */ - c->status = dcerpc_ndr_request_recv(req); + c->status = dcerpc_ndr_request_recv(s->req); NT_STATUS_NOT_OK_RETURN(c->status); - rep = (struct samr_Close*)req->ndr.struct_ptr; - /* return result */ - return rep->out.result; + c->state = SMBCLI_REQUEST_DONE; + + return NT_STATUS_OK; } @@ -128,7 +123,6 @@ static NTSTATUS userinfo_closeuser(struct composite_context *c, * * @param req rpc call context */ - static void userinfo_handler(struct rpc_request *req) { struct composite_context *c = req->async.private; @@ -137,15 +131,15 @@ static void userinfo_handler(struct rpc_request *req) /* Stages of the call */ switch (s->stage) { case USERINFO_OPENUSER: - c->status = userinfo_openuser(c, &s->io); + c->status = userinfo_openuser(c, s); break; case USERINFO_GETUSER: - c->status = userinfo_getuser(c, &s->io); + c->status = userinfo_getuser(c, s); break; case USERINFO_CLOSEUSER: - c->status = userinfo_closeuser(c, &s->io); + c->status = userinfo_closeuser(c, s); break; } @@ -166,14 +160,12 @@ static void userinfo_handler(struct rpc_request *req) * @param p dce/rpc call pipe * @param io arguments and results of the call */ - -struct composite_context* rpc_composite_userinfo_send(struct dcerpc_pipe *p, +struct composite_context *rpc_composite_userinfo_send(struct dcerpc_pipe *p, struct rpc_composite_userinfo *io) { struct composite_context *c; struct userinfo_state *s; - struct samr_OpenUser *r; struct dom_sid *sid; c = talloc_zero(p, struct composite_context); @@ -181,25 +173,25 @@ struct composite_context* rpc_composite_userinfo_send(struct dcerpc_pipe *p, s = talloc_zero(c, struct userinfo_state); if (s == NULL) goto failure; + + s->level = io->in.level; + s->pipe = p; - /* copying input parameters */ - s->io.in.domain_handle = io->in.domain_handle; - s->io.in.sid = talloc_strdup(p, io->in.sid); - s->io.in.level = io->in.level; - sid = dom_sid_parse_talloc(c, s->io.in.sid); + sid = dom_sid_parse_talloc(s, io->in.sid); if (sid == NULL) goto failure; - c->private = s; + c->state = SMBCLI_REQUEST_SEND; + c->private = s; c->event_ctx = dcerpc_event_context(p); /* preparing parameters to send rpc request */ - r = talloc_zero(p, struct samr_OpenUser); - r->in.domain_handle = &s->io.in.domain_handle; - r->in.access_mask = SEC_FLAG_MAXIMUM_ALLOWED; - r->in.rid = sid->sub_auths[sid->num_auths - 1]; + s->openuser.in.domain_handle = &io->in.domain_handle; + s->openuser.in.access_mask = SEC_FLAG_MAXIMUM_ALLOWED; + s->openuser.in.rid = sid->sub_auths[sid->num_auths - 1]; + s->openuser.out.user_handle = &s->user_handle; /* send request */ - s->req = dcerpc_samr_OpenUser_send(p, c, r); + s->req = dcerpc_samr_OpenUser_send(p, c, &s->openuser); /* callback handler */ s->req->async.callback = userinfo_handler; @@ -234,8 +226,8 @@ NTSTATUS rpc_composite_userinfo_recv(struct composite_context *c, TALLOC_CTX *me if (NT_STATUS_IS_OK(status) && io) { s = talloc_get_type(c->private, struct userinfo_state); - talloc_steal(mem_ctx, &s->io.out.info); - io->out.info = s->io.out.info; + talloc_steal(mem_ctx, s->info); + io->out.info = *s->info; } /* memory context associated to composite context is no longer needed */ diff --git a/source4/torture/libnet/userinfo.c b/source4/torture/libnet/userinfo.c index 911aa086d4..fb54b6a184 100644 --- a/source4/torture/libnet/userinfo.c +++ b/source4/torture/libnet/userinfo.c @@ -25,7 +25,6 @@ #define TEST_USERNAME "libnetuserinfotest" - static BOOL test_opendomain(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx, struct policy_handle *handle, struct samr_String *domname, struct dom_sid2 *sid) @@ -51,7 +50,7 @@ static BOOL test_opendomain(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx, r2.in.connect_handle = &h; r2.in.domain_name = domname; - printf("domain lookup\n"); + printf("domain lookup on %s\n", domname->string); status = dcerpc_samr_LookupDomain(p, mem_ctx, &r2); if (!NT_STATUS_IS_OK(status)) { @@ -96,7 +95,7 @@ static BOOL test_cleanup(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx, r1.in.num_names = 1; r1.in.names = names; - printf("user account lookup\n"); + printf("user account lookup '%s'\n", username); status = dcerpc_samr_LookupNames(p, mem_ctx, &r1); if (!NT_STATUS_IS_OK(status)) { @@ -150,7 +149,7 @@ static BOOL test_create(struct dcerpc_pipe *p, TALLOC_CTX *mem_ctx, r.out.user_handle = &user_handle; r.out.rid = rid; - printf("creating user account\n"); + printf("creating user account %s\n", name); status = dcerpc_samr_CreateUser(p, mem_ctx, &r); if (!NT_STATUS_IS_OK(status)) { @@ -237,7 +236,7 @@ BOOL torture_userinfo(void) ret = False; goto done; } - name.string = b->host; + name.string = lp_workgroup(); if (!test_opendomain(p, mem_ctx, &h, &name, &sid)) { ret = False; |