summaryrefslogtreecommitdiff
path: root/lib/vlog.c
diff options
context:
space:
mode:
authorAndy Zhou <azhou@von.org>2015-11-13 18:39:37 -0800
committerAndy Zhou <azhou@ovn.org>2015-11-19 13:13:20 -0800
commite9d6808ca6bfeb6b8d1ca30113c186098e4d2d56 (patch)
tree4f8c0c7024e1d5cb7d439cff6335a04e66126493 /lib/vlog.c
parent10d8e9c6719530a317dfe657442b10023f2aec5d (diff)
downloadopenvswitch-e9d6808ca6bfeb6b8d1ca30113c186098e4d2d56.tar.gz
vlog: Fix a deadlock bug.
Calling VLOG_FATAL() while holding the 'log_file_mutex" may lead to deadlock since VLOG_FATAL() implementation tries to acquire the same lock. Fix this by building the error message first, then call VLOG_FATAL() after the 'log_file_mutex' has been released. This bug is not likely show up in practice since chown() usually won't fail. It is still better to have a correct implementation. Reported-by: Daniele Di Proietto <ddiproietto@vmware.com> Signed-off-by: Andy Zhou <azhou@ovn.org> Acked-by: Daniele Di Proietto <ddiproietto@vmware.com>
Diffstat (limited to 'lib/vlog.c')
-rw-r--r--lib/vlog.c19
1 files changed, 14 insertions, 5 deletions
diff --git a/lib/vlog.c b/lib/vlog.c
index a4aa2a090..28cea5ddb 100644
--- a/lib/vlog.c
+++ b/lib/vlog.c
@@ -438,15 +438,24 @@ vlog_reopen_log_file(void)
void
vlog_change_owner_unix(uid_t user, gid_t group)
{
- ovs_mutex_lock(&log_file_mutex);
- int error = log_file_name ? chown(log_file_name, user, group) : 0;
+ struct ds err = DS_EMPTY_INITIALIZER;
+ int error;
+ ovs_mutex_lock(&log_file_mutex);
+ error = log_file_name ? chown(log_file_name, user, group) : 0;
if (error) {
- VLOG_FATAL("Failed to change %s ownership: %s.",
- log_file_name, ovs_strerror(errno));
+ /* Build the error message. We can not call VLOG_FATAL directly
+ * here because VLOG_FATAL() will try again to to acquire
+ * 'log_file_mutex' lock, causing deadlock.
+ */
+ ds_put_format(&err, "Failed to change %s ownership: %s.",
+ log_file_name, ovs_strerror(errno));
}
-
ovs_mutex_unlock(&log_file_mutex);
+
+ if (error) {
+ VLOG_FATAL("%s", ds_steal_cstr(&err));
+ }
}
#endif