| Message ID | 3188347c-3375-b728-cd08-ea4421d823cd@I-love.SAKURA.ne.jp (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] ARM: spectre-v2: fix smp_processor_id() warning | expand |
On Fri, Jul 15, 2022 at 10:09:01PM +0900, Tetsuo Handa wrote: > syzbot is reporting that CONFIG_HARDEN_BRANCH_PREDICTOR=y + > CONFIG_DEBUG_PREEMPT=y on ARM32 causes "BUG: using smp_processor_id() in > preemptible code" message [1], for this check was not designed to handle > attempts to access kernel memory like > > ---------- > int main() { return *(char *) -1; } > ---------- > > . Although Russell King commented that this BUG: message might help finding > possible exploit attempts [2], this is not a kernel's problem that worth > giving up fuzz testing. But shutting up a valid warning when the real problem is still there is also not acceptable. As I've said many times, the workaround needs to be run on the _same_ CPU that faulted. The warning is telling us that we're preemptible at this point, which means that can't be guaranteed. So bodging it by disabling preemption around here DOES NOT FIX THE PROBLEM. It _SHUTS UP THE VALID WARNING_. And shutting up a valid warning is A VERY BAD PRACTICE. NAK. MAK. NAK. NAK. NAK.
On 2022/07/15 22:36, Russell King (Oracle) wrote: > On Fri, Jul 15, 2022 at 10:09:01PM +0900, Tetsuo Handa wrote: >> syzbot is reporting that CONFIG_HARDEN_BRANCH_PREDICTOR=y + >> CONFIG_DEBUG_PREEMPT=y on ARM32 causes "BUG: using smp_processor_id() in >> preemptible code" message [1], for this check was not designed to handle >> attempts to access kernel memory like >> >> ---------- >> int main() { return *(char *) -1; } >> ---------- >> >> . Although Russell King commented that this BUG: message might help finding >> possible exploit attempts [2], this is not a kernel's problem that worth >> giving up fuzz testing. > > But shutting up a valid warning when the real problem is still there is > also not acceptable. Then, at least for now can we stop emitting the BUG: string? I showed an idea at https://lkml.kernel.org/r/fa786d1c-db06-f7f1-9ac9-6a468c1e8d81@I-love.SAKURA.ne.jp but I got no response. Since syzkaller stops fuzz testing upon encountering BUG: or WARNING: string, ARM32 might be failing to find other bugs for 491 days due to this problem. If you don't want to stop emitting this BUG: string, we might want to teach syzkaller to build ARM32 kernels with CONFIG_HARDEN_BRANCH_PREDICTOR=n.
diff --git a/arch/arm/include/asm/system_misc.h b/arch/arm/include/asm/system_misc.h index 98b37340376b..670e8d116770 100644 --- a/arch/arm/include/asm/system_misc.h +++ b/arch/arm/include/asm/system_misc.h @@ -20,10 +20,13 @@ typedef void (*harden_branch_predictor_fn_t)(void); DECLARE_PER_CPU(harden_branch_predictor_fn_t, harden_branch_predictor_fn); static inline void harden_branch_predictor(void) { - harden_branch_predictor_fn_t fn = per_cpu(harden_branch_predictor_fn, - smp_processor_id()); + harden_branch_predictor_fn_t fn; + + preempt_disable_notrace(); + fn = per_cpu(harden_branch_predictor_fn, raw_smp_processor_id()); if (fn) fn(); + preempt_enable_no_resched_notrace(); } #else #define harden_branch_predictor() do { } while (0)
syzbot is reporting that CONFIG_HARDEN_BRANCH_PREDICTOR=y + CONFIG_DEBUG_PREEMPT=y on ARM32 causes "BUG: using smp_processor_id() in preemptible code" message [1], for this check was not designed to handle attempts to access kernel memory like ---------- int main() { return *(char *) -1; } ---------- . Although Russell King commented that this BUG: message might help finding possible exploit attempts [2], this is not a kernel's problem that worth giving up fuzz testing. This patch explicitly disables preemption and uses raw_smp_processor_id(). Link: https://syzkaller.appspot.com/bug?extid=a7ee43e564223f195c84 [1] Link: https://lkml.kernel.org/r/YrMhVAev9wMAA8tl@shell.armlinux.org.uk [2] Reported-by: syzbot <syzbot+a7ee43e564223f195c84@syzkaller.appspotmail.com> Fixes: f5fe12b1eaee220c ("ARM: spectre-v2: harden user aborts in kernel space") Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- arch/arm/include/asm/system_misc.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)