libctf, archive: stop ctf_arc_bufopen triggering crazy unmaps
authorNick Alcock <nick.alcock@oracle.com>
Tue, 2 Jun 2020 19:55:05 +0000 (20:55 +0100)
committerNick Alcock <nick.alcock@oracle.com>
Wed, 22 Jul 2020 16:57:33 +0000 (17:57 +0100)
The archive machinery mmap()s its archives when possible: so it arranges
to do appropriately-sized unmaps by recording the unmap length in the
ctfa_magic value and unmapping that.

This brilliant (horrible) trick works less well when ctf_arc_bufopen is
called with an existing buffer (which might be a readonly mapping).
ctf_arc_bufopen always returns a ctf_archive_t wrapper, so record in
there the necessity to not unmap anything when a bufopen'ed archive is
closed again.

libctf/
* ctf-impl.h (struct ctf_archive_internal)
<ctfi_unmap_on_close>: New.
(ctf_new_archive_internal): Adjust.
* ctf-archive.c (ctf_new_archive_internal): Likewise.
Initialize ctfi_unmap_on_close.  Adjust error path.
(ctf_arc_bufopen): Adjust ctf_new_archive_internal call
(unmap_on_close is 0).
(ctf_arc_close): Only unmap if ctfi_unmap_on_close.
* ctf-open-bfd.c (ctf_fdopen): Adjust.

libctf/ChangeLog
libctf/ctf-archive.c
libctf/ctf-impl.h
libctf/ctf-open-bfd.c

index f0e20011f39faad421d5991ef80a1bd1d4bb2197..da285022dc136b46ebe8153a029c38178b044b9e 100644 (file)
@@ -1,3 +1,15 @@
+2020-07-22  Nick Alcock  <nick.alcock@oracle.com>
+
+       * ctf-impl.h (struct ctf_archive_internal)
+       <ctfi_unmap_on_close>: New.
+       (ctf_new_archive_internal): Adjust.
+       * ctf-archive.c (ctf_new_archive_internal): Likewise.
+       Initialize ctfi_unmap_on_close.  Adjust error path.
+       (ctf_arc_bufopen): Adjust ctf_new_archive_internal call
+       (unmap_on_close is 0).
+       (ctf_arc_close): Only unmap if ctfi_unmap_on_close.
+       * ctf-open-bfd.c (ctf_fdopen): Adjust.
+
 2020-07-22  Nick Alcock  <nick.alcock@oracle.com>
 
        * ctf-types.c (ctf_type_aname): Return ECTF_CORRUPT if
index d857c0109c8d60a6b9af23f6913e3fc7b43b8bfa..3c14d7d12f917850a18ff2f6366ad75abdbcab61 100644 (file)
@@ -336,10 +336,11 @@ search_modent_by_name (const void *key, const void *ent, void *arg)
 
 /* Make a new struct ctf_archive_internal wrapper for a ctf_archive or a
    ctf_file.  Closes ARC and/or FP on error.  Arrange to free the SYMSECT or
-   STRSECT, as needed, on close.  */
+   STRSECT, as needed, on close.  Possibly do not unmap on close.  */
 
 struct ctf_archive_internal *
-ctf_new_archive_internal (int is_archive, struct ctf_archive *arc,
+ctf_new_archive_internal (int is_archive, int unmap_on_close,
+                         struct ctf_archive *arc,
                          ctf_file_t *fp, const ctf_sect_t *symsect,
                          const ctf_sect_t *strsect,
                          int *errp)
@@ -349,7 +350,10 @@ ctf_new_archive_internal (int is_archive, struct ctf_archive *arc,
   if ((arci = calloc (1, sizeof (struct ctf_archive_internal))) == NULL)
     {
       if (is_archive)
-       ctf_arc_close_internal (arc);
+       {
+         if (unmap_on_close)
+           ctf_arc_close_internal (arc);
+       }
       else
        ctf_file_close (fp);
       return (ctf_set_open_errno (errp, errno));
@@ -364,6 +368,7 @@ ctf_new_archive_internal (int is_archive, struct ctf_archive *arc,
   if (strsect)
      memcpy (&arci->ctfi_strsect, strsect, sizeof (struct ctf_sect));
   arci->ctfi_free_symsect = 0;
+  arci->ctfi_unmap_on_close = unmap_on_close;
 
   return arci;
 }
@@ -382,7 +387,13 @@ ctf_arc_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
   if (ctfsect->cts_size > sizeof (uint64_t) &&
       ((*(uint64_t *) ctfsect->cts_data) == CTFA_MAGIC))
     {
-      /* The archive is mmappable, so this operation is trivial.  */
+      /* The archive is mmappable, so this operation is trivial.
+
+        This buffer is nonmodifiable, so the trick involving mmapping only part
+        of it and storing the length in the magic number is not applicable: so
+        record this fact in the archive-wrapper header.  (We cannot record it
+        in the archive, because the archive may very well be a read-only
+        mapping.)  */
 
       is_archive = 1;
       arc = (struct ctf_archive *) ctfsect->cts_data;
@@ -397,7 +408,7 @@ ctf_arc_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
          return NULL;
        }
     }
-  return ctf_new_archive_internal (is_archive, arc, fp, symsect, strsect,
+  return ctf_new_archive_internal (is_archive, 0, arc, fp, symsect, strsect,
                                   errp);
 }
 
@@ -474,7 +485,10 @@ ctf_arc_close (ctf_archive_t *arc)
     return;
 
   if (arc->ctfi_is_archive)
-    ctf_arc_close_internal (arc->ctfi_archive);
+    {
+      if (arc->ctfi_unmap_on_close)
+       ctf_arc_close_internal (arc->ctfi_archive);
+    }
   else
     ctf_file_close (arc->ctfi_file);
   if (arc->ctfi_free_symsect)
index fdd48f0d3316b3313143a6bed1560222f329de01..4661aa8c7a9cf9c56dab969eed72c4afe9276f65 100644 (file)
@@ -312,6 +312,7 @@ struct ctf_file
 struct ctf_archive_internal
 {
   int ctfi_is_archive;
+  int ctfi_unmap_on_close;
   ctf_file_t *ctfi_file;
   struct ctf_archive *ctfi_archive;
   ctf_sect_t ctfi_symsect;
@@ -443,10 +444,11 @@ extern void ctf_str_rollback (ctf_file_t *, ctf_snapshot_id_t);
 extern void ctf_str_purge_refs (ctf_file_t *);
 extern ctf_strs_writable_t ctf_str_write_strtab (ctf_file_t *);
 
-extern struct ctf_archive_internal *ctf_new_archive_internal
-       (int is_archive, struct ctf_archive *arc,
-        ctf_file_t *fp, const ctf_sect_t *symsect,
-        const ctf_sect_t *strsect, int *errp);
+extern struct ctf_archive_internal *
+ctf_new_archive_internal (int is_archive, int unmap_on_close,
+                         struct ctf_archive *, ctf_file_t *,
+                         const ctf_sect_t *symsect,
+                         const ctf_sect_t *strsect, int *errp);
 extern struct ctf_archive *ctf_arc_open_internal (const char *, int *);
 extern void ctf_arc_close_internal (struct ctf_archive *);
 extern void *ctf_set_open_errno (int *, int);
index dafa265f9d821510fc7375fb746c532f49de8448..2d2d572c88fe75110688edfe000a35887b65c8e1 100644 (file)
@@ -230,7 +230,7 @@ ctf_fdopen (int fd, const char *filename, const char *target, int *errp)
       fp->ctf_data_mmapped = data;
       fp->ctf_data_mmapped_len = (size_t) st.st_size;
 
-      return ctf_new_archive_internal (0, NULL, fp, NULL, NULL, errp);
+      return ctf_new_archive_internal (0, 1, NULL, fp, NULL, NULL, errp);
     }
 
   if ((nbytes = ctf_pread (fd, &arc_magic, sizeof (arc_magic), 0)) <= 0)
@@ -243,7 +243,7 @@ ctf_fdopen (int fd, const char *filename, const char *target, int *errp)
       if ((arc = ctf_arc_open_internal (filename, errp)) == NULL)
        return NULL;                    /* errno is set for us.  */
 
-      return ctf_new_archive_internal (1, arc, NULL, NULL, NULL, errp);
+      return ctf_new_archive_internal (1, 1, arc, NULL, NULL, NULL, errp);
     }
 
   /* Attempt to open the file with BFD.  We must dup the fd first, since bfd