diff mbox series

ld.so: Check for new cache format first and enhance corruption check

Message ID 87ftbwggqn.fsf@oldenburg2.str.redhat.com
State Superseded
Headers
Series ld.so: Check for new cache format first and enhance corruption check |

Commit Message

Florian Weimer May 19, 2020, 2:08 p.m. UTC 
  Now that ldconfig defaults to the new format (only), check for it
first.  Also apply the corruption check added in commit 2954daf00bb4d
("Add more checks for valid ld.so.cache file (bug 18093)") to the
new-format-only case.

Suggested-by: Josh Triplett <josh@joshtriplett.org>

---
Tested on i686-linux-gnu.  I double-checked manually that the cache is
still used after this change.

 elf/dl-cache.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

Comments

Andreas Schwab May 19, 2020, 2:45 p.m. UTC | #1 
On Mai 19 2020, Florian Weimer via Libc-alpha wrote:

> Now that ldconfig defaults to the new format (only), check for it
> first.  Also apply the corruption check added in commit 2954daf00bb4d
> ("Add more checks for valid ld.so.cache file (bug 18093)") to the
> new-format-only case.

Ok.

> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
> index 3eedd9afcf..14691d3d2b 100644
> --- a/elf/dl-cache.c
> +++ b/elf/dl-cache.c
> @@ -199,11 +199,21 @@ _dl_load_cache_lookup (const char *name)
>  					       PROT_READ);
>  
>        /* We can handle three different cache file formats here:
> +	 - only the new format
>  	 - the old libc5/glibc2.0/2.1 format
>  	 - the old format with the new format in it
> -	 - only the new format
>  	 The following checks if the cache contains any of these formats.  */
> -      if (file != MAP_FAILED && cachesize > sizeof *cache
> +      if (file != MAP_FAILED && cachesize > sizeof *cache_new
> +	       && memcmp (file, CACHEMAGIC_VERSION_NEW,
> +			  sizeof CACHEMAGIC_VERSION_NEW - 1) == 0

Wrong indent.

Andreas.
Florian Weimer May 19, 2020, 2:51 p.m. UTC | #2 
* Andreas Schwab:

> On Mai 19 2020, Florian Weimer via Libc-alpha wrote:
>
>> Now that ldconfig defaults to the new format (only), check for it
>> first.  Also apply the corruption check added in commit 2954daf00bb4d
>> ("Add more checks for valid ld.so.cache file (bug 18093)") to the
>> new-format-only case.
>
> Ok.

So the patch is okay to push?  Thanks.

>> diff --git a/elf/dl-cache.c b/elf/dl-cache.c
>> index 3eedd9afcf..14691d3d2b 100644
>> --- a/elf/dl-cache.c
>> +++ b/elf/dl-cache.c
>> @@ -199,11 +199,21 @@ _dl_load_cache_lookup (const char *name)
>>  					       PROT_READ);
>>  
>>        /* We can handle three different cache file formats here:
>> +	 - only the new format
>>  	 - the old libc5/glibc2.0/2.1 format
>>  	 - the old format with the new format in it
>> -	 - only the new format
>>  	 The following checks if the cache contains any of these formats.  */
>> -      if (file != MAP_FAILED && cachesize > sizeof *cache
>> +      if (file != MAP_FAILED && cachesize > sizeof *cache_new
>> +	       && memcmp (file, CACHEMAGIC_VERSION_NEW,
>> +			  sizeof CACHEMAGIC_VERSION_NEW - 1) == 0
>
> Wrong indent.

Oh, sorry. Like this?

      if (file != MAP_FAILED && cachesize > sizeof *cache_new
	  && memcmp (file, CACHEMAGIC_VERSION_NEW,
		     sizeof CACHEMAGIC_VERSION_NEW - 1) == 0
	  /* Check for corruption, avoiding overflow.  */
	  && ((cachesize - sizeof *cache_new) / sizeof (struct file_entry_new)
	      >= ((struct cache_file_new *) file)->nlibs))

Thanks,
Florian
diff mbox series

Patch

diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index 3eedd9afcf..14691d3d2b 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -199,11 +199,21 @@  _dl_load_cache_lookup (const char *name)
 					       PROT_READ);
 
       /* We can handle three different cache file formats here:
+	 - only the new format
 	 - the old libc5/glibc2.0/2.1 format
 	 - the old format with the new format in it
-	 - only the new format
 	 The following checks if the cache contains any of these formats.  */
-      if (file != MAP_FAILED && cachesize > sizeof *cache
+      if (file != MAP_FAILED && cachesize > sizeof *cache_new
+	       && memcmp (file, CACHEMAGIC_VERSION_NEW,
+			  sizeof CACHEMAGIC_VERSION_NEW - 1) == 0
+	  /* Check for corruption, avoiding overflow.  */
+	  && ((cachesize - sizeof *cache_new) / sizeof (struct file_entry_new)
+	      >= ((struct cache_file_new *) file)->nlibs))
+	{
+	  cache_new = file;
+	  cache = file;
+	}
+      else if (file != MAP_FAILED && cachesize > sizeof *cache
 	  && memcmp (file, CACHEMAGIC, sizeof CACHEMAGIC - 1) == 0
 	  /* Check for corruption, avoiding overflow.  */
 	  && ((cachesize - sizeof *cache) / sizeof (struct file_entry)
@@ -223,13 +233,6 @@  _dl_load_cache_lookup (const char *name)
 			 sizeof CACHEMAGIC_VERSION_NEW - 1) != 0)
 	    cache_new = (void *) -1;
 	}
-      else if (file != MAP_FAILED && cachesize > sizeof *cache_new
-	       && memcmp (file, CACHEMAGIC_VERSION_NEW,
-			  sizeof CACHEMAGIC_VERSION_NEW - 1) == 0)
-	{
-	  cache_new = file;
-	  cache = file;
-	}
       else
 	{
 	  if (file != MAP_FAILED)