]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
[Coverity]: Fixes for issues found from static application security testing
authorKruti <kpendharkar@vmware.com>
Fri, 7 Jun 2024 16:55:04 +0000 (09:55 -0700)
committerKruti <kpendharkar@vmware.com>
Fri, 7 Jun 2024 16:55:04 +0000 (09:55 -0700)
Adding coverity escapes for false-positive issues.

hgfsServerParameters.c -- 1 issue reported.
issue: Overrunning array of 5 bytes at byte offset 5 by dereferencing
       pointer "newName".
impact: False-Positive
fix: suppress 'overrun-local'

vmhgfs-fuse/file.c -- 2 issues reported.
issue: Overrunning array of n bytes at byte offset n by dereferencing
       pointer "newNameP" (n is 17 and 5 respectively for those 2 locations
       where the issue occured).
impact: False-Positive
fix: suppress 'overrun-local'

vmhgfs-fuse/link.c -- 2 issues reported.
issue: Overrunning array of n bytes at byte offset n by dereferencing
       pointer "fileNameP" (n is 17 and 5 respectively for those 2 locations
       where the issue occured).
impact: False-Positive
fix: suppress 'overrun-local'

vmhgfs-fuse/transport.c -- 1 issue reported.
issue: uninit_use_in_call: Using uninitialized value "reply" while calling
       HgfsCompleteReq() function.
impact: Bug
fix: Remove function, it is unused/dead code (transport.h too).

open-vm-tools/lib/hgfsServer/hgfsServerParameters.c
open-vm-tools/vmhgfs-fuse/file.c
open-vm-tools/vmhgfs-fuse/link.c
open-vm-tools/vmhgfs-fuse/transport.c
open-vm-tools/vmhgfs-fuse/transport.h

index 8b601829c0eb09af961f2565d37ab409355662cb..9836a3feab137f598c4d047f8ba714ff58ccf528 100644 (file)
@@ -1,5 +1,6 @@
 /*********************************************************
- * Copyright (C) 2010-2019 VMware, Inc. All rights reserved.
+ * Copyright (c) 2010-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
@@ -1968,6 +1969,13 @@ HgfsUnpackRenamePayloadV2(const HgfsRequestRenameV2 *requestV2, // IN: request p
    } else {
       newName = (const HgfsFileName *)((char *)(&requestV2->oldName + 1)
                                        + *cpOldNameLen);
+      /*
+       * The HgfsRequestRenameV2 structure overlay on the data has the old and
+       * new data interlaced rather. The newName pointer in the data is
+       * calculated as an offset from the oldName field. This confuses Coverity,
+       * there is no overrun here.
+       */
+      /* coverity[overrun-local] */
       if (!HgfsUnpackFileName(newName,
                               extra,
                               cpNewName,
index f0323095ab4f805673eafd91bdd556bbe7c0d919..b9b9a933fedd021f7d7679b161886be70a8f2ad3 100644 (file)
@@ -1,5 +1,6 @@
 /*********************************************************
- * Copyright (c) 2013,2018-2019, 2023 VMware, Inc. All rights reserved.
+ * Copyright (c) 2013-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
@@ -1038,6 +1039,11 @@ retry:
          result = -EINVAL;
          goto out;
       }
+      /*
+       * The usage of the space allocated in req early in the function is kept
+       * in reqSize. If oldName length was 0 we're not causing an overrun.
+       */
+      /* coverity[overrun-local] */
       newNameP->length = result;
       reqSize += result;
       newNameP->flags = 0;
@@ -1060,6 +1066,11 @@ retry:
          result = -EINVAL;
          goto out;
       }
+      /*
+       * The usage of the space allocated in req early in the function is kept
+       * in reqSize. If oldName length was 0 we're not causing an overrun.
+       */
+      /* coverity[overrun-local] */
       newNameP->length = result;
       reqSize += result;
    }
index 3a6fc912c4d9ecf3ceafba65725f1d754a6e06a8..9a607ca51d5bc7a747a6964695a10e2b11107df8 100644 (file)
@@ -1,5 +1,6 @@
 /*********************************************************
- * Copyright (C) 2013,2019 VMware, Inc. All rights reserved.
+ * Copyright (c) 2013-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
@@ -90,6 +91,12 @@ HgfsPackSymlinkCreateRequest(const char* symlink,     // IN: path of the link
       LOG(6, ("Target name: \"%s\"\n", fileNameP->name));
       /* Convert target name to CPName-lite format. */
       CPNameLite_ConvertTo(fileNameP->name, targetNameBytes - 1, '/');
+      /*
+       * The req size is always sufficient to hold the request data.
+       * There is no overrun here, coverity has issue with how the data is
+       * packed (name fields data are interlaced).
+       */
+      /* coverity[overrun-local] */
       fileNameP->length = targetNameBytes - 1;
       fileNameP->flags = 0;
       fileNameP->fid = HGFS_INVALID_HANDLE;
@@ -125,6 +132,12 @@ HgfsPackSymlinkCreateRequest(const char* symlink,     // IN: path of the link
       LOG(6, ("Target name: \"%s\"\n", fileNameP->name));
       /* Convert target name to CPName-lite format. */
       CPNameLite_ConvertTo(fileNameP->name, targetNameBytes - 1, '/');
+      /*
+       * The req size is always sufficient to hold the request data.
+       * There is no overrun here, coverity has issue with how the data is
+       * packed (name fields data are interlaced).
+       */
+      /* coverity[overrun-local] */
       fileNameP->length = targetNameBytes - 1;
       break;
    }
index 22fa5a0700b4ee67931207fb9abd7f90afb166da..4ba451a5d60b1f69cc942a4f2e82b76cb2c5bb25 100644 (file)
@@ -1,5 +1,6 @@
 /*********************************************************
- * Copyright (C) 2013,2019 VMware, Inc. All rights reserved.
+ * Copyright (c) 2013-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
@@ -266,42 +267,6 @@ HgfsTransportProcessPacket(char *receivedPacket,    //IN: received packet
 }
 
 
-/*
- *----------------------------------------------------------------------
- *
- * HgfsTransportBeforeExitingRecvThread --
- *
- *     The cleanup work to do before the recv thread exits, including
- *     completing pending requests with error.
- *
- * Results:
- *     None
- *
- * Side effects:
- *     None
- *
- *----------------------------------------------------------------------
- */
-
-void
-HgfsTransportBeforeExitingRecvThread(void)
-{
-   struct list_head *cur, *next;
-
-   /* Walk through gHgfsPendingRequests queue and reply them with error. */
-   pthread_mutex_lock(&gHgfsPendingRequestsLock);
-   list_for_each_safe(cur, next, &gHgfsPendingRequests) {
-      HgfsReq *req;
-      HgfsReply reply;
-
-      req = list_entry(cur, HgfsReq, list);
-      LOG(6, ("Injecting error reply to req id: %d\n", req->id));
-      HgfsCompleteReq(req, (char *)&reply, sizeof reply);
-   }
-   pthread_mutex_unlock(&gHgfsPendingRequestsLock);
-}
-
-
 /*
  *----------------------------------------------------------------------
  *
index e8d92e05be611612ea3647cd458c1e9aef0c32e3..49b8677c5767ce0b9d5ff3af698f132241b7c35c 100644 (file)
@@ -1,5 +1,6 @@
 /*********************************************************
- * Copyright (C) 2013 VMware, Inc. All rights reserved.
+ * Copyright (c) 2013-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
@@ -58,6 +59,5 @@ void HgfsTransportExit(void);
 int HgfsTransportSendRequest(HgfsReq *req);
 void HgfsTransportProcessPacket(char *receivedPacket,
                                 size_t receivedSize);
-void HgfsTransportBeforeExitingRecvThread(void);
 
 #endif // _HGFS_DRIVER_TRANSPORT_H_