From: Kruti Date: Tue, 21 May 2024 05:58:12 +0000 (-0700) Subject: Fix LOCK_EVASION issue found by Coverity scan. X-Git-Tag: stable-12.5.0~79 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=31894dfbfcd91efe5a2712c2f33ce9114d942e11;p=thirdparty%2Fopen-vm-tools.git Fix LOCK_EVASION issue found by Coverity scan. fileLogger.c -- 2 issues reported in file issue: MultiReader/SingleWriter lock race conditions between assign and check. fix: Mitigation more than fix. issue: Coverity seems confused by the MR/SW lock, but there is some data field assignment performed under the wrong lock to clean up. fix: Move assignment made under Read lock to Write lock. Moved setting the data->error status inside of writer lock block. Added re-checking the data->error status at reader -> writer and writer -> reader lock transitions. --- diff --git a/open-vm-tools/vgauth/service/fileLogger.c b/open-vm-tools/vgauth/service/fileLogger.c index e38007700..49f60da43 100644 --- a/open-vm-tools/vgauth/service/fileLogger.c +++ b/open-vm-tools/vgauth/service/fileLogger.c @@ -1,5 +1,6 @@ /********************************************************* - * Copyright (C) 2011-2019 VMware, Inc. All rights reserved. + * Copyright (c) 2011-2024 Broadcom. All rights reserved. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU Lesser General Public License as published @@ -233,14 +234,17 @@ ServiceFileLogger_Log(const gchar *domain, */ g_rw_lock_reader_unlock(&data->lock); g_rw_lock_writer_lock(&data->lock); - if (data->file == NULL) { + if (data->file == NULL && !data->error) { data->file = ServiceFileLoggerOpen(data); + if (data->file == NULL) { + data->error = TRUE; + fprintf(stderr, "Unable to open log file %s\n", data->path); + } } g_rw_lock_writer_unlock(&data->lock); g_rw_lock_reader_lock(&data->lock); - if (data->file == NULL) { - data->error = TRUE; - fprintf(stderr, "Unable to open log file %s\n", data->path); + if (data->error) { + /* Error set here or in another thread */ goto exit; } } @@ -258,10 +262,15 @@ ServiceFileLogger_Log(const gchar *domain, /* Drop the reader lock, grab the writer lock and re-check. */ g_rw_lock_reader_unlock(&data->lock); g_rw_lock_writer_lock(&data->lock); - if (g_atomic_int_get(&data->logSize) >= data->maxSize) { + if (!data->error && data->file != NULL && + g_atomic_int_get(&data->logSize) >= data->maxSize) { fclose(data->file); data->append = FALSE; data->file = ServiceFileLoggerOpen(data); + if (data->file == NULL) { + data->error = TRUE; + fprintf(stderr, "Unable to reopen log file %s\n", data->path); + } } g_rw_lock_writer_unlock(&data->lock); g_rw_lock_reader_lock(&data->lock);