[PATCH] ld: Pad the previous code segment with NOPs

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] ld: Pad the previous code segment with NOPs

H.J. Lu
This patch pads the previous code segment with NOPs, instead of zeros.

OK for master?

H.J.
---
        * elf.c (assign_file_positions_for_load_sections): Pad the
        previous code segment with NOPs.
---
 bfd/elf.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/bfd/elf.c b/bfd/elf.c
index 1d0eefd053..1bb91424f1 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -5319,6 +5319,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
     {
       asection **secpp;
       bfd_vma off_adjust;
+      bfd_vma off_prev_end;
       bfd_boolean no_contents;
 
       /* If elf_segment_map is not from map_sections_to_segments, the
@@ -5378,6 +5379,7 @@ assign_file_positions_for_load_sections (bfd *abfd,
 
       no_contents = FALSE;
       off_adjust = 0;
+      off_prev_end = off;
       if (p->p_type == PT_LOAD
   && m->count > 0)
  {
@@ -5429,6 +5431,28 @@ assign_file_positions_for_load_sections (bfd *abfd,
       && (off & -maxpagesize) == ((off + off_adjust) & -maxpagesize))
     off_adjust += maxpagesize;
   off += off_adjust;
+
+  if (off_adjust != 0
+      && j > 1
+      && p[-1].p_filesz != 0
+      && (p[-1].p_flags & PF_X) != 0)
+    {
+      /* Pad the previous code segment with NOPs.  */
+      char *fill = abfd->arch_info->fill (off_adjust,
+  bfd_big_endian (abfd),
+  TRUE);
+      if (fill == NULL
+  || bfd_seek (abfd, off_prev_end, SEEK_SET) != 0
+  || bfd_bwrite (fill, off_adjust, abfd) != off_adjust)
+ {
+  _bfd_error_handler
+    (_("%B: failed to pad code segment %d"), j - 1, abfd);
+  return FALSE;
+ }
+
+      free (fill);
+    }
+
   if (no_contents)
     {
       /* We shouldn't need to align the segment on disk since
--
2.14.3

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld: Pad the previous code segment with NOPs

Alan Modra-3
On Thu, Jan 11, 2018 at 07:25:14PM -0800, H.J. Lu wrote:

> @@ -5429,6 +5431,28 @@ assign_file_positions_for_load_sections (bfd *abfd,
>        && (off & -maxpagesize) == ((off + off_adjust) & -maxpagesize))
>      off_adjust += maxpagesize;
>    off += off_adjust;
> +
> +  if (off_adjust != 0
> +      && j > 1
> +      && p[-1].p_filesz != 0
> +      && (p[-1].p_flags & PF_X) != 0)
> +    {
> +      /* Pad the previous code segment with NOPs.  */
> +      char *fill = abfd->arch_info->fill (off_adjust,
> +  bfd_big_endian (abfd),
> +  TRUE);
> +      if (fill == NULL
> +  || bfd_seek (abfd, off_prev_end, SEEK_SET) != 0
> +  || bfd_bwrite (fill, off_adjust, abfd) != off_adjust)
> + {
> +  _bfd_error_handler
> +    (_("%B: failed to pad code segment %d"), j - 1, abfd);
> +  return FALSE;
> + }
> +
> +      free (fill);
> +    }
> +

Two things strike me as non-ideal here:

1) For targets that have arch_info->fill == bfd_arch_default_fill this
writes zeros to the output file, destroying a chance for the file to
be sparse.  ie. This patch may waste disk blocks.

2) The i386 fill function uses multi-byte nops.  Are any of those
patterns exploitable when executed at any offset from the start of the
insns?  Do other targets have the same potential problem?

--
Alan Modra
Australia Development Lab, IBM
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] ld: Pad the previous code segment with NOPs

H.J. Lu-30
On Fri, Jan 12, 2018 at 12:44 AM, Alan Modra <[hidden email]> wrote:

> On Thu, Jan 11, 2018 at 07:25:14PM -0800, H.J. Lu wrote:
>> @@ -5429,6 +5431,28 @@ assign_file_positions_for_load_sections (bfd *abfd,
>>             && (off & -maxpagesize) == ((off + off_adjust) & -maxpagesize))
>>           off_adjust += maxpagesize;
>>         off += off_adjust;
>> +
>> +       if (off_adjust != 0
>> +           && j > 1
>> +           && p[-1].p_filesz != 0
>> +           && (p[-1].p_flags & PF_X) != 0)
>> +         {
>> +           /* Pad the previous code segment with NOPs.  */
>> +           char *fill = abfd->arch_info->fill (off_adjust,
>> +                                               bfd_big_endian (abfd),
>> +                                               TRUE);
>> +           if (fill == NULL
>> +               || bfd_seek (abfd, off_prev_end, SEEK_SET) != 0
>> +               || bfd_bwrite (fill, off_adjust, abfd) != off_adjust)
>> +             {
>> +               _bfd_error_handler
>> +                 (_("%B: failed to pad code segment %d"), j - 1, abfd);
>> +               return FALSE;
>> +             }
>> +
>> +           free (fill);
>> +         }
>> +
>
> Two things strike me as non-ideal here:
>
> 1) For targets that have arch_info->fill == bfd_arch_default_fill this
> writes zeros to the output file, destroying a chance for the file to
> be sparse.  ie. This patch may waste disk blocks.
>
> 2) The i386 fill function uses multi-byte nops.  Are any of those
> patterns exploitable when executed at any offset from the start of the
> insns?  Do other targets have the same potential problem?

Good points.  Let leave zero padding here.

--
H.J.