[Git][ghc/ghc][ghc-8.8] Fix uninitialized field read in Linker.c

[Ben Gamari]

Ben Gamari pushed to branch ghc-8.8 at Glasgow Haskell Compiler / GHC


Commits:
16f83b57 by Ömer Sinan Ağacan at 2020-09-29T12:39:48-04:00
Fix uninitialized field read in Linker.c

Valgrind report of the bug when running the test `linker_unload`:

    ==29666== Conditional jump or move depends on uninitialised value(s)
    ==29666==    at 0x369C5B4: setOcInitialStatus (Linker.c:1305)
    ==29666==    by 0x369C6C5: mkOc (Linker.c:1347)
    ==29666==    by 0x36C027A: loadArchive_ (LoadArchive.c:522)
    ==29666==    by 0x36C0600: loadArchive (LoadArchive.c:626)
    ==29666==    by 0x2C144CD: ??? (in /home/omer/haskell/ghc_2/testsuite/tests/rts/linker/linker_unload.run/linker_unload)
    ==29666==
    ==29666== Conditional jump or move depends on uninitialised value(s)
    ==29666==    at 0x369C5B4: setOcInitialStatus (Linker.c:1305)
    ==29666==    by 0x369C6C5: mkOc (Linker.c:1347)
    ==29666==    by 0x369C9F6: preloadObjectFile (Linker.c:1507)
    ==29666==    by 0x369CA8D: loadObj_ (Linker.c:1536)
    ==29666==    by 0x369CB17: loadObj (Linker.c:1557)
    ==29666==    by 0x3866BC: main (linker_unload.c:33)

The problem is `mkOc` allocates a new `ObjectCode` and calls
`setOcInitialStatus` without initializing the `status` field.
`setOcInitialStatus` reads the field as first thing:

    static void setOcInitialStatus(ObjectCode* oc) {
        if (oc->status == OBJECT_DONT_RESOLVE)
          return;

        if (oc->archiveMemberName == NULL) {
            oc->status = OBJECT_NEEDED;
        } else {
            oc->status = OBJECT_LOADED;
        }
    }

`setOcInitialStatus` is unsed in two places for two different purposes:
in `mkOc` where we don't have the `status` field initialized yet (`mkOc`
is supposed to initialize it), and `loadOc` where we do have `status`
field initialized and we want to update it. Instead of splitting the
function into two functions which are both called just once I inline the
functions in the use sites and remove it.

Fixes #18342

(cherry picked from commit 08c1cb0f30770acbf366423f085f8ef92f7f6a06)

- - - - -


1 changed file:

- rts/Linker.c


Changes:

=====================================
rts/Linker.c
=====================================
@@ -1238,23 +1238,6 @@ void freeObjectCode (ObjectCode *oc)
     stgFree(oc);
 }
 
-/* -----------------------------------------------------------------------------
-* Sets the initial status of a fresh ObjectCode
-*/
-static void setOcInitialStatus(ObjectCode* oc) {
-    /* If a target has requested the ObjectCode not to be resolved then
-       honor this requests.  Usually this means the ObjectCode has not been
-       initialized and can't be.  */
-    if (oc->status == OBJECT_DONT_RESOLVE)
-      return;
-
-    if (oc->archiveMemberName == NULL) {
-        oc->status = OBJECT_NEEDED;
-    } else {
-        oc->status = OBJECT_LOADED;
-    }
-}
-
 ObjectCode*
 mkOc( pathchar *path, char *image, int imageSize,
       bool mapped, char *archiveMemberName, int misalignment ) {
@@ -1287,7 +1270,11 @@ mkOc( pathchar *path, char *image, int imageSize,
        oc->archiveMemberName = NULL;
    }
 
-   setOcInitialStatus( oc );
+   if (oc->archiveMemberName == NULL) {
+       oc->status = OBJECT_NEEDED;
+   } else {
+       oc->status = OBJECT_LOADED;
+   }
 
    oc->fileSize          = imageSize;
    oc->symbols           = NULL;
@@ -1558,8 +1545,17 @@ HsInt loadOc (ObjectCode* oc)
 #  endif
 #endif
 
-   /* loaded, but not resolved yet, ensure the OC is in a consistent state */
-   setOcInitialStatus( oc );
+   /* Loaded, but not resolved yet, ensure the OC is in a consistent state.
+      If a target has requested the ObjectCode not to be resolved then honor
+      this requests.  Usually this means the ObjectCode has not been initialized
+      and can't be. */
+   if (oc->status != OBJECT_DONT_RESOLVE) {
+       if (oc->archiveMemberName == NULL) {
+           oc->status = OBJECT_NEEDED;
+       } else {
+           oc->status = OBJECT_LOADED;
+       }
+   }
    IF_DEBUG(linker, debugBelch("loadOc: done.\n"));
 
    return 1;



View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/16f83b5799f976b5ca839103fc83628ccc11b01f

-- 
View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/commit/16f83b5799f976b5ca839103fc83628ccc11b01f
You're receiving this email because of your account on gitlab.haskell.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.haskell.org/pipermail/ghc-commits/attachments/20200929/d731596e/attachment-0001.html>