diff mbox series

[1/1] bfd: fix potential missing seek

Message ID 20251129114527.973538-1-code@rkapl.cz
State New
Headers
Series [1/1] bfd: fix potential missing seek |

Commit Message

Roman Kapl Nov. 29, 2025, 11:45 a.m. UTC 
  If a file was closed by cache and then bfd_open_file was called followed
by e.g. bfd_seek to the original position, the seek would be optimized out
while the real file position was still zero (as fopened).  I added
`bfd_io_force` to force the seek at next IO occasion.

This could led e.g. to appearence of a corrupted object in ld:
  symbol number 0 uses unsupported binding of 6
  or invalid string offset #X >= #Y for section

To reproduce in a real scenario, both large number of input archives and
certain object layout is needed, so it seemed to be rare. To reproduce
this better, 1) make sure to hit the cache limit (e.g lower it in code)
2) use some plugin (they do bfd_open_file) 3) from main pull in archives
that have symbols like this:
     5: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    1 C<int>::C()
     6: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    2 C<float>::C()
     7: 0000000000000000    39 FUNC    GLOBAL DEFAULT    3 export1()

The last read before cache close will be #6, after re-open it will try
to read export symbols and fail.

Signed-off-by: Roman Kapl <code@rkapl.cz>
---
 bfd/cache.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Alan Modra Nov. 30, 2025, 3:28 a.m. UTC | #1 
On Sat, Nov 29, 2025 at 12:45:27PM +0100, Roman Kapl wrote:
> If a file was closed by cache and then bfd_open_file was called followed
> by e.g. bfd_seek to the original position, the seek would be optimized out
> while the real file position was still zero (as fopened).  I added
> `bfd_io_force` to force the seek at next IO occasion.
> 
> This could led e.g. to appearence of a corrupted object in ld:
>   symbol number 0 uses unsupported binding of 6
>   or invalid string offset #X >= #Y for section
> 
> To reproduce in a real scenario, both large number of input archives and
> certain object layout is needed, so it seemed to be rare. To reproduce
> this better, 1) make sure to hit the cache limit (e.g lower it in code)
> 2) use some plugin (they do bfd_open_file) 3) from main pull in archives
> that have symbols like this:
>      5: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    1 C<int>::C()
>      6: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT    2 C<float>::C()
>      7: 0000000000000000    39 FUNC    GLOBAL DEFAULT    3 export1()
> 
> The last read before cache close will be #6, after re-open it will try
> to read export symbols and fail.
> 
> Signed-off-by: Roman Kapl <code@rkapl.cz>

Thanks!  Applied.

> ---
>  bfd/cache.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/bfd/cache.c b/bfd/cache.c
> index 3bf9c944..3342e2d5 100644
> --- a/bfd/cache.c
> +++ b/bfd/cache.c
> @@ -177,6 +177,7 @@ bfd_cache_delete (bfd *abfd)
>    BFD_ASSERT (open_files > 0);
>    --open_files;
>    abfd->flags |= BFD_CLOSED_BY_CACHE;
> +  abfd->last_io = bfd_io_force;
>  
>    return ret;
>  }
> -- 
> 2.52.0
diff mbox series

Patch

diff --git a/bfd/cache.c b/bfd/cache.c
index 3bf9c944..3342e2d5 100644
--- a/bfd/cache.c
+++ b/bfd/cache.c
@@ -177,6 +177,7 @@  bfd_cache_delete (bfd *abfd)
   BFD_ASSERT (open_files > 0);
   --open_files;
   abfd->flags |= BFD_CLOSED_BY_CACHE;
+  abfd->last_io = bfd_io_force;
 
   return ret;
 }