linux-kernel - Re: [PATCH v4 2/7] arm64: introduce interfaces to hotpatch kernel and module code

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: <20131018085638.GA2858@mudshark.cambridge.arm.com>
Date:	Fri, 18 Oct 2013 09:56:38 +0100
From:	Will Deacon <will.deacon@....com>
To:	"Jon Medhurst (Tixy)" <tixy@...aro.org>
Cc:	Jiang Liu <liuj97@...il.com>, Steven Rostedt <rostedt@...dmis.org>,
	Catalin Marinas <Catalin.Marinas@....com>,
	Sandeepa Prabhu <sandeepa.prabhu@...aro.org>,
	Jiang Liu <jiang.liu@...wei.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 2/7] arm64: introduce interfaces to hotpatch kernel
 and module code

Hi Tixy,

On Thu, Oct 17, 2013 at 04:24:01PM +0100, Jon Medhurst (Tixy) wrote:
> On Thu, 2013-10-17 at 12:38 +0100, Will Deacon wrote:
> > On Thu, Oct 17, 2013 at 07:19:35AM +0100, Jiang Liu wrote:
> > > +	/*
> > > +	 * Execute __aarch64_insn_patch_text() on every online CPU,
> > > +	 * which ensure serialization among all online CPUs.
> > > +	 */
> > > +	return stop_machine(aarch64_insn_patch_text_cb, &patch, NULL);
> > > +}
> > 
> > Whoa, whoa, whoa! The comment here is wrong -- we only run the patching on
> > *one* CPU, which is the right thing to do. However, the arch/arm/ call to
> > stop_machine in kprobes does actually run the patching code on *all* the
> > online cores (including the cache flushing!). I think this is to work around
> > cores without hardware cache maintenance broadcasting, but that could easily
> > be called out specially (like we do in patch.c) and the flushing could be
> > separated from the patching too.
> [...]
> 
> For code modifications done in 32bit ARM kprobes (and ftrace) I'm not
> sure we ever actually resolved the possible cache flushing issues. If
> there was specific reasons for flushing on all cores I can't remember
> them, sorry. I have a suspicion that doing so was a case of sticking
> with what the code was already doing, and flushing on all cores seemed
> safest to guard against problems we hadn't thought about.

[...]

> Sorry, I don't think I've added much light on things here have I?

I think you missed the bit I was confused about :) Flushing the cache on
each core is necessary if cache_ops_need_broadcast, so I can understand why
you'd have code to do that. The bit I don't understand is that you actually
patch the instruction on each core too!

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/