[ovs-dev] [PATCH] vlog: Fix a deadlock bug.

Daniele Di Proietto diproiettod at vmware.com
Thu Nov 19 19:30:01 UTC 2015


Thanks for the fix, Andy

Acked-by: Daniele Di Proietto <diproiettod at vmware.com>

On 15/11/2015 20:59, "Andy Zhou" <azhou at nicira.com> wrote:

>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 at vmware.com>
>Signed-off-by: Andy Zhou <azhou at nicira.com>
>---
> lib/vlog.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
>diff --git a/lib/vlog.c b/lib/vlog.c
>index a4aa2a0..28cea5d 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
> 
>-- 
>1.9.1
>
>_______________________________________________
>dev mailing list
>dev at openvswitch.org
>https://urldefense.proofpoint.com/v2/url?u=http-3A__openvswitch.org_mailma
>n_listinfo_dev&d=BQIGaQ&c=Sqcl0Ez6M0X8aeM67LKIiDJAXVeAw-YihVMNtXt-uEs&r=Sm
>B5nZacmXNq0gKCC1s_Cw5yUNjxgD4v5kJqZ2uWLlE&m=z501EObTNBEFvIFC6CiugNNwuDj1_Q
>EkKFHBzmREoC4&s=XawONBR44io3PX3YhsJonjY8eTVa5qWu3O2Ud3eEVyc&e= 




More information about the dev mailing list