sssd-devel November 2012

sssd-devel@lists.fedorahosted.org

15 participants
101 discussions

[PATCH] PAM: Do not leak fd after SELinux context file is written
by Jakub Hrozek 08 Nov '12

08 Nov '12

https://fedorahosted.org/sssd/ticket/1619 We don't close the fd when we write the selinux login file in the pam responder. This results in a fd leak. --- src/responder/pam/pamsrv_cmd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/responder/pam/pamsrv_cmd.c b/src/responder/pam/pamsrv_cmd.c index 2b20544d1baf78ade232f93a2c91378ec6a7645c..bb0d8db384cbbc8a3beed5dc5be0fe32956992a9 100644 --- a/src/responder/pam/pamsrv_cmd.c +++ b/src/responder/pam/pamsrv_cmd.c @@ -443,6 +443,7 @@ static errno_t write_selinux_login_file(const char *username, char *string) } else { ret = EOK; } + close(fd); fd = -1; done: -- 1.7.12.1

2 2

[PATCH] sss_cache: Remove fastcache even if sssd is not running.
by Michal Židek 07 Nov '12

07 Nov '12

https://fedorahosted.org/sssd/ticket/1584 First, I thought the race condition between sssd_nss and sss_cache should be solved by some sort of file locking mechanism, but when started working on it, the places where we needed to check for the file being locked or free were too many and spread among monitor, nss and sss_cache tool processes and it was not clear how the access is controlled. So I decided to do it this way: 1. sss_cache tries to send_sighup to monitor as usual 2. if signal_sssd returns that sssd is not running, proceed with memcache invalidation 3. As a part of memcache invalidation: - it first opens the mc file - then it checks if sssd is running (with pgrep) - if sssd is running it stops the the process of invalidation - if sssd is still not running it proceeds with the invalidation See that if sssd starts after (or during) the pgrep check (so we will not catch it as running, but will assume it is off) it is not a problem, because we have file descriptor associated with file that was present before sssd was running (we open the file before pgrep call). sssd_nss alwas creates a new memory cache file on startup, so we will only mark the old one as recycled (and not the new one), that we do not care about (because it will be deleted by sssd_nss and marking that as recycled is not a problem, I think -- the worst thing that can happen is race between nss and cache tool while both are marking the OLD file as recycled, but I can not figure out situation where this could be dangerous, because there are no "temp" states that could be harmful. Both processes are changing the value of status, but both to the same value). Another thing that I like about this is that we do not have to care about communication sssd vs sssd_nss vs sss_cache but only control sssd vs sss_cache, which is much easier to understand. Can someone see any problems that I missed? NOTE: The function sss_mc_set_recycled is copied from different module. I did not want to make this function non static and put it to a header file because it is not intended to be used directly. I tested it and it works fine for me. The patch is attached. Thanks Michal

4 19

[PATCH] sss_cache: Multiple domains not handled properly
by Michal Židek 06 Nov '12

06 Nov '12

I found this bug while working on: https://fedorahosted.org/sssd/ticket/1584 If no objects for deletion were found in the currently searched sysdb database, the other sysdb databases were not searched at all. Also the ERROR message informing about specific object not found was changed to DEBUG message. It was really annoying seeing this message for all domains, but it can be useful for debugging (it helped me by testing this patch). Now only one ERROR message is printed if no matching objects where found and all domains were searched. Patch is attached. Thanks Michal

2 3

[PATCH] RPMS: Move sss_cache tool to main package
by Stephen Gallagher 06 Nov '12

06 Nov '12

Fixes https://fedorahosted.org/sssd/ticket/1481

3 4

[PATCH] exit original process after sssd is initialized
by Pavel Březina 06 Nov '12

06 Nov '12

https://fedorahosted.org/sssd/ticket/1357 Neither systemd or our init script use pid file as a notification that sssd is finished initializing. They will continue starting up next service right after the original process is terminated.

4 26

Re: [SSSD] [Freeipa-devel] [RANT] Patchwork process
by Dmitri Pal 02 Nov '12

02 Nov '12

On 11/02/2012 07:22 AM, Petr Spacek wrote: > On 11/02/2012 11:10 AM, Petr Viktorin wrote: >> On 11/02/2012 10:46 AM, Martin Kosek wrote: >>> On 11/01/2012 07:28 PM, Simo Sorce wrote: >>>> On Thu, 2012-11-01 at 10:59 -0400, Rob Crittenden wrote: >>>>> Rob Crittenden wrote: >>>>>> Simo Sorce wrote: >>>>>>> From: Simo Sorce <ssorce(a)redhat.com> >>>>>>> >>>>>>> We check (possibly different) data from LDAP only at (re)start. >>>>>>> This way we always shutdown exactly the services we started even if >>>>>>> the list >>>>>>> changed in the meanwhile (we avoid leaving a service running >>>>>>> even if >>>>>>> it was >>>>>>> removed from LDAP as the admin decided it should not be started in >>>>>>> future). >>>>>>> >>>>>>> This should also fix a problematic deadlock with systemd when we >>>>>>> try >>>>>>> to read >>>>>>> the list of service from LDAP at shutdown. >>>>>> >>>>>> This fixes things for me but ipactl start isn't working reliably and >>>>>> I've yet to determine if it is related or not (I'm thinking not). >>>>>> >>>>>> What I see is that it considers pki-tomcatd to not have started. >>>>>> What is >>>>>> happening is the request to the getStatus URI is timing out and that >>>>>> exception is being considered a "didn't start." >>>>>> >>>>>> I modified the code to treat a timeout as a 503 and it is still >>>>>> failing >>>>>> because the timeout is so longer that it eats up all our normal >>>>>> timeout >>>>>> for waiting for the service at all. >>>>>> >>>>>> I don't see a way to pass a timeout to the http request method, I'll >>>>>> keep looking. >>>>>> >>>>>> I'm testing in F-18 btw. >>>>> >>>>> I can't reproduce the startup issues this morning, I still don't >>>>> think >>>>> they are related to this patch, so ACK. >>>>> >>>>> pushed all 3 to master and ipa-3-0 >>>> >>>> You pushed an older revision for patch 2, I reverted it on both trees >>>> and pushed the right one. >>>> >>>> Simo. >>> >>> While trying to follow this thread, I must throw my 2 conservative >>> cents in. >>> This thread is a very good example why processing our patches via >>> patchwork is >>> IMHO rather fuzzy and can cause more confusion than clarity. These >>> are my main >>> points: >>> >>> I cannot see which patches are the recent ones. Each patch is sent >>> separately >>> in mail body and I cannot quickly see by looking in thread which >>> mail contains >>> a revised patch and which is just a comment. >>> >>> When patches are attached directly to mail as files, I can quickly >>> distinguish >>> which mail has new patch set as it has "the paperclip icon". It also >>> enabled us >>> to attach a whole set of valid patches to one mail and thus make it >>> easier for >>> reviewer to pull a whole set of valid patches in one click. >> > >>> >>> Patchwork style patch sending also generates a lot of mails which >>> may confuse >>> not only me but other contributors... >> >> +1 >> >> This particular thread is extremely confusing. Each individual patch >> generated >> its own subthread, and then the revised patches were sent as a reply the >> original, spawning new threads to continue the old ones. The >> discussion is >> broken up and impossible to navigate. You can't see where the patches >> are, let >> alone where to look for latest versions. >> Thanks to Rob for showing us that even a person following the thread >> is easily >> confused. >> >> Patchwork adds a view that is incomplete and not cross-linked. >> To make a comment on a patch, you need to do it by e-mail; finding a >> patch in >> Patchwork is useless for this. Same for assigning a reviewer: you >> need to find >> it "manually" in Patchwork even if you have the mail in front of you. >> >> >> I don't really see Patchwork's advantage. For a list of "active" >> patches, we >> have the "Patch submitted" bit in Trac; to claim a larger review we >> could have >> a policy to announce this on the list. And the claim that Patchwork >> doesn't >> change our current practices is simply false. >> > > As guys described above, Patchwork influenced our process a lot and, > personally, I don't like these changes. > > +1 from me to more rational Trac usage. We can simply set bit and/or > reviewer name in Trac through API and avoid all changes introduced > with Patchwork. > > Simple script and appropriate Trac query can do better work for us. > OK. It looks like it brings more disruption than benefit. Simo, can we please get back to the original method at least for now? At this stage of development it see a lot of disruption that we can't afford. Patch tracking problem exists - I do not deny it but it seems that the change to patchwork does not bring the benefits you hoped. I have an action item to explore what we can do with other tools we considered like Gerrit. We might be not that far from it becoming an option. I will follow up and let you know. -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/

4 6

[INI] Remove code that allows duplicate sections
by Dmitri Pal 01 Nov '12

01 Nov '12

Hello, After previous set of patches have been committed I started working on the last remaining piece - the merge functionality. Re-reading the code I started some time ago I realized that at the time I left the question of whether the config object should allow partitioned section or not was still open. Consider an example of the config file: [foo] x=1 [bar] y=2 [foo] z=3 Here we have an example of the case when the section is segmented or if you look from another POW there are dup sections. So at first I thought that it is OK to parse the file as is and have the internal config object have two sections with same name. But after some deep thinking I realized that it is a bad idea and creates a lot of unnecessary complexity. So patch removes this code. This paves the way to a more simple code in the merge function as we can assume that there are no dup sections in the objects being merged. I also noticed that doc build is spitting warnings so I opened a separate ticket about it. That would be my next patch. https://fedorahosted.org/sssd/ticket/1587 -- Thank you, Dmitri Pal Sr. Engineering Manager for IdM portfolio Red Hat Inc. ------------------------------- Looking to carve out IT costs? www.redhat.com/carveoutcosts/

2 4

[PATCH] Monitor: read the correct SIGKILL timeout for providers, too
by Jakub Hrozek 01 Nov '12

01 Nov '12

We only read the configured SIGKILL time for services and actually used the value of 0, which actually translated into tevent_schedule_immediate. That meant that after we sent the SIGTERM we also sent a SIGKILL right away, which is bad...

2 4

Using patchwork for tracking SSSD patch status
by Simo Sorce 01 Nov '12

01 Nov '12

Hi all, a few days ago I installed patchwork and an instance is tracking the SSSD project here: https://patchwork.acksyn.org/project/SSSD/list/ Although patchwork is not a full cycle review system it seem to work really well for tacking patches and patchsets and their status. The only limitation with patchwork is that it really only likes one patch per email. This is why I switched fully to using git send-email both for the freeIPA (tracked as well) and the SSSD project. The advantage of using patchwork and git send-email is that it makes it easier to track feedback per patch (replies to patches are captured by patchwork) and because git send-email send the patch as plain email text content it makes it very easy to comment inline in the patch to highlight issues. Some of the SSSD developers already registered into patchwork in order to manage patches. Please feel free to register, in order to allow sssd developers to manage patches from anyone I need to explicitly give maintainership of the project (in patchwork) to your account otherwise you are allowed only to change status about your own patches (so that external contributors can handle their own if they so wish). So feel free to send me an email if you register and need maintainer status to start changing patch status. At the moment the patchwork server is not yet hooked into the master git tree, so it does not detect when patches are finally pushed. So once you push a password please simply change the status to 'Pushed' (you may also set the 'archive' bit but it is not required). You can read more about patchwork following the 'about' link. If you have any question or issue feel free to reply to this thread or contact me directly. Simo. -- Simo Sorce * Red Hat, Inc * New York

2 2

[PATCH 0/1] Better debug logging when saving groups
by Stephen Gallagher 01 Nov '12

01 Nov '12

There are many places in the sdap_save_group() routine where we could exit due to failure without logging a detailed message. This patch adds more log messages in error conditions. Stephen Gallagher (1): LDAP: Better debug logging when saving groups src/providers/ldap/sdap_async_groups.c | 86 +++++++++++++++++++++++++++++----- 1 file changed, 75 insertions(+), 11 deletions(-) -- 1.7.12.1

3 6

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

sssd-devel November 2012