diff options
author | Noriko Hosoi <nhosoi@jiji.localdomain> | 2009-09-23 15:31:35 -0700 |
---|---|---|
committer | Noriko Hosoi <nhosoi@jiji.localdomain> | 2009-09-23 15:31:35 -0700 |
commit | f2618f28c1c9637217ac27d64614bb5fa38f82ed (patch) | |
tree | a251f92489e97d6a8b475c4de7a5169b848f6065 /lib | |
parent | 3d735f37cf613e68e10ab916f6752fbe3ffc0e1a (diff) | |
download | ds-f2618f28c1c9637217ac27d64614bb5fa38f82ed.tar.gz ds-f2618f28c1c9637217ac27d64614bb5fa38f82ed.tar.xz ds-f2618f28c1c9637217ac27d64614bb5fa38f82ed.zip |
518112 rhds 81 el53 64b ns-slapd seg fault
Fixing the contention over LAS_cookie.
Considering the case 2 threads try to evaluate the IP/DNS aci almost at the
same time, one comes in first and creates context in the critical section
(between ACL_CritEnter and ACL_CritExit); another thread sees *LAS_cookie
is non NULL and assumes the context is already made. But it could be half
baked then since the second thread does not respect the critical section.
This patch is putting the line assigning *LAS_cookie to context into the
critical section, which prevents for the second thread to pick up the half
baked *LAS_cookie.
Fix proposed in the comment#19 by Rich Megginson is included, as well:
Because what if *LAS_cookie is set to a valid value after the first if() test
and before the call to ACL_CritEnter(); ? There is similar code in LASIpEval()
too.
Diffstat (limited to 'lib')
-rw-r--r-- | lib/libaccess/lasdns.cpp | 21 | ||||
-rw-r--r-- | lib/libaccess/lasip.cpp | 17 |
2 files changed, 24 insertions, 14 deletions
diff --git a/lib/libaccess/lasdns.cpp b/lib/libaccess/lasdns.cpp index de7d2a94..b345d1ba 100644 --- a/lib/libaccess/lasdns.cpp +++ b/lib/libaccess/lasdns.cpp @@ -368,21 +368,26 @@ int LASDnsEval(NSErr_t *errp, char *attr_name, CmpOp_t comparator, /* If this is the first time through, build the pattern tree first. */ if (*LAS_cookie == NULL) { - ACL_CritEnter(); + ACL_CritEnter(); if (*LAS_cookie == NULL) { /* Must check again */ *LAS_cookie = context = (LASDnsContext_t *)PERM_MALLOC(sizeof(LASDnsContext_t)); if (context == NULL) { - nserrGenerate(errp, ACLERRNOMEM, ACLERR4820, ACL_Program, 1, XP_GetAdminStr(DBT_lasdnsevalUnableToAllocateContex_)); - ACL_CritExit(); + nserrGenerate(errp, ACLERRNOMEM, ACLERR4820, ACL_Program, 1, XP_GetAdminStr(DBT_lasdnsevalUnableToAllocateContex_)); + ACL_CritExit(); return LAS_EVAL_FAIL; } - context->Table = NULL; - LASDnsBuild(errp, attr_pattern, context, aliasflg); - } - ACL_CritExit(); - } else + context->Table = NULL; + LASDnsBuild(errp, attr_pattern, context, aliasflg); + } else { + context = (LASDnsContext *) *LAS_cookie; + } + ACL_CritExit(); + } else { + ACL_CritEnter(); context = (LASDnsContext *) *LAS_cookie; + ACL_CritExit(); + } /* Call the DNS attribute getter */ #ifdef UTEST diff --git a/lib/libaccess/lasip.cpp b/lib/libaccess/lasip.cpp index 01c76aa6..0f3964d4 100644 --- a/lib/libaccess/lasip.cpp +++ b/lib/libaccess/lasip.cpp @@ -493,23 +493,28 @@ int LASIpEval(NSErr_t *errp, char *attr_name, CmpOp_t comparator, ACL_CritEnter(); if (*LAS_cookie == NULL) { /* must check again */ *LAS_cookie = context = - (LASIpContext_t *)PERM_MALLOC(sizeof(LASIpContext_t)); + (LASIpContext_t *)PERM_MALLOC(sizeof(LASIpContext_t)); if (context == NULL) { - nserrGenerate(errp, ACLERRNOMEM, ACLERR5230, ACL_Program, 1, XP_GetAdminStr(DBT_lasipevalUnableToAllocateContext_)); + nserrGenerate(errp, ACLERRNOMEM, ACLERR5230, ACL_Program, 1, XP_GetAdminStr(DBT_lasipevalUnableToAllocateContext_)); ACL_CritExit(); return LAS_EVAL_FAIL; } context->treetop = NULL; retcode = LASIpBuild(errp, attr_name, comparator, attr_pattern, - &context->treetop); + &context->treetop); if (retcode) { ACL_CritExit(); return (retcode); - } + } + } else { + context = (LASIpContext *) *LAS_cookie; } - ACL_CritExit(); - } else + ACL_CritExit(); + } else { + ACL_CritEnter(); context = (LASIpContext *) *LAS_cookie; + ACL_CritExit(); + } node = context->treetop; |