ldap/servers/slapd/slap.h | 2 ++ ldap/servers/slapd/task.c | 21 +++++++++++++++++---- 2 files changed, 19 insertions(+), 4 deletions(-)
New commits: commit f4cce0c2e74bad7e12baeac993b0dc5e1ad55e6c Author: Noriko Hosoi nhosoi@redhat.com Date: Tue Mar 16 16:45:33 2010 -0700
573896 - initializing subtree with invalid syntax crashes ns-slapd
https://bugzilla.redhat.com/show_bug.cgi?id=573896
Description: When an import is executed using a task mechanism, slapi_task_log_notice is called for logging, where task_log field points the memory storing the log messages. If multiple log messages were logged by multiple worker threads simultaneously, there was a chance that the address of the log message was switched by realloc while the other threads were accessing the old address. This patch introduces task_log_lock per task to protect task_log. Note: slapi_ch_malloc and its friends never return NULL. They rather exits. Thus, to avoid the confusion which may look leaking the lock, I eliminated 2 error returns from slapi_task_log_notice.
diff --git a/ldap/servers/slapd/slap.h b/ldap/servers/slapd/slap.h index 9a860e9..062d669 100644 --- a/ldap/servers/slapd/slap.h +++ b/ldap/servers/slapd/slap.h @@ -1359,6 +1359,8 @@ struct slapi_task { TaskCallbackFn cancel; /* task has been cancelled by user */ TaskCallbackFn destructor; /* task entry is being destroyed */ int task_refcount; + PRLock *task_log_lock; /* To protect task_log to be realloced if + it's in use */ } slapi_task; /* End of interface to support online tasks **********************************/
diff --git a/ldap/servers/slapd/task.c b/ldap/servers/slapd/task.c index 3324dea..67109b2 100644 --- a/ldap/servers/slapd/task.c +++ b/ldap/servers/slapd/task.c @@ -228,6 +228,9 @@ void slapi_task_log_notice(Slapi_Task *task, char *format, ...) PR_vsnprintf(buffer, LOG_BUFFER, format, ap); va_end(ap);
+ if (task->task_log_lock) { + PR_Lock(task->task_log_lock); + } len = 2 + strlen(buffer) + (task->task_log ? strlen(task->task_log) : 0); if ((len > MAX_SCROLLBACK_BUFFER) && task->task_log) { size_t i; @@ -241,8 +244,6 @@ void slapi_task_log_notice(Slapi_Task *task, char *format, ...) i++; len = strlen(task->task_log) - i + 2 + strlen(buffer); newbuf = (char *)slapi_ch_malloc(len); - if (! newbuf) - return; /* out of memory? */ strcpy(newbuf, task->task_log + i); slapi_ch_free((void **)&task->task_log); task->task_log = newbuf; @@ -253,13 +254,14 @@ void slapi_task_log_notice(Slapi_Task *task, char *format, ...) } else { task->task_log = (char *)slapi_ch_realloc(task->task_log, len); } - if (! task->task_log) - return; /* out of memory? */ }
if (task->task_log[0]) strcat(task->task_log, "\n"); strcat(task->task_log, buffer); + if (task->task_log_lock) { + PR_Unlock(task->task_log_lock); + }
slapi_task_status_changed(task); } @@ -278,7 +280,13 @@ void slapi_task_status_changed(Slapi_Task *task) return; }
+ if (task->task_log_lock) { + PR_Lock(task->task_log_lock); + } NEXTMOD(TASK_LOG_NAME, task->task_log); + if (task->task_log_lock) { + PR_Unlock(task->task_log_lock); + } NEXTMOD(TASK_STATUS_NAME, task->task_status); sprintf(s1, "%d", task->task_exitcode); sprintf(s2, "%d", task->task_progress); @@ -505,6 +513,8 @@ new_task(const char *dn) slapi_config_register_callback(SLAPI_OPERATION_ADD, DSE_FLAG_PREOP, dn, LDAP_SCOPE_SUBTREE, "(objectclass=*)", task_deny, NULL); #endif + /* To protect task_log to be realloced if it's in use */ + task->task_log_lock = PR_NewLock();
return task; } @@ -652,6 +662,9 @@ static void task_generic_destructor(Slapi_Task *task) if (task->task_status) { slapi_ch_free((void **)&task->task_status); } + if (task->task_log_lock) { + PR_DestroyLock(task->task_log_lock); + } task->task_log = task->task_status = NULL; }
389-commits@lists.fedoraproject.org