Hi,
I examined the TLB shootdown code in generic/src/mm/tlb.c
and I am wondering if there isn't a time window for a
race.
Minor bugs
----------
First, look at tlb_shootdown_ipi_recv():
void tlb_shootdown_ipi_recv(void)
{
// snip
for (i = 0; i < CPU->tlb_messages_count; CPU->tlb_messages_count--) {
tlb_invalidate_type_t type = CPU->tlb_messages[i].type;
// snip, no changes to "i" or tlb_message_count
if (type == TLB_INVL_ALL)
break;
}
//snip, no change to "i" or tlb_message_count
}
There are two minor bugs:
1) The for() loop executes the first message in the
queue "tlb_messages_count" times instead of processing
all messages one by one. So instead of:
for (i = 0; i < CPU->tlb_messages_count; CPU->tlb_messages_count--) {
}
.. it should be:
for (i = 0; i < CPU->tlb_messages_count; i++) {
}
CPU->tlb_messages_count = 0;
2) If it encounters a message/request to flush the
entire TLB if breaks from the loop because the remaining
messages have nothing to change in the TLB. Unfortunately,
tlb_messages_count is left unchanged, so the next time
tlb_shootdown_ipi_recv() runs, it will process previous
messages again. Ooops :-).
TLB shootdown algo
------------------
If I understand the TLB shootdown algorithm correctly,
tlb_shootdown_start() waits for all cpus to enter
the TLB shootdown code and start spinning. Once we
invoke tlb_shootdown_finalize(), all cpus stop spinning
and run the local part of their TLB shootdown algo
flushing any page mappings specified in tlb_shootdown-
_start() from their TLBs. Therefore, we can be sure
that any frames unmapped from the AS and page table
before tlb_shootdown_finalize() will *not* be accessed
after tlb_shootdown_finalize() returns. Once tlb_shoot-
down_finalize() returns, other cpus will only access
page frames used by the algo (eg stack and code pages)
before getting rid of the outdated mappings from their
TLBs for good.
Race?
-----
I am not sure if the code accounts for the possibility
of a race between an unrelated interrupt and a pending
TLB shootdown IPI (ie its handler tlb_shootdown_ipi_recv())
after tlb_shootdown_finalize() returns.
Imagine two cpus invoke tlb_shootdown_start() at the
same time. Say cpu1 acquires the tlblock spinlock and
cpu2 remains spinning for tlblock. cpu1 unmaps a page
and calls tlb_shootdown_finalize(). Now cpu2 should
remove the mapping from its local TLB. It, however,
executes the whole tlb_shootdown_start(). Interrupts
are disabled so until the kernel/cpu2 calls tlb_shoot-
down_finalize() (which restores interrupts) it won't
invoke its pending TLB IPI handler tlb_shootdown_ipi-
_recv() and remove the outdated mapping from its local
TLB. cpu2, therefore, calls tlb_shootdown_finalize()
and restores interrupts. The problem is that even though
tlb_shootdown_ipi_recv()'s interrupt is pending, it
might not be processed right after cpu2's tlb_shootdown-
_finalize() due to another pending interrupt. Depending
on the order in which interrupts are processed this
other interrupt may be handled first and may access
the unmapped page (but still in cpu2's TLB) and corrupt
a frame before tlb_shootdown_ipi_recv runs and purges
the TLB. Suppose that the other interrupt handles eg
the network card and the just unmapped page happens
to be a buffer supplied by user space to store incoming
traffic. In absence of the race, the kernel would kill
the offending user program for accessing unmapped memory.
In presence of the race, the user program may alter
contents of a kernel owned frame.
Moreover, if the kernel on cpu2 is buggy and accesses
the now unmapped page before its tlb_shootdown_finalize(),
it may silently corrupt a frame instead of raising a
page fault and panicing.
I have not studied the code in greater detail, so it
may be possible that eg locks in the AS code prevent
the race from occurring.
Proposed solution
-----------------
The race could be averted if tlb_shootdown_start processed
its own message queue before returning.
Adam
_______________________________________________
HelenOS-devel mailing list
[email protected]
http://lists.modry.cz/cgi-bin/listinfo/helenos-devel
