Message ID | b2987a1148ecf4f2a6e697fae3617effcdbf88db.1601569371.git.fweimer@redhat.com |
---|---|
State | New |
Headers | show |
Series | glibc-hwcaps support | expand |
On 01/10/2020 13:33, Florian Weimer via Libc-alpha wrote: > Use a reserved byte in the new format cache header to indicate whether > the file is in little endian or big endian format. Eventually, this > information could be used to provide a unified cache for qemu-user > and similiar scenarios. Some comments below. > --- > elf/cache.c | 11 ++++++++++ > elf/dl-cache.c | 20 +++++++++++++++++- > sysdeps/generic/dl-cache.h | 43 +++++++++++++++++++++++++++++++++++++- > 3 files changed, 72 insertions(+), 2 deletions(-) > > diff --git a/elf/cache.c b/elf/cache.c > index 1eb1455883..e0aa616352 100644 > --- a/elf/cache.c > +++ b/elf/cache.c > @@ -152,6 +152,14 @@ print_entry (const char *lib, int flag, unsigned int osversion, > printf (") => %s\n", key); > } > > +/* Print an error and exit if the new-file cache is internally > + inconsistent. */ > +static void > +check_new_cache (struct cache_file_new *cache) > +{ > + if (! cache_file_new_matches_endian (cache)) > + error (EXIT_FAILURE, 0, _("Cache file has wrong endianness.\n")); > +} > > /* Print the whole cache file, if a file contains the new cache format > hidden in the old one, print the contents of the new format. */ Ok. > @@ -193,6 +201,7 @@ print_cache (const char *cache_name) > || memcmp (cache_new->version, CACHE_VERSION, > sizeof CACHE_VERSION - 1)) > error (EXIT_FAILURE, 0, _("File is not a cache file.\n")); > + check_new_cache (cache_new); > format = 1; > /* This is where the strings start. */ > cache_data = (const char *) cache_new; Ok. > @@ -222,6 +231,7 @@ print_cache (const char *cache_name) > && memcmp (cache_new->version, CACHE_VERSION, > sizeof CACHE_VERSION - 1) == 0) > { > + check_new_cache (cache_new); > cache_data = (const char *) cache_new; > format = 1; > } Ok. > @@ -361,6 +371,7 @@ save_cache (const char *cache_name) > > file_entries_new->nlibs = cache_entry_count; > file_entries_new->len_strings = total_strlen; > + file_entries_new->flags = cache_file_new_flags_endian; > } > > /* Pad for alignment of cache_file_new. */ Ok. > diff --git a/elf/dl-cache.c b/elf/dl-cache.c > index 93d185e788..3aa8ed6c13 100644 > --- a/elf/dl-cache.c > +++ b/elf/dl-cache.c > @@ -210,6 +210,11 @@ _dl_load_cache_lookup (const char *name) > && ((cachesize - sizeof *cache_new) / sizeof (struct file_entry_new) > >= ((struct cache_file_new *) file)->nlibs)) > { > + if (! cache_file_new_matches_endian (file)) > + { > + __munmap (file, cachesize); > + file = (void *) -1; > + } > cache_new = file; > cache = file; > } Ok. > @@ -231,7 +236,20 @@ _dl_load_cache_lookup (const char *name) > if (cachesize < (offset + sizeof (struct cache_file_new)) > || memcmp (cache_new->magic, CACHEMAGIC_VERSION_NEW, > sizeof CACHEMAGIC_VERSION_NEW - 1) != 0) > - cache_new = (void *) -1; > + cache_new = (void *) -1; > + else > + { > + if (! cache_file_new_matches_endian (cache_new)) > + { > + /* The old-format part of the cache is bogus as well > + if the endianness does not match. (But it is > + unclear how the new header can be located if the > + endianess does not match.) */ > + cache = (void *) -1; > + cache_new = (void *) -1; > + __munmap (file, cachesize); > + } > + } > } > else > { Ok. > diff --git a/sysdeps/generic/dl-cache.h b/sysdeps/generic/dl-cache.h > index 6b310e9e15..1b04211f6b 100644 > --- a/sysdeps/generic/dl-cache.h > +++ b/sysdeps/generic/dl-cache.h > @@ -16,6 +16,11 @@ > License along with the GNU C Library; if not, see > <https://www.gnu.org/licenses/>. */ > > +#ifndef _DL_CACHE_H > +#define _DL_CACHE_H > + > +#include <endian.h> > +#include <stdbool.h> > #include <stdint.h> > > #ifndef _DL_CACHE_DEFAULT_ID Ok. > @@ -83,21 +88,57 @@ struct file_entry_new > uint64_t hwcap; /* Hwcap entry. */ > }; > > +/* See flags member of struct cache_file_new below. */ > +enum > + { > + cache_file_new_flags_endian = (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ > + ? 2 : 3) Why not enumerate the possible values as described below: enum cache_file_new_flags { cache_file_flag_invalid = 1, cache_file_flag_endianess = 2 }; enum { cache_file_new_flags_endian = (__BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ ? cache_file_flag_endianess : 0) | cache_file_flag_invalid }; I think this is slight less confusing than using direct values and more explicit on how the default flags value should be. > + }; > + > struct cache_file_new > { > char magic[sizeof CACHEMAGIC_NEW - 1]; > char version[sizeof CACHE_VERSION - 1]; > uint32_t nlibs; /* Number of entries. */ > uint32_t len_strings; /* Size of string table. */ > - uint32_t unused[5]; /* Leave space for future extensions > + > + /* flags & 3 is used to indicate the endianness of the cache. > + 0: no endianness information available > + (An old ldconfig version without endianness support wrote the file.) > + 1: cache is invalid > + 2: little endian > + 3: big endian > + > + The remaining bits are unused and should be generated as zero and > + ignored by readers. */ > + uint8_t flags; > + > + uint8_t padding_unsed[3]; /* Not used, for future extensions. */ > + > + uint32_t unused[4]; /* Leave space for future extensions > and align to 8 byte boundary. */ > struct file_entry_new libs[0]; /* Entries describing libraries. */ > /* After this the string table of size len_strings is found. */ > }; > > +/* Returns false if *CACHE has the wrong endianness for this > + architecture, and true if the endianness matches (or is > + unknown). */ > +static inline bool > +cache_file_new_matches_endian (const struct cache_file_new *cache) > +{ > + /* A zero value for cache->flags means that no endianness > + information is available. */ > + return cache->flags == 0 > + || (cache->flags & 3) == cache_file_new_flags_endian; > +} > + > + Using the suggestion above: static inline bool cache_file_new_matches_endian (const struct cache_file_new *cache) { return cache->flags == 0 || (cache->flags & (cache_file_flag_endianess | cache_file_flag_invalid)) == cache_file_flag_endianess; } > /* Used to align cache_file_new. */ > #define ALIGN_CACHE(addr) \ > (((addr) + __alignof__ (struct cache_file_new) -1) \ > & (~(__alignof__ (struct cache_file_new) - 1))) > > extern int _dl_cache_libcmp (const char *p1, const char *p2) attribute_hidden; > + > +#endif /* _DL_CACHE_H */ > Ok.
diff --git a/elf/cache.c b/elf/cache.c index 1eb1455883..e0aa616352 100644 --- a/elf/cache.c +++ b/elf/cache.c @@ -152,6 +152,14 @@ print_entry (const char *lib, int flag, unsigned int osversion, printf (") => %s\n", key); } +/* Print an error and exit if the new-file cache is internally + inconsistent. */ +static void +check_new_cache (struct cache_file_new *cache) +{ + if (! cache_file_new_matches_endian (cache)) + error (EXIT_FAILURE, 0, _("Cache file has wrong endianness.\n")); +} /* Print the whole cache file, if a file contains the new cache format hidden in the old one, print the contents of the new format. */ @@ -193,6 +201,7 @@ print_cache (const char *cache_name) || memcmp (cache_new->version, CACHE_VERSION, sizeof CACHE_VERSION - 1)) error (EXIT_FAILURE, 0, _("File is not a cache file.\n")); + check_new_cache (cache_new); format = 1; /* This is where the strings start. */ cache_data = (const char *) cache_new; @@ -222,6 +231,7 @@ print_cache (const char *cache_name) && memcmp (cache_new->version, CACHE_VERSION, sizeof CACHE_VERSION - 1) == 0) { + check_new_cache (cache_new); cache_data = (const char *) cache_new; format = 1; } @@ -361,6 +371,7 @@ save_cache (const char *cache_name) file_entries_new->nlibs = cache_entry_count; file_entries_new->len_strings = total_strlen; + file_entries_new->flags = cache_file_new_flags_endian; } /* Pad for alignment of cache_file_new. */ diff --git a/elf/dl-cache.c b/elf/dl-cache.c index 93d185e788..3aa8ed6c13 100644 --- a/elf/dl-cache.c +++ b/elf/dl-cache.c @@ -210,6 +210,11 @@ _dl_load_cache_lookup (const char *name) && ((cachesize - sizeof *cache_new) / sizeof (struct file_entry_new) >= ((struct cache_file_new *) file)->nlibs)) { + if (! cache_file_new_matches_endian (file)) + { + __munmap (file, cachesize); + file = (void *) -1; + } cache_new = file; cache = file; } @@ -231,7 +236,20 @@ _dl_load_cache_lookup (const char *name) if (cachesize < (offset + sizeof (struct cache_file_new)) || memcmp (cache_new->magic, CACHEMAGIC_VERSION_NEW, sizeof CACHEMAGIC_VERSION_NEW - 1) != 0) - cache_new = (void *) -1; + cache_new = (void *) -1; + else + { + if (! cache_file_new_matches_endian (cache_new)) + { + /* The old-format part of the cache is bogus as well + if the endianness does not match. (But it is + unclear how the new header can be located if the + endianess does not match.) */ + cache = (void *) -1; + cache_new = (void *) -1; + __munmap (file, cachesize); + } + } } else { diff --git a/sysdeps/generic/dl-cache.h b/sysdeps/generic/dl-cache.h index 6b310e9e15..1b04211f6b 100644 --- a/sysdeps/generic/dl-cache.h +++ b/sysdeps/generic/dl-cache.h @@ -16,6 +16,11 @@ License along with the GNU C Library; if not, see <https://www.gnu.org/licenses/>. */ +#ifndef _DL_CACHE_H +#define _DL_CACHE_H + +#include <endian.h> +#include <stdbool.h> #include <stdint.h> #ifndef _DL_CACHE_DEFAULT_ID @@ -83,21 +88,57 @@ struct file_entry_new uint64_t hwcap; /* Hwcap entry. */ }; +/* See flags member of struct cache_file_new below. */ +enum + { + cache_file_new_flags_endian = (__BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ + ? 2 : 3) + }; + struct cache_file_new { char magic[sizeof CACHEMAGIC_NEW - 1]; char version[sizeof CACHE_VERSION - 1]; uint32_t nlibs; /* Number of entries. */ uint32_t len_strings; /* Size of string table. */ - uint32_t unused[5]; /* Leave space for future extensions + + /* flags & 3 is used to indicate the endianness of the cache. + 0: no endianness information available + (An old ldconfig version without endianness support wrote the file.) + 1: cache is invalid + 2: little endian + 3: big endian + + The remaining bits are unused and should be generated as zero and + ignored by readers. */ + uint8_t flags; + + uint8_t padding_unsed[3]; /* Not used, for future extensions. */ + + uint32_t unused[4]; /* Leave space for future extensions and align to 8 byte boundary. */ struct file_entry_new libs[0]; /* Entries describing libraries. */ /* After this the string table of size len_strings is found. */ }; +/* Returns false if *CACHE has the wrong endianness for this + architecture, and true if the endianness matches (or is + unknown). */ +static inline bool +cache_file_new_matches_endian (const struct cache_file_new *cache) +{ + /* A zero value for cache->flags means that no endianness + information is available. */ + return cache->flags == 0 + || (cache->flags & 3) == cache_file_new_flags_endian; +} + + /* Used to align cache_file_new. */ #define ALIGN_CACHE(addr) \ (((addr) + __alignof__ (struct cache_file_new) -1) \ & (~(__alignof__ (struct cache_file_new) - 1))) extern int _dl_cache_libcmp (const char *p1, const char *p2) attribute_hidden; + +#endif /* _DL_CACHE_H */