* [PATCH] bfd: Always check compressed sections with the corrupt size
@ 2021-11-04 13:38 H.J. Lu
2021-11-04 22:16 ` Alan Modra
2021-11-05 2:40 ` Hans-Peter Nilsson
0 siblings, 2 replies; 3+ messages in thread
From: H.J. Lu @ 2021-11-04 13:38 UTC (permalink / raw)
To: binutils
Always check compressed sections with the corrupt size for non-MMO
files. Skip MMO files for compress_status == COMPRESS_SECTION_NONE
since MMO has special handling for COMPRESS_SECTION_NONE.
I am checking in this.
H.J.
---
PR binutils/28530
* compress.c (bfd_get_full_section_contents): Always check
compressed sections with the corrupt size.
---
bfd/compress.c | 50 +++++++++++++++++++++++++++-----------------------
1 file changed, 27 insertions(+), 23 deletions(-)
diff --git a/bfd/compress.c b/bfd/compress.c
index 4a2ada3e3eb..a3adb8d8250 100644
--- a/bfd/compress.c
+++ b/bfd/compress.c
@@ -232,6 +232,7 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
bfd_size_type save_rawsize;
bfd_byte *compressed_buffer;
unsigned int compression_header_size;
+ ufile_ptr filesize;
if (abfd->direction != write_direction && sec->rawsize != 0)
sz = sec->rawsize;
@@ -243,34 +244,37 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
return true;
}
+ filesize = bfd_get_file_size (abfd);
+ if (filesize > 0
+ && filesize < sz
+ /* PR 24753: Linker created sections can be larger than
+ the file size, eg if they are being used to hold stubs. */
+ && (bfd_section_flags (sec) & SEC_LINKER_CREATED) == 0
+ /* PR 24753: Sections which have no content should also be
+ excluded as they contain no size on disk. */
+ && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) != 0
+ /* PR 28530: Check compressed sections with the corrupt size. */
+ && (sec->compress_status != COMPRESS_SECTION_NONE
+ /* The MMO file format supports its own special compression
+ technique, but it uses COMPRESS_SECTION_NONE when loading
+ a section's contents. */
+ || bfd_get_flavour (abfd) != bfd_target_mmo_flavour))
+ {
+ /* PR 24708: Avoid attempts to allocate a ridiculous amount
+ of memory. */
+ bfd_set_error (bfd_error_file_truncated);
+ _bfd_error_handler
+ /* xgettext:c-format */
+ (_("error: %pB(%pA) section size (%#" PRIx64 " bytes) is larger than file size (%#" PRIx64 " bytes)"),
+ abfd, sec, (uint64_t) sz, (uint64_t) filesize);
+ return false;
+ }
+
switch (sec->compress_status)
{
case COMPRESS_SECTION_NONE:
if (p == NULL)
{
- ufile_ptr filesize = bfd_get_file_size (abfd);
- if (filesize > 0
- && filesize < sz
- /* PR 24753: Linker created sections can be larger than
- the file size, eg if they are being used to hold stubs. */
- && (bfd_section_flags (sec) & SEC_LINKER_CREATED) == 0
- /* PR 24753: Sections which have no content should also be
- excluded as they contain no size on disk. */
- && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) != 0
- /* The MMO file format supports its own special compression
- technique, but it uses COMPRESS_SECTION_NONE when loading
- a section's contents. */
- && bfd_get_flavour (abfd) != bfd_target_mmo_flavour)
- {
- /* PR 24708: Avoid attempts to allocate a ridiculous amount
- of memory. */
- bfd_set_error (bfd_error_file_truncated);
- _bfd_error_handler
- /* xgettext:c-format */
- (_("error: %pB(%pA) section size (%#" PRIx64 " bytes) is larger than file size (%#" PRIx64 " bytes)"),
- abfd, sec, (uint64_t) sz, (uint64_t) filesize);
- return false;
- }
p = (bfd_byte *) bfd_malloc (sz);
if (p == NULL)
{
--
2.33.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bfd: Always check compressed sections with the corrupt size
2021-11-04 13:38 [PATCH] bfd: Always check compressed sections with the corrupt size H.J. Lu
@ 2021-11-04 22:16 ` Alan Modra
2021-11-05 2:40 ` Hans-Peter Nilsson
1 sibling, 0 replies; 3+ messages in thread
From: Alan Modra @ 2021-11-04 22:16 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils, Nick Clifton
On Thu, Nov 04, 2021 at 06:38:26AM -0700, H.J. Lu wrote:
> Always check compressed sections with the corrupt size for non-MMO
> files. Skip MMO files for compress_status == COMPRESS_SECTION_NONE
> since MMO has special handling for COMPRESS_SECTION_NONE.
>
> I am checking in this.
This doesn't look correct to me. When decompressing you can read a
section that is larger than the file size.
--
Alan Modra
Australia Development Lab, IBM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] bfd: Always check compressed sections with the corrupt size
2021-11-04 13:38 [PATCH] bfd: Always check compressed sections with the corrupt size H.J. Lu
2021-11-04 22:16 ` Alan Modra
@ 2021-11-05 2:40 ` Hans-Peter Nilsson
1 sibling, 0 replies; 3+ messages in thread
From: Hans-Peter Nilsson @ 2021-11-05 2:40 UTC (permalink / raw)
To: H.J. Lu; +Cc: binutils
On Thu, 4 Nov 2021, H.J. Lu via Binutils wrote:
> Always check compressed sections with the corrupt size for non-MMO
> files. Skip MMO files for compress_status == COMPRESS_SECTION_NONE
> since MMO has special handling for COMPRESS_SECTION_NONE.
>
> I am checking in this.
Did I miss the approval?
> H.J.
> ---
> PR binutils/28530
> * compress.c (bfd_get_full_section_contents): Always check
> compressed sections with the corrupt size.
> ---
> bfd/compress.c | 50 +++++++++++++++++++++++++++-----------------------
> 1 file changed, 27 insertions(+), 23 deletions(-)
>
> diff --git a/bfd/compress.c b/bfd/compress.c
> index 4a2ada3e3eb..a3adb8d8250 100644
> --- a/bfd/compress.c
> +++ b/bfd/compress.c
> @@ -232,6 +232,7 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
> bfd_size_type save_rawsize;
> bfd_byte *compressed_buffer;
> unsigned int compression_header_size;
> + ufile_ptr filesize;
>
> if (abfd->direction != write_direction && sec->rawsize != 0)
> sz = sec->rawsize;
> @@ -243,34 +244,37 @@ bfd_get_full_section_contents (bfd *abfd, sec_ptr sec, bfd_byte **ptr)
> return true;
> }
>
> + filesize = bfd_get_file_size (abfd);
> + if (filesize > 0
> + && filesize < sz
> + /* PR 24753: Linker created sections can be larger than
> + the file size, eg if they are being used to hold stubs. */
> + && (bfd_section_flags (sec) & SEC_LINKER_CREATED) == 0
> + /* PR 24753: Sections which have no content should also be
> + excluded as they contain no size on disk. */
> + && (bfd_section_flags (sec) & SEC_HAS_CONTENTS) != 0
> + /* PR 28530: Check compressed sections with the corrupt size. */
> + && (sec->compress_status != COMPRESS_SECTION_NONE
> + /* The MMO file format supports its own special compression
> + technique, but it uses COMPRESS_SECTION_NONE when loading
> + a section's contents. */
> + || bfd_get_flavour (abfd) != bfd_target_mmo_flavour))
Looking at the BFD flavor seems like the wrong thing to me.
Perhaps add a (defaulted) BFD field or hook if needed?
Also, ELF section contents in the mmo format is embedded, not
compressed. "Coded" if you will, compared to the file contents;
not a straight blob of bytes, rather escaped sequences. Also,
for future reference, it's "mmo", not "MMO".
brgds, H-P
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-11-05 2:40 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-04 13:38 [PATCH] bfd: Always check compressed sections with the corrupt size H.J. Lu
2021-11-04 22:16 ` Alan Modra
2021-11-05 2:40 ` Hans-Peter Nilsson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).