[Intel IOMMU] Question about memory ordering in context/PASID entry updates

[Intel IOMMU] Question about memory ordering in context/PASID entry updates

[Dmytro Maluka]

When updating a context or PASID entry, we'd better make sure we set the
present bit after setting up other bits, not before, right? And indeed,
for example, in domain_context_mapping_one() we are doing it in the
following order:

	context_clear_entry(context);
	<set other bits>
	context_set_present(context);

But it doesn't look like we have any compiler barriers to actually
ensure this order? (And in case of context entries, there is even no
WRITE_ONCE or anything like that.)

So, are we just hoping that the compiler will not reorder it?

What am I missing?

Re: [Intel IOMMU] Question about memory ordering in context/PASID entry updates

[Baolu Lu]

On 12/5/25 00:49, Dmytro Maluka wrote:
> When updating a context or PASID entry, we'd better make sure we set the
> present bit after setting up other bits, not before, right? And indeed,

Yes, you are right.

> for example, in domain_context_mapping_one() we are doing it in the
> following order:
> 
> 	context_clear_entry(context);
> 	<set other bits>
> 	context_set_present(context);
> 
> But it doesn't look like we have any compiler barriers to actually
> ensure this order? (And in case of context entries, there is even no
> WRITE_ONCE or anything like that.)

Yes, we should do that to ensure the order.

> So, are we just hoping that the compiler will not reorder it?
> 
> What am I missing?

We didn't implement that primarily because we hadn't encountered any
real issues, operating under the assumption that the root/context
entries population is completed before DMA translation is really
enabled.

However, that assumption is incorrect, as users can change a device's
default domain in VT-d legacy mode. This action triggers a context entry
change while DMA translation is already running. So, yes, this is a bug.

Thanks,
baolu

Re: [Intel IOMMU] Question about memory ordering in context/PASID entry updates

[Dmytro Maluka]

On Fri, Dec 05, 2025 at 01:32:51PM +0800, Baolu Lu wrote:
> We didn't implement that primarily because we hadn't encountered any
> real issues, operating under the assumption that the root/context
> entries population is completed before DMA translation is really
> enabled.
> 
> However, that assumption is incorrect, as users can change a device's
> default domain in VT-d legacy mode. This action triggers a context entry
> change while DMA translation is already running. So, yes, this is a bug.

I see, thanks. I might try to prepare a patch, though it seems not
exactly trivial to fix this all over the code. Roughly seems we could
just modify the context_set_*() helpers to use READ_ONCE/WRITE_ONCE
similarly to pasid_set_*(), plus add barrier() before
context_set_present() and pasid_set_present() calls, plus maybe we
should also correspondingly fix updating root table entries in
iommu_context_addr() etc.

BTW why exactly I'm concerned about this: even if no one has encountered
any functional issues caused by this, DMA isolation is also a security
thing, and an attacker might be able to exploit this (albeit only in
case those operations actually happen to be reordered by the compiler in
a dangerous way in the given kernel build).

> as users can change a device's
> default domain in VT-d legacy mode.

Not in scalable mode? I'd assume users can change it in both, and I
don't immediately see anything in the code that would limit it to legacy
mode only.

> Thanks,
> baolu

Re: [Intel IOMMU] Question about memory ordering in context/PASID entry updates

[Baolu Lu]

On 12/5/25 22:09, Dmytro Maluka wrote:
>> as users can change a device's
>> default domain in VT-d legacy mode.
> Not in scalable mode? I'd assume users can change it in both, and I
> don't immediately see anything in the code that would limit it to legacy
> mode only.

In the scalable mode case, the context entry is not altered when
changing the default domain. But, yes, users can trigger context entry
changes in other cases. So this applies to both modes.

Thanks,
baolu