Thanks for the fix, Andy
Acked-by: Daniele Di Proietto <diproiet...@vmware.com>
On 15/11/2015 20:59, "Andy Zhou" <az...@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 <ddiproie...@vmware.com>
>Signed-off-by: Andy Zhou <az...@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@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=
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
