diff mbox series

[v10,6/8] linux-user/elfload: Move PT_INTERP detection to first loop

Message ID 20201002215955.254866-7-richard.henderson@linaro.org
State New
Headers show
Series linux-user: User support for AArch64 BTI | expand

Commit Message

Richard Henderson Oct. 2, 2020, 9:59 p.m. UTC
For BTI, we need to know if the executable is static or dynamic,
which means looking for PT_INTERP earlier.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 60 +++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 29 deletions(-)

Comments

Philippe Mathieu-Daudé Oct. 3, 2020, 5:38 p.m. UTC | #1
On 10/2/20 11:59 PM, Richard Henderson wrote:
> For BTI, we need to know if the executable is static or dynamic,
> which means looking for PT_INTERP earlier.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/elfload.c | 60 +++++++++++++++++++++++---------------------
>  1 file changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 735ebfa190..6b422990ff 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2421,8 +2421,10 @@ static void load_elf_image(const char *image_name, int image_fd,
>  
>      mmap_lock();
>  
> -    /* Find the maximum size of the image and allocate an appropriate
> -       amount of memory to handle that.  */
> +    /*
> +     * Find the maximum size of the image and allocate an appropriate
> +     * amount of memory to handle that.  Locate the interpreter, if any.
> +     */
>      loaddr = -1, hiaddr = 0;
>      info->alignment = 0;
>      for (i = 0; i < ehdr->e_phnum; ++i) {
> @@ -2438,6 +2440,33 @@ static void load_elf_image(const char *image_name, int image_fd,
>              }
>              ++info->nsegs;
>              info->alignment |= eppnt->p_align;
> +        } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
> +            char *interp_name;
> +
> +            if (*pinterp_name) {
> +                errmsg = "Multiple PT_INTERP entries";
> +                goto exit_errmsg;
> +            }
> +            interp_name = malloc(eppnt->p_filesz);
> +            if (!interp_name) {
> +                goto exit_perror;
> +            }
> +
> +            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
> +                memcpy(interp_name, bprm_buf + eppnt->p_offset,
> +                       eppnt->p_filesz);
> +            } else {
> +                retval = pread(image_fd, interp_name, eppnt->p_filesz,
> +                               eppnt->p_offset);
> +                if (retval != eppnt->p_filesz) {

Preexisting, free(interp_name)?

> +                    goto exit_perror;
> +                }
> +            }
> +            if (interp_name[eppnt->p_filesz - 1] != 0) {
> +                errmsg = "Invalid PT_INTERP entry";

Ditto, otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +                goto exit_errmsg;
> +            }
> +            *pinterp_name = interp_name;
>          }
>      }
>  
> @@ -2590,33 +2619,6 @@ static void load_elf_image(const char *image_name, int image_fd,
>              if (vaddr_em > info->brk) {
>                  info->brk = vaddr_em;
>              }
> -        } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
> -            char *interp_name;
> -
> -            if (*pinterp_name) {
> -                errmsg = "Multiple PT_INTERP entries";
> -                goto exit_errmsg;
> -            }
> -            interp_name = malloc(eppnt->p_filesz);
> -            if (!interp_name) {
> -                goto exit_perror;
> -            }
> -
> -            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
> -                memcpy(interp_name, bprm_buf + eppnt->p_offset,
> -                       eppnt->p_filesz);
> -            } else {
> -                retval = pread(image_fd, interp_name, eppnt->p_filesz,
> -                               eppnt->p_offset);
> -                if (retval != eppnt->p_filesz) {
> -                    goto exit_perror;
> -                }
> -            }
> -            if (interp_name[eppnt->p_filesz - 1] != 0) {
> -                errmsg = "Invalid PT_INTERP entry";
> -                goto exit_errmsg;
> -            }
> -            *pinterp_name = interp_name;
>  #ifdef TARGET_MIPS
>          } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
>              Mips_elf_abiflags_v0 abiflags;
>
Philippe Mathieu-Daudé Oct. 3, 2020, 6:05 p.m. UTC | #2
On 10/3/20 7:38 PM, Philippe Mathieu-Daudé wrote:
> On 10/2/20 11:59 PM, Richard Henderson wrote:
>> For BTI, we need to know if the executable is static or dynamic,
>> which means looking for PT_INTERP earlier.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  linux-user/elfload.c | 60 +++++++++++++++++++++++---------------------
>>  1 file changed, 31 insertions(+), 29 deletions(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 735ebfa190..6b422990ff 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2421,8 +2421,10 @@ static void load_elf_image(const char *image_name, int image_fd,
>>  
>>      mmap_lock();
>>  
>> -    /* Find the maximum size of the image and allocate an appropriate
>> -       amount of memory to handle that.  */
>> +    /*
>> +     * Find the maximum size of the image and allocate an appropriate
>> +     * amount of memory to handle that.  Locate the interpreter, if any.
>> +     */
>>      loaddr = -1, hiaddr = 0;
>>      info->alignment = 0;
>>      for (i = 0; i < ehdr->e_phnum; ++i) {
>> @@ -2438,6 +2440,33 @@ static void load_elf_image(const char *image_name, int image_fd,
>>              }
>>              ++info->nsegs;
>>              info->alignment |= eppnt->p_align;
>> +        } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
>> +            char *interp_name;
>> +
>> +            if (*pinterp_name) {
>> +                errmsg = "Multiple PT_INTERP entries";
>> +                goto exit_errmsg;
>> +            }
>> +            interp_name = malloc(eppnt->p_filesz);
>> +            if (!interp_name) {
>> +                goto exit_perror;
>> +            }
>> +
>> +            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
>> +                memcpy(interp_name, bprm_buf + eppnt->p_offset,
>> +                       eppnt->p_filesz);
>> +            } else {
>> +                retval = pread(image_fd, interp_name, eppnt->p_filesz,
>> +                               eppnt->p_offset);
>> +                if (retval != eppnt->p_filesz) {
> 
> Preexisting, free(interp_name)?

I just sent a patch using g_steal_pointer() instead:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg00792.html
(Maybe I should have tagged it RFC as this is the first
time I try this API).

> 
>> +                    goto exit_perror;
>> +                }
>> +            }
>> +            if (interp_name[eppnt->p_filesz - 1] != 0) {
>> +                errmsg = "Invalid PT_INTERP entry";
> 
> Ditto, otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
>> +                goto exit_errmsg;
>> +            }
>> +            *pinterp_name = interp_name;
>>          }
>>      }
>>  
>> @@ -2590,33 +2619,6 @@ static void load_elf_image(const char *image_name, int image_fd,
>>              if (vaddr_em > info->brk) {
>>                  info->brk = vaddr_em;
>>              }
>> -        } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
>> -            char *interp_name;
>> -
>> -            if (*pinterp_name) {
>> -                errmsg = "Multiple PT_INTERP entries";
>> -                goto exit_errmsg;
>> -            }
>> -            interp_name = malloc(eppnt->p_filesz);
>> -            if (!interp_name) {
>> -                goto exit_perror;
>> -            }
>> -
>> -            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
>> -                memcpy(interp_name, bprm_buf + eppnt->p_offset,
>> -                       eppnt->p_filesz);
>> -            } else {
>> -                retval = pread(image_fd, interp_name, eppnt->p_filesz,
>> -                               eppnt->p_offset);
>> -                if (retval != eppnt->p_filesz) {
>> -                    goto exit_perror;
>> -                }
>> -            }
>> -            if (interp_name[eppnt->p_filesz - 1] != 0) {
>> -                errmsg = "Invalid PT_INTERP entry";
>> -                goto exit_errmsg;
>> -            }
>> -            *pinterp_name = interp_name;
>>  #ifdef TARGET_MIPS
>>          } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
>>              Mips_elf_abiflags_v0 abiflags;
>>
> 
>
diff mbox series

Patch

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 735ebfa190..6b422990ff 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2421,8 +2421,10 @@  static void load_elf_image(const char *image_name, int image_fd,
 
     mmap_lock();
 
-    /* Find the maximum size of the image and allocate an appropriate
-       amount of memory to handle that.  */
+    /*
+     * Find the maximum size of the image and allocate an appropriate
+     * amount of memory to handle that.  Locate the interpreter, if any.
+     */
     loaddr = -1, hiaddr = 0;
     info->alignment = 0;
     for (i = 0; i < ehdr->e_phnum; ++i) {
@@ -2438,6 +2440,33 @@  static void load_elf_image(const char *image_name, int image_fd,
             }
             ++info->nsegs;
             info->alignment |= eppnt->p_align;
+        } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
+            char *interp_name;
+
+            if (*pinterp_name) {
+                errmsg = "Multiple PT_INTERP entries";
+                goto exit_errmsg;
+            }
+            interp_name = malloc(eppnt->p_filesz);
+            if (!interp_name) {
+                goto exit_perror;
+            }
+
+            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
+                memcpy(interp_name, bprm_buf + eppnt->p_offset,
+                       eppnt->p_filesz);
+            } else {
+                retval = pread(image_fd, interp_name, eppnt->p_filesz,
+                               eppnt->p_offset);
+                if (retval != eppnt->p_filesz) {
+                    goto exit_perror;
+                }
+            }
+            if (interp_name[eppnt->p_filesz - 1] != 0) {
+                errmsg = "Invalid PT_INTERP entry";
+                goto exit_errmsg;
+            }
+            *pinterp_name = interp_name;
         }
     }
 
@@ -2590,33 +2619,6 @@  static void load_elf_image(const char *image_name, int image_fd,
             if (vaddr_em > info->brk) {
                 info->brk = vaddr_em;
             }
-        } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
-            char *interp_name;
-
-            if (*pinterp_name) {
-                errmsg = "Multiple PT_INTERP entries";
-                goto exit_errmsg;
-            }
-            interp_name = malloc(eppnt->p_filesz);
-            if (!interp_name) {
-                goto exit_perror;
-            }
-
-            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
-                memcpy(interp_name, bprm_buf + eppnt->p_offset,
-                       eppnt->p_filesz);
-            } else {
-                retval = pread(image_fd, interp_name, eppnt->p_filesz,
-                               eppnt->p_offset);
-                if (retval != eppnt->p_filesz) {
-                    goto exit_perror;
-                }
-            }
-            if (interp_name[eppnt->p_filesz - 1] != 0) {
-                errmsg = "Invalid PT_INTERP entry";
-                goto exit_errmsg;
-            }
-            *pinterp_name = interp_name;
 #ifdef TARGET_MIPS
         } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
             Mips_elf_abiflags_v0 abiflags;