On 10/21/21 5:21 AM, Nadav Amit wrote:
> access_error() currently does not check for execution permission
> violation.
Ye
> As a result, spurious page-faults due to execution permission
> violation cause SIGSEGV.
While I could totally believe that something is goofy when VMAs are
being changed underneath a page fault, I'm having trouble figuring out
why the "if (error_code & X86_PF_WRITE)" code is being modified.
> It appears not to be an issue so far, but the next patches avoid TLB
> flushes on permission promotion, which can lead to this scenario. nodejs
> for instance crashes when TLB flush is avoided on permission promotion.
Just to be clear, "promotion" is going from something like:
W=0->W=1
or
NX=1->NX=0
right? I tend to call that "relaxing" permissions.
Currently, X86_PF_WRITE faults are considered an access error unless the
VMA to which the write occurred allows writes. Returning "no access
error" permits continuing and handling the copy-on-write.
It sounds like you want to expand that. You want to add a whole class
of new faults that can be ignored: not just that some COW handling might
be necessary, but that the PTE itself might be out of date. Just like
a "COW fault" may just result in setting the PTE.W=1 and moving on with
our day, an instruction fault might now just end up with setting
PTE.NX=0 and also moving on with our day.
I'm really confused why the "error_code & X86_PF_WRITE" case is getting
modified. I would have expected it to be something like just adding:
/* read, instruction fetch */
if (error_code & X86_PF_INSN) {
/* Avoid enforcing access error if spurious: */
if (unlikely(!(vma->vm_flags & VM_EXEC)))
return 1;
return 0;
}
I'm really confused what X86_PF_WRITE and X86_PF_INSN have in common
other than both being able to (now) be generated spuriously.
