On 08/15/2012 03:56 PM, Jakub Hrozek wrote:
On Tue, Aug 14, 2012 at 01:38:15PM +0200, Pavel Březina wrote:
On 08/13/2012 11:42 AM, Jakub Hrozek wrote:
On Mon, Aug 13, 2012 at 11:10:36AM +0200, Jakub Hrozek wrote:
On Fri, Aug 10, 2012 at 09:26:49AM +0200, Pavel Březina wrote:
On 08/08/2012 04:24 PM, Jakub Hrozek wrote:
On Wed, Aug 08, 2012 at 02:23:04PM +0200, Pavel Březina wrote:
This bug was probably introduced with the subdomain patches. The
problem was that sss_dp_get_domains_send() is called even for the local
provider. There are certainly many possible solutions of this issue. I
decided to modify sss_dp_issue_request() to call the callback
immediately if it is issued for local domain. This way, I believe, we
can avoid further problems with issuing request for local domains.
I don't think this is the best solution.
First, it leaves the check_provider of the per-domain structures we use
in responders around while duplicating the same functionality in the DP.
At the very least, the patch should remove the "check_provider" tests from
the existing loops.
What I think would be even better solution is to
1) in the RC just fix the subdomains code so that it shortcuts or
doesn't run at all for LOCAL lookups.
Patch is attached.
Thank you, this works for me.
I'm seeing this warning when LOCAL users are enumerated (as opposed to a
direct lookup):
(Mon Aug 13 11:06:58 2012) [sssd[nss]] [setent_notify] (0x0010): BUG: a
callback did not free its request. May leak memory
Given that the LOCAL domain is mostly useful for testing right now, I've
just filed a ticket:
https://fedorahosted.org/sssd/ticket/1477
Ack
Nack, upon further testing. I'm seeing segfaults when looking up
non-LOCAL users:
Core was generated by `/usr/libexec/sssd/sssd_nss -d 0x17f0'.
Program terminated with signal 11, Segmentation fault.
#0 0x000000000043acb1 in get_domains_next (req=0xf95ca0) at
src/responder/common/responder_get_domains.c:124
124 tevent_req_post(req, info->rctx->ev);
Missing separate debuginfos, use: debuginfo-install
nss-softokn-freebl-3.13.5-1.fc17.x86_64 openldap-2.4.31-3.fc17.x86_64
(gdb) bt
#0 0x000000000043acb1 in get_domains_next (req=0xf95ca0) at
src/responder/common/responder_get_domains.c:124
#1 0x000000000043b461 in sss_dp_get_domains_callback (subreq=0xf968b0) at
src/responder/common/responder_get_domains.c:215
#2 0x0000000000439dd4 in sss_dp_internal_get_done (pending=0xf944e0,
ptr=0xf99220) at src/responder/common/responder_dp.c:731
#3 0x000000342fa0c42a in complete_pending_call_and_unlock
(connection=connection@entry=0xf95e40, pending=0xf944e0,
message=message@entry=0xf98a80) at dbus-connection.c:2308
#4 0x000000342fa0f5ea in dbus_connection_dispatch (connection=0xf95e40) at
dbus-connection.c:4593
#5 0x0000000000460432 in sbus_dispatch (ev=0xf89390, te=0xf98bc0, tv=...,
data=0xf979a0) at src/sbus/sssd_dbus_connection.c:104
#6 0x000000385b407600 in tevent_common_loop_timer_delay (ev=ev@entry=0xf89390)
at ../tevent_timed.c:254
#7 0x000000385b406cac in std_event_loop_once (ev=0xf89390, location=<optimized
out>) at ../tevent_standard.c:560
#8 0x000000385b403ed0 in _tevent_loop_once (ev=ev@entry=0xf89390,
location=location@entry=0x492f97 "src/util/server.c:554") at ../tevent.c:506
#9 0x000000385b40405b in tevent_common_loop_wait (ev=0xf89390, location=0x492f97
"src/util/server.c:554") at ../tevent.c:607
#10 0x0000000000467bf4 in server_loop (main_ctx=0xf8a4a0) at
src/util/server.c:554
#11 0x0000000000408d5c in main (argc=3, argv=0x7fff2dde9cc8) at
src/responder/nss/nsssrv.c:422
(gdb) p *info
$1 = {rctx = 0x3133623833347830, dom = 0x306d61736c3a313a, hint = 0x696e403334346336
<Address 0x696e403334346336 out of bounds>, force = 114, state = 0x0}
It looks like "info" was not initialized.
Thanks.
The problem was that callback is invoked immediately when
tevent_req_done() is called. And because req is freed in this
callback, info was pointing to invalid data.
New patch is attached.
I think it would be much cleaner to remove the tevent_req_post
completely from get_domains_next() and only call it in
sss_dp_get_domains_send() if get_domains_next() returned EOK.
I wouldn't expect tevent_req_post to be called in any other function
that *_send. Also checking the SSSD code shows that we only call
tevent_req_post from *_send elsewhere.
As you wish. Patch is attached.
From ec30155ccd2006a441e587e694a17c3dc09ce30b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrez...@redhat.com>
Date: Fri, 10 Aug 2012 09:24:22 +0200
Subject: [PATCH] Fix LOCAL domain lookups
https://fedorahosted.org/sssd/ticket/1436
Now subdomains are not evaluated for local domains.
---
src/responder/common/responder_get_domains.c | 41 ++++++++++++++------------
1 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/src/responder/common/responder_get_domains.c b/src/responder/common/responder_get_domains.c
index 2660d5e8006dea20383b66055ea976e82d0836e6..a98e663416ee37a932e2cdfb3d1382f9b5bf9fed 100644
--- a/src/responder/common/responder_get_domains.c
+++ b/src/responder/common/responder_get_domains.c
@@ -56,7 +56,7 @@ struct tevent_req *sss_dp_get_domains_send(TALLOC_CTX *mem_ctx,
if (rctx->domains == NULL) {
DEBUG(SSSDBG_CRIT_FAILURE, ("No domains configured.\n"));
ret = EINVAL;
- goto done;
+ goto immediately;
}
if (!force) {
@@ -65,11 +65,11 @@ struct tevent_req *sss_dp_get_domains_send(TALLOC_CTX *mem_ctx,
if (ret == EOK) {
DEBUG(SSSDBG_TRACE_FUNC,
("Last call was too recent, nothing to do!\n"));
- goto done;
+ goto immediately;
} else if (ret != EAGAIN) {
DEBUG(SSSDBG_TRACE_FUNC, ("check_domain_request failed with [%d][%s]\n",
ret, strerror(ret)));
- goto done;
+ goto immediately;
}
}
@@ -82,20 +82,24 @@ struct tevent_req *sss_dp_get_domains_send(TALLOC_CTX *mem_ctx,
info->hint = talloc_strdup(info, "");
if (info->hint == NULL) {
ret = ENOMEM;
- goto done;
+ goto immediately;
}
}
ret = get_domains_next(req);
- if (ret == EAGAIN) {
- ret = EOK;
+ if (ret != EAGAIN) {
+ goto immediately;
}
-done:
- if (ret != EOK) {
+ return req;
+
+immediately:
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else {
tevent_req_error(req, ret);
- tevent_req_post(req, rctx->ev);
}
+ tevent_req_post(req, rctx->ev);
return req;
}
@@ -109,16 +113,13 @@ static errno_t get_domains_next(struct tevent_req *req)
char *key;
info = tevent_req_data(req, struct sss_dp_domains_info);
+
+ /* Skip all local domains. */
+ while(info->dom != NULL && !NEED_CHECK_PROVIDER(info->dom->provider)) {
+ info->dom = info->dom->next;
+ }
+
if (info->dom == NULL) {
- /* Note that tevent_req_post() is not here. This will
- * influence the program only in case that this will
- * be the first call of the function (i.e. there is no
- * problem when this is called from get_domains_done(),
- * it is in fact required). In case no domains are in
- * the state, it should be treated as an error one level
- * above.
- */
- tevent_req_done(req);
return EOK;
}
@@ -210,7 +211,9 @@ static void sss_dp_get_domains_callback(struct tevent_req *subreq)
info->dom = info->dom->next;
ret = get_domains_next(req);
- if (ret != EOK && ret != EAGAIN) {
+ if (ret == EOK) {
+ tevent_req_done(req);
+ } else if (ret != EAGAIN) {
goto fail;
}
--
1.7.6.5
_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
