linux-kernel - Re: [PATCH] riscv: Stop considering R_RISCV

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250711134909.GA73430@iscas.ac.cn>
Date: Fri, 11 Jul 2025 21:49:09 +0800
From: Jingwei Wang <wangjingwei@...as.ac.cn>
To: Palmer Dabbelt <palmer@...belt.com>,
	Alexandre Ghiti <alexghiti@...osinc.com>
Cc: alexghiti@...osinc.com, Nelson Chu <nelson@...osinc.com>,
	Paul Walmsley <paul.walmsley@...ive.com>, aou@...s.berkeley.edu,
	Alexandre Ghiti <alex@...ti.fr>, linux-riscv@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] riscv: Stop considering R_RISCV_NONE as bad relocations

Hi all,

On Thu, Jul 10, 2025 at 11:43:00AM -0700, Palmer Dabbelt wrote:
> On Thu, 10 Jul 2025 01:34:31 PDT (-0700), alexghiti@...osinc.com wrote:
> > Even though those relocations should not be present in the final
> > vmlinux, there are a lot of them. And since those relocations are
> > considered "bad", they flood the compilation output which may hide some
> > legitimate bad relocations.
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@...osinc.com>
> > ---
> >  arch/riscv/tools/relocs_check.sh | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/tools/relocs_check.sh b/arch/riscv/tools/relocs_check.sh
> > index baeb2e7b2290558d696afbc5429d6a3c69ae49e1..742993e6a8cba72c657dd2f8f5dabc4c415e84bd 100755
> > --- a/arch/riscv/tools/relocs_check.sh
> > +++ b/arch/riscv/tools/relocs_check.sh
> > @@ -14,7 +14,9 @@ bad_relocs=$(
> >  ${srctree}/scripts/relocs_check.sh "$@" |
> >  	# These relocations are okay
> >  	#	R_RISCV_RELATIVE
> > -	grep -F -w -v 'R_RISCV_RELATIVE'
> > +	#	R_RISCV_NONE
> > +	grep -F -w -v 'R_RISCV_RELATIVE
> > +R_RISCV_NONE'
> >  )
>
> I'm not super opposed to it, but is there a way to just warn once or
> something?  It's probably best to still report something, as there's likely
> some sort of toolchain issue here.
>

I think Alexandre's approach is ideal from the kernel's perspective.
This doesn't really seem to be a bug; I see it more as a case where the
toolchain's handling of R_RISCV_NONE doesn't quite match the kernel's
expectations.

I found the large number of R_RISCV_NONE relocs only appear in the final
vmlinux. The key difference is the kernel build's --emit-relocs flag
and the *(.rela.text*) directive in vmlinux.lds.S. This combination
forces all relocation entries, including those marked as R_RISCV_NONE,
to be written verbatim into the final vmlinux.

I traced this back to BFD's implementation and found that during
relaxation (e.g., when an auipc+jalr is optimized to a jal), the linker
modifies the first reloc slot to R_RISCV_JAL and marks the second,
now-useless slot as R_RISCV_DELETE. In the cleanup phase, to prevent
reprocessing, BFD then changes the cleaned-up DELETE marker to
R_RISCV_NONE. This is how, when the kernel specifies --emit-relocs,
these R_RISCV_NONE markers get preserved in the final .rela.text section.

To truly change this, it seems to depend on whether the binutils
is willing to add a stage to clean up these harmless but
useless markers.

If possible, I was thinking we could perhaps iterate and remove the
R_RISCV_NONE entries from .rela.text before the alignment pass.

But if there's no agreement on the BFD side, Alexandre's approach still
seems correct and aligns with the psABI, where R_RISCV_NONE has no
operational meaning.

> Also: if you can reproduce it, Nelson can probably fix it.  I'm CCing him.
>

Reproducing the issue is simple: you just need a call instruction to
create a relaxation opportunity, then link with --emit-relocs and a
linker script that includes *(.rela.text*). :)

For convenience, I've put a minimal, self-contained reproduction case
here: https://gist.github.com/Jingwiw/762606e1dc3b77c352b394e8c5e846de

> >
> >  if [ -z "$bad_relocs" ]; then
>

Reviewed-by: Jingwei Wang <wangjingwei@...as.ac.cn>

Thanks,
Jingwei