libctf: endianness fixes
authorNick Alcock <nick.alcock@oracle.com>
Wed, 19 Jun 2019 11:34:56 +0000 (12:34 +0100)
committerNick Alcock <nick.alcock@oracle.com>
Fri, 21 Jun 2019 12:04:02 +0000 (13:04 +0100)
Testing of the first code to generate CTF_K_SLICEs on big-endian
revealed a bunch of new problems in this area.  Most importantly, the
trick we did earlier to avoid wasting two bytes on padding in the
ctf_slice_t is best avoided: because it leads to the whole file after
that point no longer being naturally aligned, all multibyte accesses
from then on must use memmove() to avoid unaligned access on platforms
where that is fatal.  In future, this is planned, but for now we are
still doing direct access in many places, so we must revert to making
ctf_slice_t properly aligned for storage in an array.

Rather than wasting bytes on padding, we boost the size of cts_offset
and cts_bits.  This is still a waste of space (we cannot have offsets or
bits in bitfields > 256) but it cannot be avoided for now, and slices
are not so common that this will be a serious problem.

A possibly-worse endianness problem fixed at the same time involves
a codepath used only for foreign-endian, uncompressed CTF files, where
we were not copying the actual CTF data into the buffer, leading to
libctf reading only zeroes (or, possibly, uninitialized garbage).

Finally, when we read in a CTF file, we copy the header and work from
the copy.  We were flipping the endianness of the header copy, and of
the body of the file buffer, but not of the header in the file buffer
itself: so if we write the file back out again we end up with an
unreadable frankenfile with header and body of different endiannesses.
Fix by flipping both copies of the header.

include/
* ctf.h (ctf_slice_t): Make cts_offset and cts_bits unsigned
short, so following structures are properly aligned.

libctf/
* ctf-open.c (get_vbytes_common): Return the new slice size.
(ctf_bufopen): Flip the endianness of the CTF-section header copy.
Remember to copy in the CTF data when opening an uncompressed
foreign-endian CTF file.  Prune useless variable manipulation.

include/ChangeLog
include/ctf.h
libctf/ChangeLog
libctf/ctf-open.c

index 8169b6aae34243aae30988d27235b884e2ebf493..81b667066856334456f40ecb279d11d698e5f1b4 100644 (file)
@@ -1,3 +1,8 @@
+2019-06-19  Nick Alcock <nick.alcock@oracle.com>
+
+       * ctf.h (ctf_slice_t): Make cts_offset and cts_bits unsigned
+       short, so following structures are properly aligned.
+
 2019-06-14  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
        * elf/aarch64.h (R_AARCH64_P32_MOVW_PREL_G0): Define.
index e99a673109c81bb309c4a768606dc2e5e5fb22da..2b357816bafbfee21a91d08decc6d15ae1f6da03 100644 (file)
@@ -430,13 +430,17 @@ union
    ctt_type, which must be a type which has an encoding (fp, int, or enum).  We
    also store the referenced type in here, because it is easier to keep the
    ctt_size correct for the slice than to shuffle the size into here and keep
-   the ctt_type where it is for other types.  */
+   the ctt_type where it is for other types.
+
+   In a future version, where we loosen requirements on alignment in the CTF
+   file, the cts_offset and cts_bits will be chars: but for now they must be
+   shorts or everything after a slice will become unaligned.  */
 
 typedef struct ctf_slice
 {
   uint32_t cts_type;
-  unsigned char cts_offset;
-  unsigned char cts_bits;
+  unsigned short cts_offset;
+  unsigned short cts_bits;
 } ctf_slice_t;
 
 typedef struct ctf_array_v1
index ad5685b69147767dac01875e17da755e22effcfc..0d55ea0776083aaceecf108b2a572207c8244486 100644 (file)
@@ -1,3 +1,10 @@
+2019-06-19  Nick Alcock <nick.alcock@oracle.com>
+
+       * ctf-open.c (get_vbytes_common): Return the new slice size.
+       (ctf_bufopen): Flip the endianness of the CTF-section header copy.
+       Remember to copy in the CTF data when opening an uncompressed
+       foreign-endian CTF file.  Prune useless variable manipulation.
+
 2019-06-19  Nick Alcock <nick.alcock@oracle.com>
 
        * ctf-open.c (ctf_types): Fail when unidentified type kinds are
index 14f7062590abe48f02ed7fb2ec831259d8a77e30..777a6b5ca63ac6dcfcd8a2da59965acdd06a467b 100644 (file)
@@ -150,8 +150,7 @@ get_vbytes_common (unsigned short kind, ssize_t size _libctf_unused_,
     case CTF_K_FLOAT:
       return (sizeof (uint32_t));
     case CTF_K_SLICE:
-      return (offsetof (ctf_slice_t, cts_bits) +
-             sizeof (((ctf_slice_t *)0)->cts_bits));
+      return (sizeof (ctf_slice_t));
     case CTF_K_ENUM:
       return (sizeof (ctf_enum_t) * vlen);
     case CTF_K_FORWARD:
@@ -1208,7 +1207,7 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
   const ctf_preamble_t *pp;
   ctf_header_t hp;
   ctf_file_t *fp;
-  void *buf, *base;
+  void *base;
   size_t size, hdrsz;
   int foreign_endian = 0;
   int err;
@@ -1317,6 +1316,7 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
       uLongf dstlen;
       const void *src;
       int rc = Z_OK;
+      void *buf;
 
       if ((base = ctf_alloc (size + hdrsz)) == NULL)
        return (ctf_set_open_errno (errp, ECTF_ZALLOC));
@@ -1349,12 +1349,16 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect,
     {
       if ((base = ctf_alloc (size + hdrsz)) == NULL)
        return (ctf_set_open_errno (errp, ECTF_ZALLOC));
+      memcpy (base, ctfsect->cts_data, size + hdrsz);
     }
   else
-    {
-      base = (void *) ctfsect->cts_data;
-      buf = (unsigned char *) base + hdrsz;
-    }
+    base = (void *) ctfsect->cts_data;
+
+  /* Flip the endianness of the copy of the header in the section, to avoid
+     ending up with a partially-endian-flipped file.  */
+
+  if (foreign_endian)
+    flip_header ((ctf_header_t *) base);
 
   /* Once we have uncompressed and validated the CTF data buffer, we can
      proceed with allocating a ctf_file_t and initializing it.