Hi Martin,
I have confirmed your suggestion works. The patch tested is attached.
Will this fix be accepted into the public release, if not what is the
way forward?
Cheers,
Alex
On 11/06/12 21:19, Martin Simmons wrote:
The code is broken (not thread-safe).
Try moving the locks furthur out, i.e. move
lock_jobs();
LockRes();
before
if (already_here) {
and move
UnlockRes();
unlock_jobs();
after
already_here = false;
__Martin
On Sat, 09 Jun 2012 11:55:43 +1000, Alexander Wigen said:
Hi Bacula developers,
I've opened up a bug for this:
http://bugs.bacula.org/view.php?id=1889
I'll happily provide a patch for this, but I can't until I understand
what sort of recursion the original author was worried about.
Sincerely,
Alexander Wigen
On 06/06/12 22:02, Alexander Wigen wrote:
Hi Bacula developers,
We have stumbled on a race condition where two concurrent reload request
can cause bacula-dir to abort().
The following is enough to crash bacula-dir in most cases:
(echo reload | bconsole)& (echo reload | bconsole)&
One of our developers Jeremy Peterson has been working on this problem and
has traced the abort statement to dird.c:
void reload_config(int sig)
static bool already_here = false;
if (already_here) {
abort(); /* Oops, recursion -> die */
}
Patching Bacula so that the abort() doesn't fire mitigates the race
condition and the director doesn't die. This piece of code has not changed
since since moving from cvs to git (in 2002).
I assume the abort statement is there for a reason, to stop recursion, but
we are not sure what would cause this recursion and therefore I'm asking
what the correct fix for this bug should be.
Sincerely,
Alexander Wigen
diff --git a/bacula/src/dird/dird.c b/bacula/src/dird/dird.c
index 4a1b3ed..9a822b3 100644
--- a/bacula/src/dird/dird.c
+++ b/bacula/src/dird/dird.c
@@ -502,6 +502,8 @@ void reload_config(int sig)
int table, rtable;
bool ok;
+ lock_jobs();
+ LockRes();
if (already_here) {
abort(); /* Oops, recursion -> die */
}
@@ -513,8 +515,6 @@ void reload_config(int sig)
sigprocmask(SIG_BLOCK, &set, NULL);
#endif
- lock_jobs();
- LockRes();
table = find_free_reload_table_entry();
if (table < 0) {
@@ -573,13 +573,13 @@ void reload_config(int sig)
}
bail_out:
- UnlockRes();
- unlock_jobs();
#if !defined(HAVE_WIN32)
sigprocmask(SIG_UNBLOCK, &set, NULL);
signal(SIGHUP, reload_config);
#endif
already_here = false;
+ UnlockRes();
+ unlock_jobs();
}
/*
------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and
threat landscape has changed and how IT managers can respond. Discussions
will include endpoint security, mobile security and the latest in malware
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Bacula-devel mailing list
Bacula-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bacula-devel
