Date:   Fri, 3 Feb 2023 16:12:06 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Borislav Petkov <bp@alien8.de>
Cc:     x86@kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
        "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Andrew Cooper <Andrew.Cooper3@citrix.com>,
        Josh Poimboeuf <jpoimboe@redhat.com>,
        linux-kernel@vger.kernel.org
Subject: Re: [RFC] x86/alternative: Support relocations in alternatives
Message-ID: <Y90kRtZA0d8nsRRj@hirez.programming.kicks-ass.net>
References: <Y9py2a5Xw0xbB8ou@hirez.programming.kicks-ass.net>
 <Y90IaYHDbCN9P0OX@zn.tnic>
 <Y90ilxBFMAQZ/dRg@hirez.programming.kicks-ass.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <Y90ilxBFMAQZ/dRg@hirez.programming.kicks-ass.net>
Precedence: bulk

On Fri, Feb 03, 2023 at 04:04:55PM +0100, Peter Zijlstra wrote:

> > > +		switch (insn.opcode.bytes[0]) {
> > > +		case 0x0f:
> > > +			if (insn.opcode.bytes[1] < 0x80 ||
> > > +			    insn.opcode.bytes[1] > 0x8f)
> > > +				break;
> > > +
> > > +			fallthrough;	/* Jcc.d32 */
> > > +		case 0x70 ... 0x7f:	/* Jcc.d8 */
> > > +		case JMP8_INSN_OPCODE:
> > > +		case JMP32_INSN_OPCODE:
> > > +		case CALL_INSN_OPCODE:
> > > +			u8 *target = src + i + insn.length + insn.immediate.value;
> > > +			if (target < src || target > src + src_len) {
> > > +				apply_reloc(insn.immediate.nbytes,
> > > +					    instr + i + insn_offset_immediate(&insn),
> > > +					    src - dest);
> > > +			}
> > 
> > Uff, it took me a while to parse this. So this can be simpler. The basic
> > math is:
> > 
> > 	new_offset = next_rip - target_address;
> > 
> > where
> > 	next_rip = instr + insn.length;
> > 
> > and I admit that my function was a bit clumsy but I think yours can be
> > made simpler while keeping it cleaner.
> 
> I'm not sure what you're saying here... so let me walk through the whole
> thing (specifically the immediate case, since that's what you quote). So
> what we need is:
> 
> 	src_imm = target - src_next_ip			(1)
> 
> what we want is:
> 
> 	dst_imm = target - dst_next_ip			(2)
> 
> so what we do is rework (1) as an expression for target like:
> 
> 	target = src_imm + src_next_ip			(1a)
> 
> and substitute in (2) to get:
> 
> 	dst_imm = (src_imm + src_next_ip) - dst_next_ip	(3)
> 
> Now, since the instruction stream is 'identical' at src and dest
> we can state that:
> 
> 	src_next_ip = src + ip_offset
> 	dst_next_ip = dst + ip_offset			(4)
> 
> Substitute (4) in (3) and observe ip_offset being cancelled out to
> obtain:
> 
> 	dst_imm = src_imm + (src + ip_offset) - (dst + ip_offset)
> 	        = src_imm + src - dst + ip_offset - ip_offset
> 	        = src_imm + src - dst			(5)
> 
> The very thing I did.
> 
> IOW, we can correct the displacement without caring about which actual
> instruction in the stream we're correcting since the relative offset is
> given by 'src - dst'. IOW, we don't give a crap about insn.length in
> this case ;-)

Specifically, in the above case ip_offset would be given by:

	ip_offset = i + insn.length

where i is the offset in the buffer to the current instruction and
insn.length is well, the length of the current instruction ;-)