diff mbox series

[1/8] elf: Bound the tunable cache string table against the mapping size

Message ID 20260706211524.3801466-2-adhemerval.zanella@linaro.org (mailing list archive)
State Committed
Commit 3d3cd10c4ec8bf5fb48faaad8c0acea3da225a35
Headers
Series Fixes for the system-wide tunables support |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent

Commit Message

Adhemerval Zanella Netto July 6, 2026, 9:14 p.m. UTC 
  _dl_load_cache_tunables bounds each entry's string offsets against
[s_start, start + cache_new->len_strings], but len_strings is an
unvalidated 32-bit field from ld.so.cache and s_start/s_end were int.  A
corrupt cache with an oversized len_strings could make s_end exceed the
mapping (or overflow), letting an offset point outside the mmap; the
following strcmp/__strdup would then read unmapped memory.

Compute the offsets as size_t and clamp s_end to cachesize, matching how
the regular library lookup bounds string offsets against the mapping size.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 elf/dl-cache.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

Comments

DJ Delorie July 9, 2026, 2:02 a.m. UTC | #1 
Adhemerval Zanella <adhemerval.zanella@linaro.org> writes:
> -  /* Validate each entry.  */
> -  int s_start = (const char *) (&cache_new->libs[cache_new->nlibs]) - *data;
> -  int s_end = s_start + cache_new->len_strings;
> +  /* Validate each entry.  The string table lies between the file entries
> +     and the end of the mapping; clamp its end to CACHESIZE so that a bogus
> +     len_strings cannot make an offset point outside the mapped file.  */
> +  size_t s_start = (const char *) (&cache_new->libs[cache_new->nlibs]) - *data;

s_start is always set

> +  size_t s_end;
> +  if (s_start >= cachesize
> +      || INT_ADD_WRAPV (s_start, cache_new->len_strings, &s_end))
> +    return NULL;

s_end might be set here, but we return if not

> +  if (s_end > cachesize)
> +    s_end = cachesize;

s_end is definitely set here.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>
diff mbox series

Patch

diff --git a/elf/dl-cache.c b/elf/dl-cache.c
index d05afe27004..5d8d3cae908 100644
--- a/elf/dl-cache.c
+++ b/elf/dl-cache.c
@@ -17,6 +17,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <assert.h>
+#include <intprops.h>
 #include <unistd.h>
 #include <ldsodefs.h>
 #include <sys/mman.h>
@@ -649,9 +650,16 @@  _dl_load_cache_tunables (const char **data)
       != (void *) & tec[count])
     return NULL;
 
-  /* Validate each entry.  */
-  int s_start = (const char *) (&cache_new->libs[cache_new->nlibs]) - *data;
-  int s_end = s_start + cache_new->len_strings;
+  /* Validate each entry.  The string table lies between the file entries
+     and the end of the mapping; clamp its end to CACHESIZE so that a bogus
+     len_strings cannot make an offset point outside the mapped file.  */
+  size_t s_start = (const char *) (&cache_new->libs[cache_new->nlibs]) - *data;
+  size_t s_end;
+  if (s_start >= cachesize
+      || INT_ADD_WRAPV (s_start, cache_new->len_strings, &s_end))
+    return NULL;
+  if (s_end > cachesize)
+    s_end = cachesize;
   for (i = 0; i < count; i ++)
     {
       if (thc->tunables[i].name_offset < s_start