See comments in commit message.
Also add patch to fix some talloc_free -> talloc_zfree cases in sysdb
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 93543a3f6cc6fc9e2c38f2e5824363f5cc4c155d Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@metason.pico.li.ssimo.org>
Date: Mon, 26 Oct 2009 15:11:14 -0400
Subject: [PATCH 1/2] Zero pointers on free
If the pointer stays around, zero it when it is freed, so we do not risk
access to released memory in case of bugs.
---
server/db/sysdb.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/server/db/sysdb.c b/server/db/sysdb.c
index a0c1338..5811ddc 100644
--- a/server/db/sysdb.c
+++ b/server/db/sysdb.c
@@ -471,7 +471,7 @@ int sysdb_transaction_commit_recv(struct tevent_req *req)
/* finally free handle
* this will also trigger the next transaction in the queue if any */
- talloc_free(state->handle);
+ talloc_zfree(state->handle);
if (tevent_req_is_error(req, &tstate, &err)) {
return err;
@@ -1227,7 +1227,7 @@ static int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
ret = EIO;
goto done;
}
- talloc_free(msg);
+ talloc_zfree(msg);
/* == create Users tree == */
@@ -1255,7 +1255,7 @@ static int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
ret = EIO;
goto done;
}
- talloc_free(msg);
+ talloc_zfree(msg);
/* == create Groups tree == */
@@ -1283,7 +1283,7 @@ static int sysdb_domain_init_internal(TALLOC_CTX *mem_ctx,
ret = EIO;
goto done;
}
- talloc_free(msg);
+ talloc_zfree(msg);
ret = EOK;
--
1.6.2.5
>From 279112329f777608101ade9af19942697fed6274 Mon Sep 17 00:00:00 2001
From: Simo Sorce <s...@metason.pico.li.ssimo.org>
Date: Mon, 26 Oct 2009 15:12:45 -0400
Subject: [PATCH 2/2] Use standard coding practice to set last login
This rewrite should also fix a segfault in the code that may happen when
exiting in case of error conditions. The previous code was attaching the
transaction handle to llreq structure and then calling prepare_reply() from
within the request handlers which could ultimately free the preq and
llreq and handle before the transaction request was actually completed
by tevent.
---
server/responder/pam/pamsrv_cmd.c | 220 ++++++++++++++++++++++---------------
1 files changed, 130 insertions(+), 90 deletions(-)
diff --git a/server/responder/pam/pamsrv_cmd.c b/server/responder/pam/pamsrv_cmd.c
index 770afaa..a1da310 100644
--- a/server/responder/pam/pamsrv_cmd.c
+++ b/server/responder/pam/pamsrv_cmd.c
@@ -30,18 +30,6 @@
#include "responder/pam/pamsrv.h"
#include "db/sysdb.h"
-struct last_login_request {
- struct tevent_context *ev;
- struct sysdb_ctx *dbctx;
- struct sysdb_attrs *mod_attrs;
- struct sysdb_handle *handle;
-
- struct ldb_result *res;
- int error;
-
- struct pam_auth_req *preq;
-};
-
static void pam_reply(struct pam_auth_req *preq);
static int extract_authtok(uint32_t *type, uint32_t *size, uint8_t **tok, uint8_t *body, size_t blen, size_t *c) {
@@ -280,146 +268,198 @@ static int pam_parse_in_data(struct sss_names_ctx *snctx,
return EOK;
}
-static void prepare_reply(struct last_login_request *llreq)
-{
- struct pam_data *pd;
+/*=Save-Last-Login-State===================================================*/
- pd = llreq->preq->pd;
+struct set_last_login_state {
+ struct tevent_context *ev;
+ struct sysdb_ctx *dbctx;
- if (llreq->error != EOK && pd->pam_status == PAM_SUCCESS)
- pd->pam_status = PAM_SYSTEM_ERR;
+ struct sss_domain_info *dom;
+ const char *username;
+ struct sysdb_attrs *attrs;
- llreq->preq->callback(llreq->preq);
-}
+ struct sysdb_handle *handle;
+
+ struct ldb_result *res;
+};
+
+static void set_last_login_trans_done(struct tevent_req *subreq);
+static void set_last_login_attrs_done(struct tevent_req *subreq);
+static void set_last_login_done(struct tevent_req *subreq);
-static void set_user_last_login_done(struct tevent_req *req)
+static struct tevent_req *set_last_login_send(TALLOC_CTX *memctx,
+ struct tevent_context *ev,
+ struct sysdb_ctx *dbctx,
+ struct sss_domain_info *dom,
+ const char *username,
+ struct sysdb_attrs *attrs)
{
- struct last_login_request *llreq =
- tevent_req_callback_data(req,
- struct last_login_request);
- int ret;
+ struct tevent_req *req, *subreq;
+ struct set_last_login_state *state;
- ret = sysdb_transaction_commit_recv(req);
- if(ret != EOK) {
- DEBUG(2, ("set_last_login failed.\n"));
- llreq->error = ret;
+ req = tevent_req_create(memctx, &state, struct set_last_login_state);
+ if (!req) {
+ return NULL;
}
- llreq->preq->pd->last_auth_saved = true;
+ state->ev = ev;
+ state->dbctx = dbctx;
+ state->dom = dom;
+ state->username = username;
+ state->attrs = attrs;
+ state->handle = NULL;
+
+ subreq = sysdb_transaction_send(state, state->ev, state->dbctx);
+ if (!subreq) {
+ talloc_free(req);
+ return NULL;
+ }
+ tevent_req_set_callback(subreq, set_last_login_trans_done, req);
- prepare_reply(llreq);
+ return req;
}
-static void set_user_last_login_req_done(struct tevent_req *subreq);
-static void set_user_last_login_req(struct tevent_req *req)
+static void set_last_login_trans_done(struct tevent_req *subreq)
{
- struct last_login_request *llreq =
- tevent_req_callback_data(req,
- struct last_login_request);
- struct tevent_req *subreq;
+ struct tevent_req *req = tevent_req_callback_data(subreq,
+ struct tevent_req);
+ struct set_last_login_state *state = tevent_req_data(req,
+ struct set_last_login_state);
int ret;
- ret = sysdb_transaction_recv(req, llreq, &llreq->handle);
+ ret = sysdb_transaction_recv(subreq, state, &state->handle);
+ talloc_zfree(subreq);
if (ret != EOK) {
- llreq->error = ret;
- return prepare_reply(llreq);
+ DEBUG(1, ("Unable to acquire sysdb transaction lock\n"));
+ tevent_req_error(req, ret);
+ return;
}
- subreq = sysdb_set_user_attr_send(llreq, llreq->ev, llreq->handle,
- llreq->preq->domain,
- llreq->preq->pd->user,
- llreq->mod_attrs, SYSDB_MOD_REP);
+ subreq = sysdb_set_user_attr_send(state, state->ev, state->handle,
+ state->dom, state->username,
+ state->attrs, SYSDB_MOD_REP);
if (!subreq) {
- /* Cancel transaction */
- talloc_zfree(llreq->handle);
- llreq->error = EIO;
- return prepare_reply(llreq);
+ tevent_req_error(req, ENOMEM);
+ return;
}
- tevent_req_set_callback(subreq, set_user_last_login_req_done, llreq);
+ tevent_req_set_callback(subreq, set_last_login_attrs_done, req);
}
-static void set_user_last_login_req_done(struct tevent_req *subreq)
+static void set_last_login_attrs_done(struct tevent_req *subreq)
{
- struct last_login_request *llreq =
- tevent_req_callback_data(subreq,
- struct last_login_request);
- struct tevent_req *req;
+ struct tevent_req *req = tevent_req_callback_data(subreq,
+ struct tevent_req);
+ struct set_last_login_state *state = tevent_req_data(req,
+ struct set_last_login_state);
int ret;
ret = sysdb_set_user_attr_recv(subreq);
talloc_zfree(subreq);
+ if (ret != EOK) {
+ DEBUG(4, ("set_user_attr_callback, status [%d][%s]\n",
+ ret, strerror(ret)));
+ tevent_req_error(req, ret);
+ return;
+ }
- DEBUG(4, ("set_user_attr_callback, status [%d][%s]\n", ret, strerror(ret)));
-
- if (ret) {
- llreq->error = ret;
- goto fail;
+ subreq = sysdb_transaction_commit_send(state, state->ev, state->handle);
+ if (!subreq) {
+ tevent_req_error(req, ENOMEM);
+ return;
}
+ tevent_req_set_callback(subreq, set_last_login_done, req);
+}
- req = sysdb_transaction_commit_send(llreq, llreq->ev, llreq->handle);
- if (!req) {
- llreq->error = ENOMEM;
- goto fail;
+static void set_last_login_done(struct tevent_req *subreq)
+{
+ struct tevent_req *req = tevent_req_callback_data(subreq,
+ struct tevent_req);
+ int ret;
+
+ ret = sysdb_transaction_commit_recv(req);
+ if (ret != EOK) {
+ DEBUG(2, ("set_last_login failed.\n"));
+ tevent_req_error(req, ret);
+ return;
}
- tevent_req_set_callback(req, set_user_last_login_done, llreq);
-fail:
- DEBUG(2, ("set_last_login failed.\n"));
+ tevent_req_done(req);
+}
- /* cancel transaction */
- talloc_zfree(llreq->handle);
+static int set_last_login_recv(struct tevent_req *req)
+{
+ enum tevent_req_state tstate;
+ uint64_t err = 0;
+
+ if (tevent_req_is_error(req, &tstate, &err)) {
+ return err;
+ }
- prepare_reply(llreq);
+ return EOK;
}
+/*=========================================================================*/
+
+
+static void set_last_login_reply(struct tevent_req *req);
+
static errno_t set_last_login(struct pam_auth_req *preq)
{
- struct last_login_request *llreq;
struct tevent_req *req;
+ struct sysdb_ctx *dbctx;
+ struct sysdb_attrs *attrs;
errno_t ret;
- llreq = talloc_zero(preq, struct last_login_request);
- if (!llreq) {
+ attrs = sysdb_new_attrs(preq);
+ if (!attrs) {
ret = ENOMEM;
goto fail;
}
- llreq->ev = preq->cctx->ev;
- llreq->preq = preq;
- llreq->mod_attrs = sysdb_new_attrs(llreq);
- if (!llreq->mod_attrs) {
- ret = ENOMEM;
- goto fail;
- }
-
- ret = sysdb_attrs_add_long(llreq->mod_attrs, SYSDB_LAST_ONLINE_AUTH,
- (long)time(NULL));
+ ret = sysdb_attrs_add_time_t(attrs, SYSDB_LAST_ONLINE_AUTH, time(NULL));
if (ret != EOK) {
goto fail;
}
- ret = sysdb_get_ctx_from_list(preq->cctx->rctx->db_list,
- preq->domain, &llreq->dbctx);
+ ret = sysdb_get_ctx_from_list(preq->cctx->rctx->db_list, preq->domain,
+ &dbctx);
if (ret != EOK) {
- DEBUG(0, ("Fatal: Sysdb CTX not found for this domain!\n"));
+ DEBUG(0, ("Fatal: Sysdb context not found for this domain!\n"));
goto fail;
}
- req = sysdb_transaction_send(llreq, llreq->ev, llreq->dbctx);
+ req = set_last_login_send(preq, preq->cctx->ev, dbctx,
+ preq->domain, preq->pd->user, attrs);
if (!req) {
- DEBUG(1, ("Unable to acquire sysdb transaction lock\n"));
- ret = EIO;
+ ret = ENOMEM;
goto fail;
}
+ tevent_req_set_callback(req, set_last_login_reply, preq);
- tevent_req_set_callback(req, set_user_last_login_req, llreq);
return EOK;
fail:
- talloc_free(llreq);
return ret;
}
+static void set_last_login_reply(struct tevent_req *req)
+{
+ struct pam_auth_req *preq = tevent_req_callback_data(req,
+ struct pam_auth_req);
+ int ret;
+
+ ret = set_last_login_recv(req);
+ if (ret != EOK) {
+ if (preq->pd->pam_status == PAM_SUCCESS) {
+ preq->pd->pam_status = PAM_SYSTEM_ERR;
+ }
+ } else {
+ preq->pd->last_auth_saved = true;
+ }
+
+ preq->callback(preq);
+}
+
static void pam_reply_delay(struct tevent_context *ev, struct tevent_timer *te,
struct timeval tv, void *pvt)
{
--
1.6.2.5
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://fedorahosted.org/mailman/listinfo/sssd-devel
