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.
From 298fe5d688d83c1933376afee12a86f5e97efb7b 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 |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/src/responder/common/responder_get_domains.c b/src/responder/common/responder_get_domains.c
index 2660d5e8006dea20383b66055ea976e82d0836e6..d46b2d1741299c954073818ede6bd40b279d9d82 100644
--- a/src/responder/common/responder_get_domains.c
+++ b/src/responder/common/responder_get_domains.c
@@ -109,15 +109,18 @@ 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.
+        /* This may be the first call if all available domains are local.
+         * Therefore, we need to call tevent_req_post() to make sure that
+         * callback is called anyway.
          */
+        tevent_req_post(req, info->rctx->ev);
         tevent_req_done(req);
         return EOK;
     }
-- 
1.7.6.5

_______________________________________________
sssd-devel mailing list
sssd-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

Reply via email to