[SSSD] [PATCH] get_next_domain() test dom->parent->next for NULL

Jakub Hrozek jhrozek at redhat.com
Thu Feb 14 18:31:51 UTC 2013


On Thu, Feb 14, 2013 at 12:21:06PM -0500, Simo Sorce wrote:
> On Thu, 2013-02-14 at 14:07 +0100, Jakub Hrozek wrote:
> > On Tue, Feb 12, 2013 at 01:14:17PM +0100, Jakub Hrozek wrote:
> > > On Tue, Feb 12, 2013 at 11:27:11AM +0100, Pavel Březina wrote:
> > > > This is a fix for recent simo's patches.
> > > > 
> > > > I have one domain and one subdomain. When I tried to su aduser at addom a
> > > > responder crashed. NSS responder crash during the first try, PAM
> > > > responder during seconds. Log and patch is attached.
> > > 
> > > Interesting, I tested this case, but maybe the setup was different..oh
> > > well..
> > > 
> > > 
> > > > From cfc155edf5be8b65532b5e6c2302859c4194fcfd Mon Sep 17 00:00:00 2001
> > > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina at redhat.com>
> > > > Date: Tue, 12 Feb 2013 11:12:59 +0100
> > > > Subject: [PATCH] get_next_domain() test dom->parent->next for NULL
> > > > 
> > > > Otherwise dom may be set to NULL and we will segfault when
> > > > dereferencing dom.
> > > > ---
> > > >  src/util/domain_info_utils.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/src/util/domain_info_utils.c b/src/util/domain_info_utils.c
> > > > index d8ca55da141e7f91d58b18921696f8ac490c92f6..039d66f868d4326101eace3108cb21cc747a8c7d 100644
> > > > --- a/src/util/domain_info_utils.c
> > > > +++ b/src/util/domain_info_utils.c
> > > > @@ -33,7 +33,7 @@ struct sss_domain_info *get_next_domain(struct sss_domain_info *domain,
> > > >              dom = dom->subdomains;
> > > >          } else if (dom->next) {
> > > >              dom = dom->next;
> > > > -        } else if (descend && IS_SUBDOMAIN(dom)) {
> > > > +        } else if (descend && IS_SUBDOMAIN(dom) && dom->parent->next) {
> > > >              dom = dom->parent->next;
> > > >          } else {
> > > >              return NULL;
> > > > -- 
> > > > 1.7.11.7
> > > > 
> > > 
> > > I think the patch makes sense. Unless simo has any reservations, I'll
> > > push it.
> > 
> > Ack and pushed to master.
> 
> we could have changed the code to set dom = NULL in the last branch
> where we have return NULL; right now and then change the dom->disabled
> check to be if (!dom || !dom->disabled) break;
> 
> It reads cleaner to me, but it is not a big deal, and I am late so I do
> not get to vote anymore :)
> 

We should strive for a more readable code nevertheless:
https://fedorahosted.org/sssd/ticket/1812



More information about the sssd-devel mailing list