Re: [EXT] Re: [PATCH v3 03/13] task_isolation: add instruction synchronization memory barrier

On Mon, Apr 20, 2020 at 02:55:23PM +0100, Will Deacon wrote:

On Mon, Apr 20, 2020 at 01:36:28PM +0100, Mark Rutland wrote:

quoted

On Mon, Apr 20, 2020 at 01:23:51PM +0100, Will Deacon wrote:

quoted

IIUC, we don't need to do anything on arm64 because taking an exception acts
as a context synchronization event, so I don't think you should try to
expose this as a new barrier macro. Instead, just make it a pre-requisite
that architectures need to ensure this behaviour when entering the kernel
from userspace if they are to select HAVE_ARCH_TASK_ISOLATION.

The CSE from the exception isn't sufficient here, because it needs to
occur after the CPU has re-registered to receive IPIs for
kick_all_cpus_sync(). Otherwise there's a window between taking the
exception and re-registering where a necessary context synchronization
event can be missed. e.g.

CPU A				CPU B
[ Modifies some code ]		
				[ enters exception ]
[ D cache maintenance ]
[ I cache maintenance ]
[ IPI ]				// IPI not taken
  ...				[ register for IPI ] 
[ IPI completes ] 
				[ execute stale code here ]

Thanks.

quoted

However, I think 'IMB' is far too generic, and we should have an arch
hook specific to task isolation, as it's far less likely to be abused as
IMB will.

What guarantees we don't run any unsynchronised module code between
exception entry and registering for the IPI? It seems like we'd want that
code to run as early as possible, e.g. as part of
task_isolation_user_exit() but that doesn't seem to be what's happening.

Sorry, I guess that's more a question for Alex.

Alex -- do you think we could move the "register for IPI" step earlier
so that it's easier to reason about the code that runs in the dead zone
during exception entry?

Will