Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
Date:   Wed, 23 Feb 2022 17:18:04 -0800
From:   Joao Moreira <joao@overdrivepizza.com>
To:     Peter Zijlstra <peterz@infradead.org>
Cc:     x86@kernel.org, hjl.tools@gmail.com, jpoimboe@redhat.com,
        andrew.cooper3@citrix.com, linux-kernel@vger.kernel.org,
        ndesaulniers@google.com, keescook@chromium.org,
        samitolvanen@google.com, mark.rutland@arm.com,
        alyssa.milburn@intel.com
Subject: Re: [PATCH 24/29] x86/text-patching: Make text_gen_insn() IBT aware
In-Reply-To: <20220218171409.990016403@infradead.org>
References: <20220218164902.008644515@infradead.org>
 <20220218171409.990016403@infradead.org>
Message-ID: <5c77889ca6b988b0dff65136264bd1fb@overdrivepizza.com>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

> +#ifdef CONFIG_X86_IBT
> +	if (is_endbr(dest))
> +		dest += 4;
> +#endif

Hi, FWIIW I saw this snippet trigger a bug in the jump_label infra where 
the target displacement would not fit in a JMP8 operand. The behavior 
was seen because clang, for whatever reason (probably a bug?) inlined an 
ENDBR function along with a function, thus the JMP8 target was 
incremented. I compared the faulty kernel to one compiled with GCC and 
the latter wont emit/inline the ENDBR.

The displacement I'm using in my experimentation is a few bytes more 
than just 4, because I'm also adding extra instrumentation that should 
be skipped when not reached indirectly. Of course this is more prone to 
triggering the bug, but I don't think it is impossible to happen in the 
current implementation.

For these cases perhaps we can verify if the displacement fits the 
operand and, if not, simply ignore and lose the decode cycle which may 
not be a huge problem and remains semantically correct. Seems more 
sensible than padding jump tables with nops. In the meantime I'll 
investigate clang's behavior and if it is really a bug, I'll work on a 
patch.