PR32870: ld: arm32: fix segfault when linking with LLVMgold.so plugin under LTO

Message ID cec025e66b0c4deb9ed2d1d8f5a1d004@huawei.com
State Committed
Delegated to: Richard Earnshaw
Headers
Series PR32870: ld: arm32: fix segfault when linking with LLVMgold.so plugin under LTO |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_binutils_check--master-aarch64 success Test passed

Commit Message

dongjianqiang (A) April 15, 2025, 9:14 a.m. UTC
  Hi, 

Proposed patch to PR32870.
When handling the iplt in ARM32, the ld must check whether the input_bfd
is in ELF format. If the input is an LLVM Bitcode file, an error will be raised.

Any suggestions? Thanks.

---
	PR 32870
	* elf32-arm.c (elf32_arm_output_arch_local_syms): Check input_bfd
  

Comments

Richard Earnshaw (foss) June 3, 2025, 3:58 p.m. UTC | #1
On 15/04/2025 10:14, dongjianqiang (A) wrote:
> Hi,
> 
> Proposed patch to PR32870.
> When handling the iplt in ARM32, the ld must check whether the input_bfd
> is in ELF format. If the input is an LLVM Bitcode file, an error will be raised.
> 
> Any suggestions? Thanks.
> 
> ---
> 	PR 32870
> 	* elf32-arm.c (elf32_arm_output_arch_local_syms): Check input_bfd
> 
> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> index 3b7cee3de1c..3af964ce034 100644
> --- a/bfd/elf32-arm.c
> +++ b/bfd/elf32-arm.c
> @@ -18345,6 +18345,9 @@ elf32_arm_output_arch_local_syms (bfd *output_bfd,
>   	  struct arm_local_iplt_info **local_iplt;
>   	  unsigned int i, num_syms;
>   
> +	  if (!is_arm_elf (input_bfd))
> +	    continue;
> +
>   	  local_iplt = elf32_arm_local_iplt (input_bfd);
>   	  if (local_iplt != NULL)
>   	    {

I'd like Alan's opinion on this one.  To me it's strange that something 
that is not remotely like a object file is still in the list of input 
BFDs at this point.  This feels like it might be yet another rich source 
of fuzzing bugs.

If keeping this in the bfd list is correct, then I think a better fix 
would be to refuse to create a local iplt for this bfd; then we'll 
simply skip the following code because elf32_arm_local_iplt will simply 
return NULL.

R.
  
Alan Modra June 4, 2025, 12:34 a.m. UTC | #2
On Tue, Jun 03, 2025 at 04:58:36PM +0100, Richard Earnshaw wrote:
> On 15/04/2025 10:14, dongjianqiang (A) wrote:
> > Hi,
> > 
> > Proposed patch to PR32870.
> > When handling the iplt in ARM32, the ld must check whether the input_bfd
> > is in ELF format. If the input is an LLVM Bitcode file, an error will be raised.
> > 
> > Any suggestions? Thanks.
> > 
> > ---
> > 	PR 32870
> > 	* elf32-arm.c (elf32_arm_output_arch_local_syms): Check input_bfd
> > 
> > diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> > index 3b7cee3de1c..3af964ce034 100644
> > --- a/bfd/elf32-arm.c
> > +++ b/bfd/elf32-arm.c
> > @@ -18345,6 +18345,9 @@ elf32_arm_output_arch_local_syms (bfd *output_bfd,
> >   	  struct arm_local_iplt_info **local_iplt;
> >   	  unsigned int i, num_syms;
> > +	  if (!is_arm_elf (input_bfd))
> > +	    continue;
> > +
> >   	  local_iplt = elf32_arm_local_iplt (input_bfd);
> >   	  if (local_iplt != NULL)
> >   	    {
> 
> I'd like Alan's opinion on this one.  To me it's strange that something that
> is not remotely like a object file is still in the list of input BFDs at
> this point.  This feels like it might be yet another rich source of fuzzing
> bugs.

The patch is OK.

GNU ld can link foreign object files, if they have no relocations or
the relocs are simple enough and the target support for that object
properly describes them in reloc howtos.

> If keeping this in the bfd list is correct, then I think a better fix would
> be to refuse to create a local iplt for this bfd; then we'll simply skip the
> following code because elf32_arm_local_iplt will simply return NULL.

Well, as things stand you need to check the input bfd is the correct
type and thus has elf_arm_obj_tdata before even accessing
elf32_arm_local_iplt.
  
dongjianqiang (A) June 4, 2025, 6:10 a.m. UTC | #3
Hi Alan and Richard,
Thank you for reviewing this, can you please help install this patch ? I am not a binutils committer.

> -----Original Message-----
> From: Alan Modra [mailto:amodra@gmail.com]
> Sent: Wednesday, June 4, 2025 8:34 AM
> To: Richard Earnshaw <Richard.Earnshaw@arm.com>
> Cc: dongjianqiang (A) <dongjianqiang2@huawei.com>;
> binutils@sourceware.org; Zhangwen(Esan)
> <zwzhangwen.zhang@huawei.com>; Yangjian (Compiler)
> <yangjian86@huawei.com>; sam@gentoo.org
> Subject: Re: [PATCH] PR32870: ld: arm32: fix segfault when linking with
> LLVMgold.so plugin under LTO
> 
> On Tue, Jun 03, 2025 at 04:58:36PM +0100, Richard Earnshaw wrote:
> > On 15/04/2025 10:14, dongjianqiang (A) wrote:
> > > Hi,
> > >
> > > Proposed patch to PR32870.
> > > When handling the iplt in ARM32, the ld must check whether the
> input_bfd
> > > is in ELF format. If the input is an LLVM Bitcode file, an error will be raised.
> > >
> > > Any suggestions? Thanks.
> > >
> > > ---
> > > 	PR 32870
> > > 	* elf32-arm.c (elf32_arm_output_arch_local_syms): Check input_bfd
> > >
> > > diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
> > > index 3b7cee3de1c..3af964ce034 100644
> > > --- a/bfd/elf32-arm.c
> > > +++ b/bfd/elf32-arm.c
> > > @@ -18345,6 +18345,9 @@ elf32_arm_output_arch_local_syms (bfd
> *output_bfd,
> > >   	  struct arm_local_iplt_info **local_iplt;
> > >   	  unsigned int i, num_syms;
> > > +	  if (!is_arm_elf (input_bfd))
> > > +	    continue;
> > > +
> > >   	  local_iplt = elf32_arm_local_iplt (input_bfd);
> > >   	  if (local_iplt != NULL)
> > >   	    {
> >
> > I'd like Alan's opinion on this one.  To me it's strange that something that
> > is not remotely like a object file is still in the list of input BFDs at
> > this point.  This feels like it might be yet another rich source of fuzzing
> > bugs.
> 
> The patch is OK.
> 
> GNU ld can link foreign object files, if they have no relocations or
> the relocs are simple enough and the target support for that object
> properly describes them in reloc howtos.
> 
> > If keeping this in the bfd list is correct, then I think a better fix would
> > be to refuse to create a local iplt for this bfd; then we'll simply skip the
> > following code because elf32_arm_local_iplt will simply return NULL.
> 
> Well, as things stand you need to check the input bfd is the correct
> type and thus has elf_arm_obj_tdata before even accessing
> elf32_arm_local_iplt.
> 
> --
> Alan Modra
  
Richard Earnshaw (foss) June 11, 2025, 1:39 p.m. UTC | #4
On 04/06/2025 07:10, dongjianqiang (A) wrote:
> Hi Alan and Richard,
> Thank you for reviewing this, can you please help install this patch ? I am not a binutils committer.

Pushed.

Thanks for the patch,

R.
> 
>> -----Original Message-----
>> From: Alan Modra [mailto:amodra@gmail.com]
>> Sent: Wednesday, June 4, 2025 8:34 AM
>> To: Richard Earnshaw <Richard.Earnshaw@arm.com>
>> Cc: dongjianqiang (A) <dongjianqiang2@huawei.com>;
>> binutils@sourceware.org; Zhangwen(Esan)
>> <zwzhangwen.zhang@huawei.com>; Yangjian (Compiler)
>> <yangjian86@huawei.com>; sam@gentoo.org
>> Subject: Re: [PATCH] PR32870: ld: arm32: fix segfault when linking with
>> LLVMgold.so plugin under LTO
>>
>> On Tue, Jun 03, 2025 at 04:58:36PM +0100, Richard Earnshaw wrote:
>>> On 15/04/2025 10:14, dongjianqiang (A) wrote:
>>>> Hi,
>>>>
>>>> Proposed patch to PR32870.
>>>> When handling the iplt in ARM32, the ld must check whether the
>> input_bfd
>>>> is in ELF format. If the input is an LLVM Bitcode file, an error will be raised.
>>>>
>>>> Any suggestions? Thanks.
>>>>
>>>> ---
>>>> 	PR 32870
>>>> 	* elf32-arm.c (elf32_arm_output_arch_local_syms): Check input_bfd
>>>>
>>>> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
>>>> index 3b7cee3de1c..3af964ce034 100644
>>>> --- a/bfd/elf32-arm.c
>>>> +++ b/bfd/elf32-arm.c
>>>> @@ -18345,6 +18345,9 @@ elf32_arm_output_arch_local_syms (bfd
>> *output_bfd,
>>>>    	  struct arm_local_iplt_info **local_iplt;
>>>>    	  unsigned int i, num_syms;
>>>> +	  if (!is_arm_elf (input_bfd))
>>>> +	    continue;
>>>> +
>>>>    	  local_iplt = elf32_arm_local_iplt (input_bfd);
>>>>    	  if (local_iplt != NULL)
>>>>    	    {
>>>
>>> I'd like Alan's opinion on this one.  To me it's strange that something that
>>> is not remotely like a object file is still in the list of input BFDs at
>>> this point.  This feels like it might be yet another rich source of fuzzing
>>> bugs.
>>
>> The patch is OK.
>>
>> GNU ld can link foreign object files, if they have no relocations or
>> the relocs are simple enough and the target support for that object
>> properly describes them in reloc howtos.
>>
>>> If keeping this in the bfd list is correct, then I think a better fix would
>>> be to refuse to create a local iplt for this bfd; then we'll simply skip the
>>> following code because elf32_arm_local_iplt will simply return NULL.
>>
>> Well, as things stand you need to check the input bfd is the correct
>> type and thus has elf_arm_obj_tdata before even accessing
>> elf32_arm_local_iplt.
>>
>> --
>> Alan Modra
  

Patch

diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
index 3b7cee3de1c..3af964ce034 100644
--- a/bfd/elf32-arm.c
+++ b/bfd/elf32-arm.c
@@ -18345,6 +18345,9 @@  elf32_arm_output_arch_local_syms (bfd *output_bfd,
 	  struct arm_local_iplt_info **local_iplt;
 	  unsigned int i, num_syms;
 
+	  if (!is_arm_elf (input_bfd))
+	    continue;
+
 	  local_iplt = elf32_arm_local_iplt (input_bfd);
 	  if (local_iplt != NULL)
 	    {