linux-kernel - Re: [PATCH v2 52/62] objtool/klp: Introduce klp diff subcommand for diffing object files

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 for Android: free password hash cracker in your pocket

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

Message-ID: <hfs4v4frscapujmbdgpsbhnuvsqu3s6gltquyioatyeysed64e@izldzwjqsvu4>
Date: Thu, 5 Jun 2025 00:32:12 -0700
From: Josh Poimboeuf <jpoimboe@...nel.org>
To: Peter Zijlstra <peterz@...radead.org>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org, 
	Petr Mladek <pmladek@...e.com>, Miroslav Benes <mbenes@...e.cz>, 
	Joe Lawrence <joe.lawrence@...hat.com>, live-patching@...r.kernel.org, Song Liu <song@...nel.org>, 
	laokz <laokz@...mail.com>, Jiri Kosina <jikos@...nel.org>, 
	Marcos Paulo de Souza <mpdesouza@...e.com>, Weinan Liu <wnliu@...gle.com>, 
	Fazla Mehrab <a.mehrab@...edance.com>, Chen Zhongjin <chenzhongjin@...wei.com>, 
	Puranjay Mohan <puranjay@...nel.org>
Subject: Re: [PATCH v2 52/62] objtool/klp: Introduce klp diff subcommand for
 diffing object files

On Mon, May 26, 2025 at 08:27:19PM +0200, Peter Zijlstra wrote:
> On Fri, May 09, 2025 at 01:17:16PM -0700, Josh Poimboeuf wrote:
> > +static void scan_for_insn(struct section *sec, unsigned long offset,
> > +			  unsigned long *insn_off, unsigned int *insn_len)
> > +{
> > +	unsigned long o = 0;
> > +	struct insn insn;
> > +
> > +	while (1) {
> > +
> > +		insn_decode(&insn, sec->data->d_buf + o, sec_size(sec) - o,
> > +			    INSN_MODE_64);
> > +
> > +		if (o + insn.length > offset) {
> > +			*insn_off = o;
> > +			*insn_len = insn.length;
> > +			return;
> > +		}
> > +
> > +		o += insn.length;
> > +	}
> > +}
> > +
> > +u64 arch_adjusted_addend(struct reloc *reloc)
> > +{
> > +	unsigned int type = reloc_type(reloc);
> > +	s64 addend = reloc_addend(reloc);
> > +	unsigned long insn_off;
> > +	unsigned int insn_len;
> > +
> > +	if (type == R_X86_64_PLT32)
> > +		return addend + 4;
> > +
> > +	if (type != R_X86_64_PC32 || !is_text_sec(reloc->sec->base))
> > +		return addend;
> > +
> > +	scan_for_insn(reloc->sec->base, reloc_offset(reloc),
> > +		      &insn_off, &insn_len);
> > +
> > +	return addend + insn_off + insn_len - reloc_offset(reloc);
> > +}
> 
> This looks like a rather expensive proposition; it will have to decode
> the section nr_reloc times.
> 
> Does it not make more sense to fully decode the section like 'normal' ?

Yeah, I'm not crazy about it either, but it at least keeps the pain
nicely localized to x86, and avoids pulling in struct instruction,
struct objtool_file, etc.

Also this typically doesn't need to be all that fast as this is only
done for changed functions, and only for a subset of relocations (those
which might be references to non-bundled data in a text section).

To give a general idea, in one of my tests, for a patch with 22
functions, it only calls scan_for_insn() 41 times.

-- 
Josh